Uploaded image for project: 'Hadoop Common'
  1. Hadoop Common
  2. HADOOP-7945

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

    Details

    • Type: Improvement
    • Status: Patch Available
    • Priority: 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
          qwertymaniac 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
          qwertymaniac 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
          nielsbasjes 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
          nielsbasjes 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
          qwertymaniac 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
          qwertymaniac 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
          nielsbasjes 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
          nielsbasjes 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
          nielsbasjes 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
          nielsbasjes 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
          qwertymaniac 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
          qwertymaniac 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
          nielsbasjes 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
          nielsbasjes 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
          qwertymaniac 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
          qwertymaniac 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
          neelesh77 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
          neelesh77 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
          hadoopqa 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
          hadoopqa 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
          hadoopqa 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
          hadoopqa 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
          vinodkv 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
          vinodkv 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.
          Hide
          poeppt Thomas Poepping added a comment -

          Do we have any plan to push this through? It seems like there's inconsistencies throughout the entire Hadoop ecosystem. Hadoop and HDFS should define what's allowed and what isn't. If that's following URI syntax as per the original RFC, then so be it.

          Show
          poeppt Thomas Poepping added a comment - Do we have any plan to push this through? It seems like there's inconsistencies throughout the entire Hadoop ecosystem. Hadoop and HDFS should define what's allowed and what isn't. If that's following URI syntax as per the original RFC, then so be it.
          Hide
          hadoopqa Hadoop QA added a comment -
          -1 overall



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



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

          This message was automatically generated.

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

            People

            • Assignee:
              Unassigned
              Reporter:
              qwertymaniac Harsh J
            • Votes:
              3 Vote for this issue
              Watchers:
              9 Start watching this issue

              Dates

              • Created:
                Updated:

                Development