Uploaded image for project: 'Hadoop YARN'
  1. Hadoop YARN
  2. YARN-1728

Workaround guice3x-undecoded pathInfo in YARN WebApp

    Details

    • Type: Bug
    • Status: Resolved
    • Priority: Major
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 2.8.0, 2.7.4, 3.0.0-alpha4
    • Component/s: None
    • Labels:
      None

      Description

      For example, going to the job history server page http://localhost:19888/jobhistory/logs/localhost%3A8041/container_1391466602060_0011_01_000001/job_1391466602060_0011/admin/stderr results in the following error:

      Cannot get container logs. Invalid nodeId: test-cdh5-hue.ent.cloudera.com%3A8041
      

      Where the url decoded version works:
      http://localhost:19888/jobhistory/logs/localhost:8041/container_1391466602060_0011_01_000001/job_1391466602060_0011/admin/stderr

      It seems like both should be supported as the former is simply percent encoding.

      1. test-case-for-trunk.patch
        2 kB
        Yuanbo Liu
      2. YARN-1728-branch-2.001.patch
        2 kB
        Yuanbo Liu
      3. YARN-1728-branch-2.002.patch
        3 kB
        Yuanbo Liu
      4. YARN-1728-branch-2.003.patch
        3 kB
        Yuanbo Liu
      5. YARN-1728-branch-2.004.patch
        3 kB
        Yuanbo Liu
      6. YARN-1728-branch-2.005.patch
        3 kB
        Yuanbo Liu

        Issue Links

          Activity

          Hide
          jira.shegalov Gera Shegalov added a comment -

          This must have something to do with how Jetty web container interprets percent-encoding RFC. The node id is resolved out of getPathInfo, which does the decoding. Section 2.4 says

          "Implementations must not
          percent-encode or decode the same string more than once, as decoding
          an already decoded string might lead to misinterpreting a percent
          data octet as the beginning of a percent-encoding, or vice versa in
          the case of percent-encoding an already percent-encoded string."

          Thus it would be incorrect to attempt decoding once again. On the other hand, since ':' has no special meaning for the path component unlike '/', Jetty might have chosen not to interpret %3A .

          Show
          jira.shegalov Gera Shegalov added a comment - This must have something to do with how Jetty web container interprets percent-encoding RFC. The node id is resolved out of getPathInfo , which does the decoding. Section 2.4 says "Implementations must not percent-encode or decode the same string more than once, as decoding an already decoded string might lead to misinterpreting a percent data octet as the beginning of a percent-encoding, or vice versa in the case of percent-encoding an already percent-encoded string." Thus it would be incorrect to attempt decoding once again. On the other hand, since ':' has no special meaning for the path component unlike '/', Jetty might have chosen not to interpret %3A .
          Hide
          jayunit100 jay vyas added a comment -

          Possble link to MAPREDUCE-5902, not sure exactly how this would pop up in two places, but it seems almost the exact same problem, just on the DFS side instead of on the web side.

          Show
          jayunit100 jay vyas added a comment - Possble link to MAPREDUCE-5902 , not sure exactly how this would pop up in two places, but it seems almost the exact same problem, just on the DFS side instead of on the web side.
          Hide
          yuanbo Yuanbo Liu added a comment -

          Would like to take it over, and see why the path is not decoded. We've met such kind of defect in hadoop-2.7.3.

          Show
          yuanbo Yuanbo Liu added a comment - Would like to take it over, and see why the path is not decoded. We've met such kind of defect in hadoop-2.7.3.
          Hide
          yuanbo Yuanbo Liu added a comment -

          The root cause of this defect is that the third-party jar named guice-3.0 doesn't obey the rule which is introduced by Gera Shegalov
          With HADOOP-12064, this defect will never be a problem in trunk and hadoop-3, but in branch-2, it still exists.
          I strongly suggest that this defect should be addressed in hadoop-2 because encoded path in url is quite often and the implemented method in guice-3.0 doesn't follow the contract of the interface HttpServletRequest#getPathInfo.

          Show
          yuanbo Yuanbo Liu added a comment - The root cause of this defect is that the third-party jar named guice-3.0 doesn't obey the rule which is introduced by Gera Shegalov With HADOOP-12064 , this defect will never be a problem in trunk and hadoop-3, but in branch-2, it still exists. I strongly suggest that this defect should be addressed in hadoop-2 because encoded path in url is quite often and the implemented method in guice-3.0 doesn't follow the contract of the interface HttpServletRequest#getPathInfo .
          Hide
          yuanbo Yuanbo Liu added a comment -

          Gera Shegalov / Robert Kanter
          Would you mind having a look at my patch and give some thoughts. Thanks in advance!

          Show
          yuanbo Yuanbo Liu added a comment - Gera Shegalov / Robert Kanter Would you mind having a look at my patch and give some thoughts. Thanks in advance!
          Hide
          hadoopqa Hadoop QA added a comment -
          +1 overall



          Vote Subsystem Runtime Comment
          0 reexec 0m 18s Docker mode activated.
          +1 @author 0m 0s The patch does not contain any @author tags.
          +1 test4tests 0m 0s The patch appears to include 1 new or modified test files.
          +1 mvninstall 10m 9s branch-2 passed
          +1 compile 0m 36s branch-2 passed with JDK v1.8.0_121
          +1 compile 0m 32s branch-2 passed with JDK v1.7.0_121
          +1 checkstyle 0m 23s branch-2 passed
          +1 mvnsite 0m 40s branch-2 passed
          +1 mvneclipse 0m 16s branch-2 passed
          +1 findbugs 1m 25s branch-2 passed
          +1 javadoc 0m 36s branch-2 passed with JDK v1.8.0_121
          +1 javadoc 0m 35s branch-2 passed with JDK v1.7.0_121
          +1 mvninstall 0m 31s the patch passed
          +1 compile 0m 32s the patch passed with JDK v1.8.0_121
          +1 javac 0m 32s the patch passed
          +1 compile 0m 28s the patch passed with JDK v1.7.0_121
          +1 javac 0m 28s the patch passed
          +1 checkstyle 0m 20s the patch passed
          +1 mvnsite 0m 34s the patch passed
          +1 mvneclipse 0m 12s the patch passed
          +1 whitespace 0m 0s The patch has no whitespace issues.
          +1 findbugs 1m 26s the patch passed
          +1 javadoc 0m 27s the patch passed with JDK v1.8.0_121
          +1 javadoc 0m 32s the patch passed with JDK v1.7.0_121
          +1 unit 2m 34s hadoop-yarn-common in the patch passed with JDK v1.7.0_121.
          +1 asflicense 0m 20s The patch does not generate ASF License warnings.
          27m 43s



          Subsystem Report/Notes
          Docker Image:yetus/hadoop:b59b8b7
          JIRA Issue YARN-1728
          JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12853238/YARN-1728-branch-2.001.patch
          Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle
          uname Linux 934090df6616 3.13.0-105-generic #152-Ubuntu SMP Fri Dec 2 15:37:11 UTC 2016 x86_64 x86_64 x86_64 GNU/Linux
          Build tool maven
          Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh
          git revision branch-2 / 06386b7
          Default Java 1.7.0_121
          Multi-JDK versions /usr/lib/jvm/java-8-oracle:1.8.0_121 /usr/lib/jvm/java-7-openjdk-amd64:1.7.0_121
          findbugs v3.0.0
          JDK v1.7.0_121 Test Results https://builds.apache.org/job/PreCommit-YARN-Build/14999/testReport/
          modules C: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common U: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common
          Console output https://builds.apache.org/job/PreCommit-YARN-Build/14999/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 18s Docker mode activated. +1 @author 0m 0s The patch does not contain any @author tags. +1 test4tests 0m 0s The patch appears to include 1 new or modified test files. +1 mvninstall 10m 9s branch-2 passed +1 compile 0m 36s branch-2 passed with JDK v1.8.0_121 +1 compile 0m 32s branch-2 passed with JDK v1.7.0_121 +1 checkstyle 0m 23s branch-2 passed +1 mvnsite 0m 40s branch-2 passed +1 mvneclipse 0m 16s branch-2 passed +1 findbugs 1m 25s branch-2 passed +1 javadoc 0m 36s branch-2 passed with JDK v1.8.0_121 +1 javadoc 0m 35s branch-2 passed with JDK v1.7.0_121 +1 mvninstall 0m 31s the patch passed +1 compile 0m 32s the patch passed with JDK v1.8.0_121 +1 javac 0m 32s the patch passed +1 compile 0m 28s the patch passed with JDK v1.7.0_121 +1 javac 0m 28s the patch passed +1 checkstyle 0m 20s the patch passed +1 mvnsite 0m 34s the patch passed +1 mvneclipse 0m 12s the patch passed +1 whitespace 0m 0s The patch has no whitespace issues. +1 findbugs 1m 26s the patch passed +1 javadoc 0m 27s the patch passed with JDK v1.8.0_121 +1 javadoc 0m 32s the patch passed with JDK v1.7.0_121 +1 unit 2m 34s hadoop-yarn-common in the patch passed with JDK v1.7.0_121. +1 asflicense 0m 20s The patch does not generate ASF License warnings. 27m 43s Subsystem Report/Notes Docker Image:yetus/hadoop:b59b8b7 JIRA Issue YARN-1728 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12853238/YARN-1728-branch-2.001.patch Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle uname Linux 934090df6616 3.13.0-105-generic #152-Ubuntu SMP Fri Dec 2 15:37:11 UTC 2016 x86_64 x86_64 x86_64 GNU/Linux Build tool maven Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh git revision branch-2 / 06386b7 Default Java 1.7.0_121 Multi-JDK versions /usr/lib/jvm/java-8-oracle:1.8.0_121 /usr/lib/jvm/java-7-openjdk-amd64:1.7.0_121 findbugs v3.0.0 JDK v1.7.0_121 Test Results https://builds.apache.org/job/PreCommit-YARN-Build/14999/testReport/ modules C: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common U: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common Console output https://builds.apache.org/job/PreCommit-YARN-Build/14999/console Powered by Apache Yetus 0.5.0-SNAPSHOT http://yetus.apache.org This message was automatically generated.
          Hide
          haibochen Haibo Chen added a comment -

          Thanks Yuanbo Liu for the patch! The patch looks good to me except for two knits
          1) bq. // Make sure path is decoded.
          This is confusing if we just look at javadoc in HttpServletRequest.getPathInfo() based on the method signature, because it already decodes the URL. Maybe we should explicitly mention Guice 3.0 here to make it clear
          2) The unit test is added to an existing test method for custom routes. I think we should create a new test method specifically for the decoding issue here.

          Show
          haibochen Haibo Chen added a comment - Thanks Yuanbo Liu for the patch! The patch looks good to me except for two knits 1) bq. // Make sure path is decoded. This is confusing if we just look at javadoc in HttpServletRequest.getPathInfo() based on the method signature, because it already decodes the URL. Maybe we should explicitly mention Guice 3.0 here to make it clear 2) The unit test is added to an existing test method for custom routes. I think we should create a new test method specifically for the decoding issue here.
          Hide
          yuanbo Yuanbo Liu added a comment -

          Haibo Chen Thanks for your review.
          Upload v2 patch to address your comments.

          Show
          yuanbo Yuanbo Liu added a comment - Haibo Chen Thanks for your review. Upload v2 patch to address your comments.
          Hide
          hadoopqa Hadoop QA added a comment -
          +1 overall



          Vote Subsystem Runtime Comment
          0 reexec 0m 23s Docker mode activated.
          +1 @author 0m 0s The patch does not contain any @author tags.
          +1 test4tests 0m 0s The patch appears to include 1 new or modified test files.
          +1 mvninstall 8m 35s branch-2 passed
          +1 compile 0m 35s branch-2 passed with JDK v1.8.0_121
          +1 compile 0m 34s branch-2 passed with JDK v1.7.0_121
          +1 checkstyle 0m 27s branch-2 passed
          +1 mvnsite 0m 39s branch-2 passed
          +1 mvneclipse 0m 17s branch-2 passed
          +1 findbugs 1m 27s branch-2 passed
          +1 javadoc 0m 37s branch-2 passed with JDK v1.8.0_121
          +1 javadoc 0m 41s branch-2 passed with JDK v1.7.0_121
          +1 mvninstall 0m 34s the patch passed
          +1 compile 0m 30s the patch passed with JDK v1.8.0_121
          +1 javac 0m 30s the patch passed
          +1 compile 0m 34s the patch passed with JDK v1.7.0_121
          +1 javac 0m 34s the patch passed
          +1 checkstyle 0m 24s the patch passed
          +1 mvnsite 0m 40s the patch passed
          +1 mvneclipse 0m 14s the patch passed
          +1 whitespace 0m 0s The patch has no whitespace issues.
          +1 findbugs 1m 37s the patch passed
          +1 javadoc 0m 29s the patch passed with JDK v1.8.0_121
          +1 javadoc 0m 31s the patch passed with JDK v1.7.0_121
          +1 unit 2m 39s hadoop-yarn-common in the patch passed with JDK v1.7.0_121.
          +1 asflicense 0m 19s The patch does not generate ASF License warnings.
          27m 5s



          Subsystem Report/Notes
          Docker Image:yetus/hadoop:b59b8b7
          JIRA Issue YARN-1728
          JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12854145/YARN-1728-branch-2.002.patch
          Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle
          uname Linux 569187d5ceab 3.13.0-105-generic #152-Ubuntu SMP Fri Dec 2 15:37:11 UTC 2016 x86_64 x86_64 x86_64 GNU/Linux
          Build tool maven
          Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh
          git revision branch-2 / 37edbd3
          Default Java 1.7.0_121
          Multi-JDK versions /usr/lib/jvm/java-8-oracle:1.8.0_121 /usr/lib/jvm/java-7-openjdk-amd64:1.7.0_121
          findbugs v3.0.0
          JDK v1.7.0_121 Test Results https://builds.apache.org/job/PreCommit-YARN-Build/15056/testReport/
          modules C: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common U: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common
          Console output https://builds.apache.org/job/PreCommit-YARN-Build/15056/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 23s Docker mode activated. +1 @author 0m 0s The patch does not contain any @author tags. +1 test4tests 0m 0s The patch appears to include 1 new or modified test files. +1 mvninstall 8m 35s branch-2 passed +1 compile 0m 35s branch-2 passed with JDK v1.8.0_121 +1 compile 0m 34s branch-2 passed with JDK v1.7.0_121 +1 checkstyle 0m 27s branch-2 passed +1 mvnsite 0m 39s branch-2 passed +1 mvneclipse 0m 17s branch-2 passed +1 findbugs 1m 27s branch-2 passed +1 javadoc 0m 37s branch-2 passed with JDK v1.8.0_121 +1 javadoc 0m 41s branch-2 passed with JDK v1.7.0_121 +1 mvninstall 0m 34s the patch passed +1 compile 0m 30s the patch passed with JDK v1.8.0_121 +1 javac 0m 30s the patch passed +1 compile 0m 34s the patch passed with JDK v1.7.0_121 +1 javac 0m 34s the patch passed +1 checkstyle 0m 24s the patch passed +1 mvnsite 0m 40s the patch passed +1 mvneclipse 0m 14s the patch passed +1 whitespace 0m 0s The patch has no whitespace issues. +1 findbugs 1m 37s the patch passed +1 javadoc 0m 29s the patch passed with JDK v1.8.0_121 +1 javadoc 0m 31s the patch passed with JDK v1.7.0_121 +1 unit 2m 39s hadoop-yarn-common in the patch passed with JDK v1.7.0_121. +1 asflicense 0m 19s The patch does not generate ASF License warnings. 27m 5s Subsystem Report/Notes Docker Image:yetus/hadoop:b59b8b7 JIRA Issue YARN-1728 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12854145/YARN-1728-branch-2.002.patch Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle uname Linux 569187d5ceab 3.13.0-105-generic #152-Ubuntu SMP Fri Dec 2 15:37:11 UTC 2016 x86_64 x86_64 x86_64 GNU/Linux Build tool maven Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh git revision branch-2 / 37edbd3 Default Java 1.7.0_121 Multi-JDK versions /usr/lib/jvm/java-8-oracle:1.8.0_121 /usr/lib/jvm/java-7-openjdk-amd64:1.7.0_121 findbugs v3.0.0 JDK v1.7.0_121 Test Results https://builds.apache.org/job/PreCommit-YARN-Build/15056/testReport/ modules C: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common U: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common Console output https://builds.apache.org/job/PreCommit-YARN-Build/15056/console Powered by Apache Yetus 0.5.0-SNAPSHOT http://yetus.apache.org This message was automatically generated.
          Hide
          haibochen Haibo Chen added a comment -

          Thanks Yuanbo Liu for your update! A few more nits.
          1) bq. // Guice-3.0 doesn't support encoded path info,
          Support seems a little vague. We can be more concrete by saying like Guice-3.0 does not decode paths that are encoded
          2) Can we rename testEncodedText to testEncodedUrl?
          3) app.stop() is called in the final block in all other test methods, we should follow that practice as well.

          Show
          haibochen Haibo Chen added a comment - Thanks Yuanbo Liu for your update! A few more nits. 1) bq. // Guice-3.0 doesn't support encoded path info, Support seems a little vague. We can be more concrete by saying like Guice-3.0 does not decode paths that are encoded 2) Can we rename testEncodedText to testEncodedUrl? 3) app.stop() is called in the final block in all other test methods, we should follow that practice as well.
          Hide
          yuanbo Yuanbo Liu added a comment -

          Haibo Chen Thanks a lot for your comments.
          Upload v3 patch.

          Show
          yuanbo Yuanbo Liu added a comment - Haibo Chen Thanks a lot for your comments. Upload v3 patch.
          Hide
          hadoopqa Hadoop QA added a comment -
          +1 overall



          Vote Subsystem Runtime Comment
          0 reexec 0m 17s Docker mode activated.
          +1 @author 0m 0s The patch does not contain any @author tags.
          +1 test4tests 0m 0s The patch appears to include 1 new or modified test files.
          +1 mvninstall 6m 51s branch-2 passed
          +1 compile 0m 25s branch-2 passed with JDK v1.8.0_121
          +1 compile 0m 28s branch-2 passed with JDK v1.7.0_121
          +1 checkstyle 0m 21s branch-2 passed
          +1 mvnsite 0m 32s branch-2 passed
          +1 mvneclipse 0m 15s branch-2 passed
          +1 findbugs 1m 16s branch-2 passed
          +1 javadoc 0m 26s branch-2 passed with JDK v1.8.0_121
          +1 javadoc 0m 30s branch-2 passed with JDK v1.7.0_121
          +1 mvninstall 0m 27s the patch passed
          +1 compile 0m 23s the patch passed with JDK v1.8.0_121
          +1 javac 0m 23s the patch passed
          +1 compile 0m 28s the patch passed with JDK v1.7.0_121
          +1 javac 0m 28s the patch passed
          +1 checkstyle 0m 18s the patch passed
          +1 mvnsite 0m 32s the patch passed
          +1 mvneclipse 0m 13s the patch passed
          +1 whitespace 0m 0s The patch has no whitespace issues.
          +1 findbugs 1m 20s the patch passed
          +1 javadoc 0m 25s the patch passed with JDK v1.8.0_121
          +1 javadoc 0m 28s the patch passed with JDK v1.7.0_121
          +1 unit 2m 30s hadoop-yarn-common in the patch passed with JDK v1.7.0_121.
          +1 asflicense 0m 18s The patch does not generate ASF License warnings.
          22m 44s



          Subsystem Report/Notes
          Docker Image:yetus/hadoop:b59b8b7
          JIRA Issue YARN-1728
          JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12854366/YARN-1728-branch-2.003.patch
          Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle
          uname Linux aac500b023e2 3.13.0-103-generic #150-Ubuntu SMP Thu Nov 24 10:34:17 UTC 2016 x86_64 x86_64 x86_64 GNU/Linux
          Build tool maven
          Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh
          git revision branch-2 / 40bc9e7
          Default Java 1.7.0_121
          Multi-JDK versions /usr/lib/jvm/java-8-oracle:1.8.0_121 /usr/lib/jvm/java-7-openjdk-amd64:1.7.0_121
          findbugs v3.0.0
          JDK v1.7.0_121 Test Results https://builds.apache.org/job/PreCommit-YARN-Build/15073/testReport/
          modules C: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common U: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common
          Console output https://builds.apache.org/job/PreCommit-YARN-Build/15073/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 17s Docker mode activated. +1 @author 0m 0s The patch does not contain any @author tags. +1 test4tests 0m 0s The patch appears to include 1 new or modified test files. +1 mvninstall 6m 51s branch-2 passed +1 compile 0m 25s branch-2 passed with JDK v1.8.0_121 +1 compile 0m 28s branch-2 passed with JDK v1.7.0_121 +1 checkstyle 0m 21s branch-2 passed +1 mvnsite 0m 32s branch-2 passed +1 mvneclipse 0m 15s branch-2 passed +1 findbugs 1m 16s branch-2 passed +1 javadoc 0m 26s branch-2 passed with JDK v1.8.0_121 +1 javadoc 0m 30s branch-2 passed with JDK v1.7.0_121 +1 mvninstall 0m 27s the patch passed +1 compile 0m 23s the patch passed with JDK v1.8.0_121 +1 javac 0m 23s the patch passed +1 compile 0m 28s the patch passed with JDK v1.7.0_121 +1 javac 0m 28s the patch passed +1 checkstyle 0m 18s the patch passed +1 mvnsite 0m 32s the patch passed +1 mvneclipse 0m 13s the patch passed +1 whitespace 0m 0s The patch has no whitespace issues. +1 findbugs 1m 20s the patch passed +1 javadoc 0m 25s the patch passed with JDK v1.8.0_121 +1 javadoc 0m 28s the patch passed with JDK v1.7.0_121 +1 unit 2m 30s hadoop-yarn-common in the patch passed with JDK v1.7.0_121. +1 asflicense 0m 18s The patch does not generate ASF License warnings. 22m 44s Subsystem Report/Notes Docker Image:yetus/hadoop:b59b8b7 JIRA Issue YARN-1728 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12854366/YARN-1728-branch-2.003.patch Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle uname Linux aac500b023e2 3.13.0-103-generic #150-Ubuntu SMP Thu Nov 24 10:34:17 UTC 2016 x86_64 x86_64 x86_64 GNU/Linux Build tool maven Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh git revision branch-2 / 40bc9e7 Default Java 1.7.0_121 Multi-JDK versions /usr/lib/jvm/java-8-oracle:1.8.0_121 /usr/lib/jvm/java-7-openjdk-amd64:1.7.0_121 findbugs v3.0.0 JDK v1.7.0_121 Test Results https://builds.apache.org/job/PreCommit-YARN-Build/15073/testReport/ modules C: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common U: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common Console output https://builds.apache.org/job/PreCommit-YARN-Build/15073/console Powered by Apache Yetus 0.5.0-SNAPSHOT http://yetus.apache.org This message was automatically generated.
          Hide
          haibochen Haibo Chen added a comment -

          Latest patch looks good to me. non-binding +1

          Show
          haibochen Haibo Chen added a comment - Latest patch looks good to me. non-binding +1
          Hide
          jira.shegalov Gera Shegalov added a comment -

          Hi Yuanbo Liu, thanks for addressing the issue. I see that Guice itself fixed it using java.net.URI#getPath. Let us use it here so the behavior is consistent with newer Guice.

          I suggest we use:

          decodedPathInfo = URI.create(pathInfo).getPath();
          
          Show
          jira.shegalov Gera Shegalov added a comment - Hi Yuanbo Liu , thanks for addressing the issue. I see that Guice itself fixed it using java.net.URI#getPath . Let us use it here so the behavior is consistent with newer Guice. I suggest we use: decodedPathInfo = URI.create(pathInfo).getPath();
          Hide
          yuanbo Yuanbo Liu added a comment -

          Gera Shegalov Thanks for your comments.
          Agree with you. since the patch of guice uses pathInfo = new URI(pathInfo).getPath() with try.. catch expression, and URI.create will raise a run-time exception, I use new URI instead of URI.create for compatibility. Upload v4 patch for this JIRA, please review it.

          Show
          yuanbo Yuanbo Liu added a comment - Gera Shegalov Thanks for your comments. Agree with you. since the patch of guice uses pathInfo = new URI(pathInfo).getPath() with try.. catch expression, and URI.create will raise a run-time exception, I use new URI instead of URI.create for compatibility. Upload v4 patch for this JIRA, please review it.
          Hide
          hadoopqa Hadoop QA added a comment -
          +1 overall



          Vote Subsystem Runtime Comment
          0 reexec 0m 22s Docker mode activated.
          +1 @author 0m 0s The patch does not contain any @author tags.
          +1 test4tests 0m 0s The patch appears to include 1 new or modified test files.
          +1 mvninstall 6m 50s branch-2 passed
          +1 compile 0m 25s branch-2 passed with JDK v1.8.0_121
          +1 compile 0m 28s branch-2 passed with JDK v1.7.0_121
          +1 checkstyle 0m 20s branch-2 passed
          +1 mvnsite 0m 32s branch-2 passed
          +1 mvneclipse 0m 13s branch-2 passed
          +1 findbugs 1m 9s branch-2 passed
          +1 javadoc 0m 26s branch-2 passed with JDK v1.8.0_121
          +1 javadoc 0m 31s branch-2 passed with JDK v1.7.0_121
          +1 mvninstall 0m 27s the patch passed
          +1 compile 0m 21s the patch passed with JDK v1.8.0_121
          +1 javac 0m 21s the patch passed
          +1 compile 0m 26s the patch passed with JDK v1.7.0_121
          +1 javac 0m 26s the patch passed
          +1 checkstyle 0m 17s hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common: The patch generated 0 new + 8 unchanged - 1 fixed = 8 total (was 9)
          +1 mvnsite 0m 29s the patch passed
          +1 mvneclipse 0m 11s the patch passed
          +1 whitespace 0m 0s The patch has no whitespace issues.
          +1 findbugs 1m 18s the patch passed
          +1 javadoc 0m 24s the patch passed with JDK v1.8.0_121
          +1 javadoc 0m 29s the patch passed with JDK v1.7.0_121
          +1 unit 2m 26s hadoop-yarn-common in the patch passed with JDK v1.7.0_121.
          +1 asflicense 0m 18s The patch does not generate ASF License warnings.
          22m 30s



          Subsystem Report/Notes
          Docker Image:yetus/hadoop:b59b8b7
          JIRA Issue YARN-1728
          JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12854808/YARN-1728-branch-2.004.patch
          Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle
          uname Linux 6da7d575e8b5 3.13.0-106-generic #153-Ubuntu SMP Tue Dec 6 15:44:32 UTC 2016 x86_64 x86_64 x86_64 GNU/Linux
          Build tool maven
          Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh
          git revision branch-2 / f3cdf29
          Default Java 1.7.0_121
          Multi-JDK versions /usr/lib/jvm/java-8-oracle:1.8.0_121 /usr/lib/jvm/java-7-openjdk-amd64:1.7.0_121
          findbugs v3.0.0
          JDK v1.7.0_121 Test Results https://builds.apache.org/job/PreCommit-YARN-Build/15090/testReport/
          modules C: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common U: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common
          Console output https://builds.apache.org/job/PreCommit-YARN-Build/15090/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 22s Docker mode activated. +1 @author 0m 0s The patch does not contain any @author tags. +1 test4tests 0m 0s The patch appears to include 1 new or modified test files. +1 mvninstall 6m 50s branch-2 passed +1 compile 0m 25s branch-2 passed with JDK v1.8.0_121 +1 compile 0m 28s branch-2 passed with JDK v1.7.0_121 +1 checkstyle 0m 20s branch-2 passed +1 mvnsite 0m 32s branch-2 passed +1 mvneclipse 0m 13s branch-2 passed +1 findbugs 1m 9s branch-2 passed +1 javadoc 0m 26s branch-2 passed with JDK v1.8.0_121 +1 javadoc 0m 31s branch-2 passed with JDK v1.7.0_121 +1 mvninstall 0m 27s the patch passed +1 compile 0m 21s the patch passed with JDK v1.8.0_121 +1 javac 0m 21s the patch passed +1 compile 0m 26s the patch passed with JDK v1.7.0_121 +1 javac 0m 26s the patch passed +1 checkstyle 0m 17s hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common: The patch generated 0 new + 8 unchanged - 1 fixed = 8 total (was 9) +1 mvnsite 0m 29s the patch passed +1 mvneclipse 0m 11s the patch passed +1 whitespace 0m 0s The patch has no whitespace issues. +1 findbugs 1m 18s the patch passed +1 javadoc 0m 24s the patch passed with JDK v1.8.0_121 +1 javadoc 0m 29s the patch passed with JDK v1.7.0_121 +1 unit 2m 26s hadoop-yarn-common in the patch passed with JDK v1.7.0_121. +1 asflicense 0m 18s The patch does not generate ASF License warnings. 22m 30s Subsystem Report/Notes Docker Image:yetus/hadoop:b59b8b7 JIRA Issue YARN-1728 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12854808/YARN-1728-branch-2.004.patch Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle uname Linux 6da7d575e8b5 3.13.0-106-generic #153-Ubuntu SMP Tue Dec 6 15:44:32 UTC 2016 x86_64 x86_64 x86_64 GNU/Linux Build tool maven Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh git revision branch-2 / f3cdf29 Default Java 1.7.0_121 Multi-JDK versions /usr/lib/jvm/java-8-oracle:1.8.0_121 /usr/lib/jvm/java-7-openjdk-amd64:1.7.0_121 findbugs v3.0.0 JDK v1.7.0_121 Test Results https://builds.apache.org/job/PreCommit-YARN-Build/15090/testReport/ modules C: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common U: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common Console output https://builds.apache.org/job/PreCommit-YARN-Build/15090/console Powered by Apache Yetus 0.5.0-SNAPSHOT http://yetus.apache.org This message was automatically generated.
          Hide
          jira.shegalov Gera Shegalov added a comment -

          Yuanbo Liu, thanks for the latest patch. I suggested URI.create because we are guaranteed to get a valid pathInfo from the servlet container but it's indeed good to be defensive since we are already dealing with a servlet bug.

          I am generally +1

          In trunk the issue is fixed thanks to guice 4.0/HADOOP-12064 cc: Tsuyoshi Ozawa. And as the quote from the spec says we must not decode twice. Therefore I suggest we split this patch. The test-only patch should go into both trunk and branch-2 such that we catch the issue in all releases. The actual fix should go in branch-2.

          Show
          jira.shegalov Gera Shegalov added a comment - Yuanbo Liu , thanks for the latest patch. I suggested URI.create because we are guaranteed to get a valid pathInfo from the servlet container but it's indeed good to be defensive since we are already dealing with a servlet bug. I am generally +1 In trunk the issue is fixed thanks to guice 4.0/ HADOOP-12064 cc: Tsuyoshi Ozawa . And as the quote from the spec says we must not decode twice. Therefore I suggest we split this patch. The test-only patch should go into both trunk and branch-2 such that we catch the issue in all releases. The actual fix should go in branch-2.
          Hide
          jira.shegalov Gera Shegalov added a comment -

          Minor thing, since we have this catch clause, can we add the pathInfo value and stack trace to the log message:

          }  catch (URISyntaxException ex) {
            LOG.error(pathInfo + ": Failed to decode path.", ex);
          }
          
          Show
          jira.shegalov Gera Shegalov added a comment - Minor thing, since we have this catch clause, can we add the pathInfo value and stack trace to the log message: } catch (URISyntaxException ex) { LOG.error(pathInfo + ": Failed to decode path." , ex); }
          Hide
          yuanbo Yuanbo Liu added a comment -

          Gera Shegalov Thanks for your comments.
          Upload v5 patch and test case for trunk to address your comments.

          Show
          yuanbo Yuanbo Liu added a comment - Gera Shegalov Thanks for your comments. Upload v5 patch and test case for trunk to address your comments.
          Hide
          hadoopqa Hadoop QA added a comment -
          +1 overall



          Vote Subsystem Runtime Comment
          0 reexec 0m 17s Docker mode activated.
          +1 @author 0m 0s The patch does not contain any @author tags.
          +1 test4tests 0m 0s The patch appears to include 1 new or modified test files.
          +1 mvninstall 12m 54s trunk passed
          +1 compile 0m 29s trunk passed
          +1 checkstyle 0m 24s trunk passed
          +1 mvnsite 0m 36s trunk passed
          +1 mvneclipse 0m 14s trunk passed
          +1 findbugs 1m 9s trunk passed
          +1 javadoc 0m 33s trunk passed
          +1 mvninstall 0m 35s the patch passed
          +1 compile 0m 33s the patch passed
          +1 javac 0m 33s the patch passed
          +1 checkstyle 0m 21s the patch passed
          +1 mvnsite 0m 35s the patch passed
          +1 mvneclipse 0m 13s the patch passed
          +1 whitespace 0m 0s The patch has no whitespace issues.
          +1 findbugs 1m 21s the patch passed
          +1 javadoc 0m 30s the patch passed
          +1 unit 2m 42s hadoop-yarn-common in the patch passed.
          +1 asflicense 0m 19s The patch does not generate ASF License warnings.
          25m 12s



          Subsystem Report/Notes
          Docker Image:yetus/hadoop:a9ad5d6
          JIRA Issue YARN-1728
          JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12855045/test-case-for-trunk.patch
          Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle
          uname Linux f71806ba0f31 3.13.0-106-generic #153-Ubuntu SMP Tue Dec 6 15:44:32 UTC 2016 x86_64 x86_64 x86_64 GNU/Linux
          Build tool maven
          Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh
          git revision trunk / 55c07bb
          Default Java 1.8.0_121
          findbugs v3.0.0
          Test Results https://builds.apache.org/job/PreCommit-YARN-Build/15101/testReport/
          modules C: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common U: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common
          Console output https://builds.apache.org/job/PreCommit-YARN-Build/15101/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 17s Docker mode activated. +1 @author 0m 0s The patch does not contain any @author tags. +1 test4tests 0m 0s The patch appears to include 1 new or modified test files. +1 mvninstall 12m 54s trunk passed +1 compile 0m 29s trunk passed +1 checkstyle 0m 24s trunk passed +1 mvnsite 0m 36s trunk passed +1 mvneclipse 0m 14s trunk passed +1 findbugs 1m 9s trunk passed +1 javadoc 0m 33s trunk passed +1 mvninstall 0m 35s the patch passed +1 compile 0m 33s the patch passed +1 javac 0m 33s the patch passed +1 checkstyle 0m 21s the patch passed +1 mvnsite 0m 35s the patch passed +1 mvneclipse 0m 13s the patch passed +1 whitespace 0m 0s The patch has no whitespace issues. +1 findbugs 1m 21s the patch passed +1 javadoc 0m 30s the patch passed +1 unit 2m 42s hadoop-yarn-common in the patch passed. +1 asflicense 0m 19s The patch does not generate ASF License warnings. 25m 12s Subsystem Report/Notes Docker Image:yetus/hadoop:a9ad5d6 JIRA Issue YARN-1728 JIRA Patch URL https://issues.apache.org/jira/secure/attachment/12855045/test-case-for-trunk.patch Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle uname Linux f71806ba0f31 3.13.0-106-generic #153-Ubuntu SMP Tue Dec 6 15:44:32 UTC 2016 x86_64 x86_64 x86_64 GNU/Linux Build tool maven Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh git revision trunk / 55c07bb Default Java 1.8.0_121 findbugs v3.0.0 Test Results https://builds.apache.org/job/PreCommit-YARN-Build/15101/testReport/ modules C: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common U: hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common Console output https://builds.apache.org/job/PreCommit-YARN-Build/15101/console Powered by Apache Yetus 0.5.0-SNAPSHOT http://yetus.apache.org This message was automatically generated.
          Hide
          jira.shegalov Gera Shegalov added a comment -

          +1m committing

          Show
          jira.shegalov Gera Shegalov added a comment - +1m committing
          Hide
          hudson Hudson added a comment -

          SUCCESS: Integrated in Jenkins build Hadoop-trunk-Commit #11320 (See https://builds.apache.org/job/Hadoop-trunk-Commit/11320/)
          YARN-1728. Regression test for guice-undecoded pathInfo in YARN WebApp. (gera: rev 480b4dd574d0355bf6c976a38bb45cb86adb2714)

          • (edit) hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common/src/test/java/org/apache/hadoop/yarn/webapp/TestWebApp.java
          Show
          hudson Hudson added a comment - SUCCESS: Integrated in Jenkins build Hadoop-trunk-Commit #11320 (See https://builds.apache.org/job/Hadoop-trunk-Commit/11320/ ) YARN-1728 . Regression test for guice-undecoded pathInfo in YARN WebApp. (gera: rev 480b4dd574d0355bf6c976a38bb45cb86adb2714) (edit) hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common/src/test/java/org/apache/hadoop/yarn/webapp/TestWebApp.java
          Hide
          jira.shegalov Gera Shegalov added a comment -

          Thank you Yuanbo Liu for your contribution. Thank you Haibo Chen for additional review. Committed test to trunk, and the full patch YARN-1728-branch-2.005 to branch-2 and branch-2.7.

          Show
          jira.shegalov Gera Shegalov added a comment - Thank you Yuanbo Liu for your contribution. Thank you Haibo Chen for additional review. Committed test to trunk, and the full patch YARN-1728 -branch-2.005 to branch-2 and branch-2.7.
          Hide
          yuanbo Yuanbo Liu added a comment -

          Thanks a lot!

          Show
          yuanbo Yuanbo Liu added a comment - Thanks a lot!

            People

            • Assignee:
              yuanbo Yuanbo Liu
              Reporter:
              abec Abraham Elmahrek
            • Votes:
              0 Vote for this issue
              Watchers:
              14 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development