Commons IO
  1. Commons IO
  2. IO-280

Dubious use of mkdirs() return code

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 2.1
    • Component/s: None
    • Labels:
      None

      Description

      FileUtils.openOutputStream() has the following code:

      File parent = file.getParentFile();
      if (parent != null && parent.exists() == false) {
          if (parent.mkdirs() == false) {
              throw new IOException("File '" + file + "' could not be created");
          }
      }
      

      Now mkdirs() returns true only if the method actually created the directories; it's theoretically possible for the directory to be created in the window between the exists() and mkdirs() invocations. [Indeed the class actually checks for this in the forceMkdir() method]

      It would be safer to use:

      File parent = file.getParentFile();
      if (parent != null && !parent.mkdirs() && !parent.isDirectory()) {
          throw new IOException("Directory '" + parent + "' could not be created"); // note changed text
      }
      

      Similarly elsewhere in the class where mkdirs() is used.

        Issue Links

          Activity

          Transition Time In Source Status Execution Times Last Executer Last Execution Date
          Open Open Resolved Resolved
          14d 16h 16m 1 Sebb 30/Aug/11 14:56
          Resolved Resolved Closed Closed
          41d 1h 51m 1 Gary Gregory 10/Oct/11 16:47
          Sebb made changes -
          Description FileUtils.openOutputStream() has the following code:

          {code}
          File parent = file.getParentFile();
          if (parent != null && parent.exists() == false) {
              if (parent.mkdirs() == false) {
                  throw new IOException("File '" + file + "' could not be created");
              }
          }
          {code}

          Now mkdirs() returns true only if the method actually created the directories; it's theoretically possible for the directory to be created in the window between the exists() and mkdirs() invocations. [Indeed the class actually checks for this in the forceMkdir() method]

          It would be safer to use:

          {code}
          File parent = file.getParentFile();
          if (parent != null && !parent.mkdirs() && !parent.isDirectory()) {
                  throw new IOException("Directory '" + parent + "' could not be created"); // note changed text
              }
          }
          {code}

          Similarly elsewhere in the class where mkdirs() is used.
          FileUtils.openOutputStream() has the following code:

          {code}
          File parent = file.getParentFile();
          if (parent != null && parent.exists() == false) {
              if (parent.mkdirs() == false) {
                  throw new IOException("File '" + file + "' could not be created");
              }
          }
          {code}

          Now mkdirs() returns true only if the method actually created the directories; it's theoretically possible for the directory to be created in the window between the exists() and mkdirs() invocations. [Indeed the class actually checks for this in the forceMkdir() method]

          It would be safer to use:

          {code}
          File parent = file.getParentFile();
          if (parent != null && !parent.mkdirs() && !parent.isDirectory()) {
              throw new IOException("Directory '" + parent + "' could not be created"); // note changed text
          }
          {code}

          Similarly elsewhere in the class where mkdirs() is used.
          Gary Gregory made changes -
          Fix Version/s 2.1 [ 12316027 ]
          Gary Gregory made changes -
          Status Resolved [ 5 ] Closed [ 6 ]
          Hide
          Gary Gregory added a comment -

          Closing, we released version 2.1.

          Show
          Gary Gregory added a comment - Closing, we released version 2.1.
          Sebb made changes -
          Status Open [ 1 ] Resolved [ 5 ]
          Resolution Fixed [ 1 ]
          Sebb made changes -
          Field Original Value New Value
          Link This issue relates to JCI-67 [ JCI-67 ]
          Sebb created issue -

            People

            • Assignee:
              Unassigned
              Reporter:
              Sebb
            • Votes:
              0 Vote for this issue
              Watchers:
              0 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development