Hadoop Common
  1. Hadoop Common
  2. HADOOP-7945

Document that Path objects do not support ":" in them.

    Details

    • Type: Improvement Improvement
    • Status: Patch Available
    • Priority: Critical Critical
    • Resolution: Unresolved
    • Affects Version/s: 0.20.0
    • Fix Version/s: None
    • Component/s: None
    • Labels:

      Description

      Until HADOOP-3257 is fixed, this particular exclusion should be documented. This is a major upsetter to many beginners.

        Issue Links

          Activity

          Hide
          Harsh J added a comment -

          We can document this in Path's javadocs, I think that should suffice. But brownie points if you can think of somewhere more visible and relevant

          Show
          Harsh J added a comment - We can document this in Path's javadocs, I think that should suffice. But brownie points if you can think of somewhere more visible and relevant
          Hide
          Niels Basjes added a comment -

          How about adding simple input check in Path that throws an exception with a descriptive message. That way the log files of the job/task contain the reason for the failure.

          Show
          Niels Basjes added a comment - How about adding simple input check in Path that throws an exception with a descriptive message. That way the log files of the job/task contain the reason for the failure.
          Hide
          Harsh J added a comment -

          I guess that could bring in compatibility issues. (People begin expecting that to fail, and at some point in future a bug fix like HADOOP-3257 regresses against that thought.) But otherwise, that idea is sound.

          Do we emit no "Invalid char in path" form of messages today? If we do, then this is not a worry, we could inject looking for colon in that somehow.

          Want to take a shot at it Niels?

          Show
          Harsh J added a comment - I guess that could bring in compatibility issues. (People begin expecting that to fail, and at some point in future a bug fix like HADOOP-3257 regresses against that thought.) But otherwise, that idea is sound. Do we emit no "Invalid char in path" form of messages today? If we do, then this is not a worry, we could inject looking for colon in that somehow. Want to take a shot at it Niels?
          Hide
          Niels Basjes added a comment -

          Quick shot: Simply add another check to the already existing checkPathArg method in the Path class.
          I.e. something like this (I never tested this code):

          diff --git hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/Path.java hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/Path.java
          index 81c79db..e135efb 100644
          — hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/Path.java
          +++ hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/Path.java
          @@ -90,7 +90,13 @@ public class Path implements Comparable {
          if( path.length() == 0 )

          { throw new IllegalArgumentException( "Can not create a Path from an empty string"); - }


          + }
          + if( path.contains(":") )

          { + throw new IllegalArgumentException( + "Can not create Path because Path is limited by URI semantics "+ + "in the sense that one cannot create files whose names include "+ + "characters such as \":\" etc."); + }

          }

          /** Construct a path from a String. Path strings are URIs, but with

          Show
          Niels Basjes added a comment - Quick shot: Simply add another check to the already existing checkPathArg method in the Path class. I.e. something like this (I never tested this code): diff --git hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/Path.java hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/Path.java index 81c79db..e135efb 100644 — hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/Path.java +++ hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/Path.java @@ -90,7 +90,13 @@ public class Path implements Comparable { if( path.length() == 0 ) { throw new IllegalArgumentException( "Can not create a Path from an empty string"); - } + } + if( path.contains(":") ) { + throw new IllegalArgumentException( + "Can not create Path because Path is limited by URI semantics "+ + "in the sense that one cannot create files whose names include "+ + "characters such as \":\" etc."); + } } /** Construct a path from a String. Path strings are URIs, but with
          Hide
          Niels Basjes added a comment -

          Sorry, that was unreadable.

          diff --git hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/Path.java hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/Path.java
          index 81c79db..e135efb 100644
          --- hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/Path.java
          +++ hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/Path.java
          @@ -90,7 +90,13 @@ public class Path implements Comparable {
               if( path.length() == 0 ) {
                  throw new IllegalArgumentException(
                      "Can not create a Path from an empty string");
          -    }   
          +    }
          +    if( path.contains(":") ) {
          +      throw new IllegalArgumentException(
          +          "Can not create Path because Path is limited by URI semantics "+
          +          "in the sense that one cannot create files whose names include "+
          +          "characters such as \":\" etc.");
          +    }
             }
             
             /** Construct a path from a String.  Path strings are URIs, but with
          
          
          Show
          Niels Basjes added a comment - Sorry, that was unreadable. diff --git hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/Path.java hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/Path.java index 81c79db..e135efb 100644 --- hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/Path.java +++ hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/Path.java @@ -90,7 +90,13 @@ public class Path implements Comparable { if ( path.length() == 0 ) { throw new IllegalArgumentException( "Can not create a Path from an empty string" ); - } + } + if ( path.contains( ":" ) ) { + throw new IllegalArgumentException( + "Can not create Path because Path is limited by URI semantics " + + "in the sense that one cannot create files whose names include " + + "characters such as \" :\ " etc." ); + } } /** Construct a path from a String . Path strings are URIs, but with
          Hide
          Harsh J added a comment -

          Nope, given checkPathArg's positioning, that wouldn't work for path that are fully qualified (i.e. need a ":" as part of the file:///path/to/some/file/:/with/colon).

          Show
          Harsh J added a comment - Nope, given checkPathArg's positioning, that wouldn't work for path that are fully qualified (i.e. need a ":" as part of the file:///path/to/some/file/:/with/colon ).
          Hide
          Niels Basjes added a comment -

          I added a unit test to force to trigger the error. It passes this and all existing unit tests.
          So I think I found the right spot.
          Harsh, can you check it?

          Show
          Niels Basjes added a comment - I added a unit test to force to trigger the error. It passes this and all existing unit tests. So I think I found the right spot. Harsh, can you check it?
          Hide
          Harsh J added a comment -

          Am thinking it might just be better to document this and add a test case (that'd fail the day its fixed).

          Maybe this approach could cause incompatibility, as am not sure if there are ways to 'tweak' the Path into accepting ":" (escaped/encoded).

          Another character that causes issue is a slash in the filename itself, even if escaped/encoded.

          Show
          Harsh J added a comment - Am thinking it might just be better to document this and add a test case (that'd fail the day its fixed). Maybe this approach could cause incompatibility, as am not sure if there are ways to 'tweak' the Path into accepting ":" (escaped/encoded). Another character that causes issue is a slash in the filename itself, even if escaped/encoded.
          Hide
          Neelesh Srinivas Salian added a comment -

          Harsh J has this been resolved or is it in progress?
          We should get this in to avoid confusion.

          I would like to work on it if possible.

          Thank you.

          Show
          Neelesh Srinivas Salian added a comment - Harsh J has this been resolved or is it in progress? We should get this in to avoid confusion. I would like to work on it if possible. Thank you.
          Hide
          Hadoop QA added a comment -



          -1 overall



          Vote Subsystem Runtime Comment
          -1 patch 0m 0s The patch command could not apply the patch during dryrun.



          Subsystem Report/Notes
          Patch URL http://issues.apache.org/jira/secure/attachment/12509018/HADOOP-7945.patch
          Optional Tests javadoc javac unit findbugs checkstyle
          git revision trunk / 5db371f
          Console output https://builds.apache.org/job/PreCommit-HADOOP-Build/7749/console

          This message was automatically generated.

          Show
          Hadoop QA added a comment - -1 overall Vote Subsystem Runtime Comment -1 patch 0m 0s The patch command could not apply the patch during dryrun. Subsystem Report/Notes Patch URL http://issues.apache.org/jira/secure/attachment/12509018/HADOOP-7945.patch Optional Tests javadoc javac unit findbugs checkstyle git revision trunk / 5db371f Console output https://builds.apache.org/job/PreCommit-HADOOP-Build/7749/console This message was automatically generated.
          Hide
          Hadoop QA added a comment -
          -1 overall



          Vote Subsystem Runtime Comment
          0 reexec 0m 0s Docker mode activated.
          -1 patch 0m 4s HADOOP-7945 does not apply to trunk. Rebase required? Wrong Branch? See https://wiki.apache.org/hadoop/HowToContribute for help.



          Subsystem Report/Notes
          JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12509018/HADOOP-7945.patch
          JIRA Issue HADOOP-7945
          Console output https://builds.apache.org/job/PreCommit-HADOOP-Build/10014/console
          Powered by Apache Yetus 0.4.0-SNAPSHOT http://yetus.apache.org

          This message was automatically generated.

          Show
          Hadoop QA added a comment - -1 overall Vote Subsystem Runtime Comment 0 reexec 0m 0s Docker mode activated. -1 patch 0m 4s HADOOP-7945 does not apply to trunk. Rebase required? Wrong Branch? See https://wiki.apache.org/hadoop/HowToContribute for help. Subsystem Report/Notes JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12509018/HADOOP-7945.patch JIRA Issue HADOOP-7945 Console output https://builds.apache.org/job/PreCommit-HADOOP-Build/10014/console Powered by Apache Yetus 0.4.0-SNAPSHOT http://yetus.apache.org This message was automatically generated.
          Hide
          Vinod Kumar Vavilapalli added a comment -

          No update on this very old documentation patch. Removing target-versions. Let's add it back to the next release when a patch becomes ready.

          Show
          Vinod Kumar Vavilapalli added a comment - No update on this very old documentation patch. Removing target-versions. Let's add it back to the next release when a patch becomes ready.

            People

            • Assignee:
              Unassigned
              Reporter:
              Harsh J
            • Votes:
              2 Vote for this issue
              Watchers:
              8 Start watching this issue

              Dates

              • Created:
                Updated:

                Development