Details

    • Type: Bug
    • Status: Closed
    • Priority: Major
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 2.8.0, 2.7.3, 2.6.5, 3.0.0-alpha1
    • Component/s: None
    • Labels:
      None
    • Hadoop Flags:
      Reviewed

      Description

      The Shell class makes assumptions that the parameters won't have spaces or other special characters, even when it invokes bash.

      1. HADOOP-13434.patch
        13 kB
        Owen O'Malley
      2. HADOOP-13434.patch
        13 kB
        Owen O'Malley
      3. HADOOP-13434.patch
        13 kB
        Owen O'Malley
      4. HADOOP-13434-branch-2.7.01.patch
        5 kB
        Arpit Agarwal

        Issue Links

          Activity

          Hide
          owen.omalley Owen O'Malley added a comment -

          This patch adds quoting to the Shell class on each command that invokes bash with string parameters.

          Show
          owen.omalley Owen O'Malley added a comment - This patch adds quoting to the Shell class on each command that invokes bash with string parameters.
          Hide
          githubbot ASF GitHub Bot added a comment -

          GitHub user omalley opened a pull request:

          https://github.com/apache/hadoop/pull/119

          HADOOP-13434. Add bash quoting to Shell class.

          You can merge this pull request into a Git repository by running:

          $ git pull https://github.com/omalley/hadoop hadoop-13434

          Alternatively you can review and apply these changes as the patch at:

          https://github.com/apache/hadoop/pull/119.patch

          To close this pull request, make a commit to your master/trunk branch
          with (at least) the following in the commit message:

          This closes #119


          commit 1ba826fcf3d8a0c94bf8880dd031f376cc0cd417
          Author: Owen O'Malley <omalley@apache.org>
          Date: 2016-07-28T18:26:21Z

          HADOOP-13434. Add bash quoting to Shell class.


          Show
          githubbot ASF GitHub Bot added a comment - GitHub user omalley opened a pull request: https://github.com/apache/hadoop/pull/119 HADOOP-13434 . Add bash quoting to Shell class. You can merge this pull request into a Git repository by running: $ git pull https://github.com/omalley/hadoop hadoop-13434 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/hadoop/pull/119.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #119 commit 1ba826fcf3d8a0c94bf8880dd031f376cc0cd417 Author: Owen O'Malley <omalley@apache.org> Date: 2016-07-28T18:26:21Z HADOOP-13434 . Add bash quoting to Shell class.
          Hide
          owen.omalley Owen O'Malley added a comment -

          This iteration removes the use of bash from the cases where it wasn't necessary.

          Show
          owen.omalley Owen O'Malley added a comment - This iteration removes the use of bash from the cases where it wasn't necessary.
          Hide
          lmccay Larry McCay added a comment - - edited

          LGTM - removes bash where there are alternatives and quotes the rest.
          +1 - pending jenkins green light.

          Show
          lmccay Larry McCay added a comment - - edited LGTM - removes bash where there are alternatives and quotes the rest. +1 - pending jenkins green light.
          Hide
          hadoopqa Hadoop QA added a comment -
          -1 overall



          Vote Subsystem Runtime Comment
          0 reexec 13m 1s 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 43s trunk passed
          +1 compile 6m 41s trunk passed
          +1 checkstyle 0m 23s trunk passed
          +1 mvnsite 0m 54s trunk passed
          +1 mvneclipse 0m 13s trunk passed
          +1 findbugs 1m 20s trunk passed
          +1 javadoc 0m 45s trunk passed
          +1 mvninstall 0m 36s the patch passed
          +1 compile 6m 40s the patch passed
          +1 javac 6m 40s the patch passed
          -0 checkstyle 0m 23s hadoop-common-project/hadoop-common: The patch generated 11 new + 49 unchanged - 10 fixed = 60 total (was 59)
          +1 mvnsite 0m 51s the patch passed
          +1 mvneclipse 0m 12s the patch passed
          +1 whitespace 0m 0s The patch has no whitespace issues.
          +1 findbugs 1m 23s the patch passed
          +1 javadoc 0m 45s the patch passed
          -1 unit 7m 34s hadoop-common in the patch failed.
          +1 asflicense 0m 22s The patch does not generate ASF License warnings.
          50m 13s



          Reason Tests
          Failed junit tests hadoop.metrics2.impl.TestGangliaMetrics



          Subsystem Report/Notes
          Docker Image:yetus/hadoop:9560f25
          JIRA Issue HADOOP-13434
          GITHUB PR https://github.com/apache/hadoop/pull/119
          Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle
          uname Linux 8ef61c839f69 3.13.0-36-lowlatency #63-Ubuntu SMP PREEMPT Wed Sep 3 21:56:12 UTC 2014 x86_64 x86_64 x86_64 GNU/Linux
          Build tool maven
          Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh
          git revision trunk / 95f2b98
          Default Java 1.8.0_101
          findbugs v3.0.0
          checkstyle https://builds.apache.org/job/PreCommit-HADOOP-Build/10120/artifact/patchprocess/diff-checkstyle-hadoop-common-project_hadoop-common.txt
          unit https://builds.apache.org/job/PreCommit-HADOOP-Build/10120/artifact/patchprocess/patch-unit-hadoop-common-project_hadoop-common.txt
          Test Results https://builds.apache.org/job/PreCommit-HADOOP-Build/10120/testReport/
          modules C: hadoop-common-project/hadoop-common U: hadoop-common-project/hadoop-common
          Console output https://builds.apache.org/job/PreCommit-HADOOP-Build/10120/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 13m 1s 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 43s trunk passed +1 compile 6m 41s trunk passed +1 checkstyle 0m 23s trunk passed +1 mvnsite 0m 54s trunk passed +1 mvneclipse 0m 13s trunk passed +1 findbugs 1m 20s trunk passed +1 javadoc 0m 45s trunk passed +1 mvninstall 0m 36s the patch passed +1 compile 6m 40s the patch passed +1 javac 6m 40s the patch passed -0 checkstyle 0m 23s hadoop-common-project/hadoop-common: The patch generated 11 new + 49 unchanged - 10 fixed = 60 total (was 59) +1 mvnsite 0m 51s the patch passed +1 mvneclipse 0m 12s the patch passed +1 whitespace 0m 0s The patch has no whitespace issues. +1 findbugs 1m 23s the patch passed +1 javadoc 0m 45s the patch passed -1 unit 7m 34s hadoop-common in the patch failed. +1 asflicense 0m 22s The patch does not generate ASF License warnings. 50m 13s Reason Tests Failed junit tests hadoop.metrics2.impl.TestGangliaMetrics Subsystem Report/Notes Docker Image:yetus/hadoop:9560f25 JIRA Issue HADOOP-13434 GITHUB PR https://github.com/apache/hadoop/pull/119 Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle uname Linux 8ef61c839f69 3.13.0-36-lowlatency #63-Ubuntu SMP PREEMPT Wed Sep 3 21:56:12 UTC 2014 x86_64 x86_64 x86_64 GNU/Linux Build tool maven Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh git revision trunk / 95f2b98 Default Java 1.8.0_101 findbugs v3.0.0 checkstyle https://builds.apache.org/job/PreCommit-HADOOP-Build/10120/artifact/patchprocess/diff-checkstyle-hadoop-common-project_hadoop-common.txt unit https://builds.apache.org/job/PreCommit-HADOOP-Build/10120/artifact/patchprocess/patch-unit-hadoop-common-project_hadoop-common.txt Test Results https://builds.apache.org/job/PreCommit-HADOOP-Build/10120/testReport/ modules C: hadoop-common-project/hadoop-common U: hadoop-common-project/hadoop-common Console output https://builds.apache.org/job/PreCommit-HADOOP-Build/10120/console Powered by Apache Yetus 0.4.0-SNAPSHOT http://yetus.apache.org This message was automatically generated.
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user arp7 commented on the issue:

          https://github.com/apache/hadoop/pull/119

          +1 lgtm.

          Show
          githubbot ASF GitHub Bot added a comment - Github user arp7 commented on the issue: https://github.com/apache/hadoop/pull/119 +1 lgtm.
          Hide
          owen.omalley Owen O'Malley added a comment -

          I made some tweaks to make check style happy.

          Show
          owen.omalley Owen O'Malley added a comment - I made some tweaks to make check style happy.
          Hide
          hadoopqa Hadoop QA added a comment -
          -1 overall



          Vote Subsystem Runtime Comment
          0 reexec 0m 14s 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 0s trunk passed
          +1 compile 7m 56s trunk passed
          +1 checkstyle 0m 26s trunk passed
          +1 mvnsite 1m 3s trunk passed
          +1 mvneclipse 0m 12s trunk passed
          +1 findbugs 1m 31s trunk passed
          +1 javadoc 0m 42s trunk passed
          +1 mvninstall 0m 38s the patch passed
          +1 compile 6m 45s the patch passed
          +1 javac 6m 45s the patch passed
          +1 checkstyle 0m 23s hadoop-common-project/hadoop-common: The patch generated 0 new + 48 unchanged - 11 fixed = 48 total (was 59)
          +1 mvnsite 0m 54s 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 45s the patch passed
          -1 unit 7m 30s hadoop-common in the patch failed.
          +1 asflicense 0m 21s The patch does not generate ASF License warnings.
          40m 22s



          Reason Tests
          Failed junit tests hadoop.metrics2.impl.TestGangliaMetrics



          Subsystem Report/Notes
          Docker Image:yetus/hadoop:9560f25
          JIRA Issue HADOOP-13434
          GITHUB PR https://github.com/apache/hadoop/pull/119
          Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle
          uname Linux a2c95e691938 3.13.0-36-lowlatency #63-Ubuntu SMP PREEMPT Wed Sep 3 21:56:12 UTC 2014 x86_64 x86_64 x86_64 GNU/Linux
          Build tool maven
          Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh
          git revision trunk / 95f2b98
          Default Java 1.8.0_101
          findbugs v3.0.0
          unit https://builds.apache.org/job/PreCommit-HADOOP-Build/10125/artifact/patchprocess/patch-unit-hadoop-common-project_hadoop-common.txt
          Test Results https://builds.apache.org/job/PreCommit-HADOOP-Build/10125/testReport/
          modules C: hadoop-common-project/hadoop-common U: hadoop-common-project/hadoop-common
          Console output https://builds.apache.org/job/PreCommit-HADOOP-Build/10125/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 14s 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 0s trunk passed +1 compile 7m 56s trunk passed +1 checkstyle 0m 26s trunk passed +1 mvnsite 1m 3s trunk passed +1 mvneclipse 0m 12s trunk passed +1 findbugs 1m 31s trunk passed +1 javadoc 0m 42s trunk passed +1 mvninstall 0m 38s the patch passed +1 compile 6m 45s the patch passed +1 javac 6m 45s the patch passed +1 checkstyle 0m 23s hadoop-common-project/hadoop-common: The patch generated 0 new + 48 unchanged - 11 fixed = 48 total (was 59) +1 mvnsite 0m 54s 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 45s the patch passed -1 unit 7m 30s hadoop-common in the patch failed. +1 asflicense 0m 21s The patch does not generate ASF License warnings. 40m 22s Reason Tests Failed junit tests hadoop.metrics2.impl.TestGangliaMetrics Subsystem Report/Notes Docker Image:yetus/hadoop:9560f25 JIRA Issue HADOOP-13434 GITHUB PR https://github.com/apache/hadoop/pull/119 Optional Tests asflicense compile javac javadoc mvninstall mvnsite unit findbugs checkstyle uname Linux a2c95e691938 3.13.0-36-lowlatency #63-Ubuntu SMP PREEMPT Wed Sep 3 21:56:12 UTC 2014 x86_64 x86_64 x86_64 GNU/Linux Build tool maven Personality /testptch/hadoop/patchprocess/precommit/personality/provided.sh git revision trunk / 95f2b98 Default Java 1.8.0_101 findbugs v3.0.0 unit https://builds.apache.org/job/PreCommit-HADOOP-Build/10125/artifact/patchprocess/patch-unit-hadoop-common-project_hadoop-common.txt Test Results https://builds.apache.org/job/PreCommit-HADOOP-Build/10125/testReport/ modules C: hadoop-common-project/hadoop-common U: hadoop-common-project/hadoop-common Console output https://builds.apache.org/job/PreCommit-HADOOP-Build/10125/console Powered by Apache Yetus 0.4.0-SNAPSHOT http://yetus.apache.org This message was automatically generated.
          Hide
          aw Allen Wittenauer added a comment -
          +      : new String[] { "/bin/bash", bashQuote(absolutePath) };
          

          While we're here, let's remove the path to /bin/bash since that won't work on every platform.

          Show
          aw Allen Wittenauer added a comment - + : new String [] { "/bin/bash" , bashQuote(absolutePath) }; While we're here, let's remove the path to /bin/bash since that won't work on every platform.
          Hide
          lmccay Larry McCay added a comment -

          Allen Wittenauer - that seems reasonable and correct to me.
          Owen O'Malley - Have you checked the unit test failure from the jenkins run - it strikes me as unrelated.

          Show
          lmccay Larry McCay added a comment - Allen Wittenauer - that seems reasonable and correct to me. Owen O'Malley - Have you checked the unit test failure from the jenkins run - it strikes me as unrelated.
          Hide
          arpitagarwal Arpit Agarwal added a comment - - edited

          The test failure looks unrelated (won't repro for me).

          I'd like to commit this tomorrow. Allen, I can post a separate patch for the fix you suggested after I commit this, assuming you don't object.

          Show
          arpitagarwal Arpit Agarwal added a comment - - edited The test failure looks unrelated (won't repro for me). I'd like to commit this tomorrow. Allen, I can post a separate patch for the fix you suggested after I commit this, assuming you don't object.
          Hide
          arpitagarwal Arpit Agarwal added a comment -

          Pushed this through to branch-2.8. Thank you for fixing this Owen O'Malley and Larry McCay for the code review. Filed HADOOP-13457 for the bug-fix suggested by Allen.

          Cherry-picking to branch-2.7 threw up many conflicts. I suspect they will be straightforward. I'll post a 2.7.x backport patch later today.

          Show
          arpitagarwal Arpit Agarwal added a comment - Pushed this through to branch-2.8. Thank you for fixing this Owen O'Malley and Larry McCay for the code review. Filed HADOOP-13457 for the bug-fix suggested by Allen. Cherry-picking to branch-2.7 threw up many conflicts. I suspect they will be straightforward. I'll post a 2.7.x backport patch later today.
          Hide
          hudson Hudson added a comment -

          SUCCESS: Integrated in Hadoop-trunk-Commit #10196 (See https://builds.apache.org/job/Hadoop-trunk-Commit/10196/)
          HADOOP-13434. Add bash quoting to Shell class. (Owen O'Malley) (arp: rev 954465e7ba3af3e5b083b6251562e5e77529f906)

          • hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/util/Shell.java
          • hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/util/TestShell.java
          Show
          hudson Hudson added a comment - SUCCESS: Integrated in Hadoop-trunk-Commit #10196 (See https://builds.apache.org/job/Hadoop-trunk-Commit/10196/ ) HADOOP-13434 . Add bash quoting to Shell class. (Owen O'Malley) (arp: rev 954465e7ba3af3e5b083b6251562e5e77529f906) hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/util/Shell.java hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/util/TestShell.java
          Hide
          arpitagarwal Arpit Agarwal added a comment -

          Reopening to attach branch-2.7 patch.

          Show
          arpitagarwal Arpit Agarwal added a comment - Reopening to attach branch-2.7 patch.
          Hide
          arpitagarwal Arpit Agarwal added a comment -

          Backport of Owen's patch. The conflicts were easy to resolve.

          Show
          arpitagarwal Arpit Agarwal added a comment - Backport of Owen's patch. The conflicts were easy to resolve.
          Hide
          hadoopqa Hadoop QA added a comment -
          -1 overall



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



          Subsystem Report/Notes
          JIRA Issue HADOOP-13434
          GITHUB PR https://github.com/apache/hadoop/pull/119
          Console output https://builds.apache.org/job/PreCommit-HADOOP-Build/10155/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 5s HADOOP-13434 does not apply to trunk. Rebase required? Wrong Branch? See https://wiki.apache.org/hadoop/HowToContribute for help. Subsystem Report/Notes JIRA Issue HADOOP-13434 GITHUB PR https://github.com/apache/hadoop/pull/119 Console output https://builds.apache.org/job/PreCommit-HADOOP-Build/10155/console Powered by Apache Yetus 0.4.0-SNAPSHOT http://yetus.apache.org This message was automatically generated.
          Hide
          arpitagarwal Arpit Agarwal added a comment - - edited

          Pushed to branch-2.7 and branch-2.7.3 also based (thanks Larry McCay for taking a look at the 2.7 patch).

          Show
          arpitagarwal Arpit Agarwal added a comment - - edited Pushed to branch-2.7 and branch-2.7.3 also based (thanks Larry McCay for taking a look at the 2.7 patch).
          Hide
          lmccay Larry McCay added a comment -

          Don't know what happened to my +1 for the 2.7 patch - so here it is again:

          +1

          Show
          lmccay Larry McCay added a comment - Don't know what happened to my +1 for the 2.7 patch - so here it is again: +1
          Hide
          vinodkv Vinod Kumar Vavilapalli added a comment -

          Closing the JIRA as part of 2.7.3 release.

          Show
          vinodkv Vinod Kumar Vavilapalli added a comment - Closing the JIRA as part of 2.7.3 release.
          Hide
          sjlee0 Sangjin Lee added a comment -

          Cherry-picked it to 2.6.5 (trivial).

          Show
          sjlee0 Sangjin Lee added a comment - Cherry-picked it to 2.6.5 (trivial).
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user aajisaka commented on the issue:

          https://github.com/apache/hadoop/pull/119

          HADOOP-13434 has been fixed. Hi @omalley, would you close this pull request?

          Show
          githubbot ASF GitHub Bot added a comment - Github user aajisaka commented on the issue: https://github.com/apache/hadoop/pull/119 HADOOP-13434 has been fixed. Hi @omalley, would you close this pull request?
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user asfgit closed the pull request at:

          https://github.com/apache/hadoop/pull/119

          Show
          githubbot ASF GitHub Bot added a comment - Github user asfgit closed the pull request at: https://github.com/apache/hadoop/pull/119

            People

            • Assignee:
              owen.omalley Owen O'Malley
              Reporter:
              owen.omalley Owen O'Malley
            • Votes:
              0 Vote for this issue
              Watchers:
              17 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development