Hadoop Map/Reduce
  1. Hadoop Map/Reduce
  2. MAPREDUCE-5748

Potential null pointer dereference in ShuffleHandler#Shuffle#messageReceived()

    Details

    • Type: Bug Bug
    • Status: Patch Available
    • Priority: Minor Minor
    • Resolution: Unresolved
    • Affects Version/s: 2.7.0
    • Fix Version/s: None
    • Component/s: None
    • Labels:
    • Target Version/s:

      Description

      Starting around line 510:

            ChannelFuture lastMap = null;
            for (String mapId : mapIds) {
      ...
            }
            lastMap.addListener(metrics);
            lastMap.addListener(ChannelFutureListener.CLOSE);
      

      If mapIds is empty, lastMap would remain null, leading to NPE in addListener() call.

      1. MAPREDUCE-5748.03.patch
        5 kB
        Varun Saxena
      2. MAPREDUCE-5748.02.patch
        5 kB
        Varun Saxena
      3. 0001-MAPREDUCE-5748-Potential-null-pointer-deference-in-S.patch
        5 kB
        Hussein Baghdadi

        Activity

        Hide
        Varun Saxena added a comment -

        Devaraj K, looked at it. I do not think null pointer dereference can occur i.e. the newly added code which checks against mapIds.size() being 0 is not possible. Even if map parameter is empty i.e. map=, String#split would still return an array of strings with one element(which will be empty string). So mapIds size would atleast be 1.
        Ted Yu, thoughts on this ?

        We can however add a basic check for empty parameters because empty parameter strings do not make any sense.

        Show
        Varun Saxena added a comment - Devaraj K , looked at it. I do not think null pointer dereference can occur i.e. the newly added code which checks against mapIds.size() being 0 is not possible. Even if map parameter is empty i.e. map= , String#split would still return an array of strings with one element(which will be empty string). So mapIds size would atleast be 1. Ted Yu , thoughts on this ? We can however add a basic check for empty parameters because empty parameter strings do not make any sense.
        Hide
        Varun Saxena added a comment -

        OK Devaraj K..Will check this.

        Show
        Varun Saxena added a comment - OK Devaraj K ..Will check this.
        Hide
        Devaraj K added a comment -

        As Karthik mentioned in the above comment, newly added test passes without the src changes as well. This test is not verifying the code changes, can you have a look into the newly added test?

        Show
        Devaraj K added a comment - As Karthik mentioned in the above comment, newly added test passes without the src changes as well. This test is not verifying the code changes, can you have a look into the newly added test?
        Hide
        Hadoop QA added a comment -



        +1 overall



        Vote Subsystem Runtime Comment
        0 pre-patch 14m 37s Pre-patch trunk compilation is healthy.
        +1 @author 0m 0s The patch does not contain any @author tags.
        +1 tests included 0m 0s The patch appears to include 1 new or modified test files.
        +1 javac 7m 31s There were no new javac warning messages.
        +1 javadoc 9m 31s There were no new javadoc warning messages.
        +1 release audit 0m 22s The applied patch does not increase the total number of release audit warnings.
        +1 checkstyle 0m 20s There were no new checkstyle issues.
        +1 whitespace 0m 0s The patch has no lines that end in whitespace.
        +1 install 1m 37s mvn install still works.
        +1 eclipse:eclipse 0m 33s The patch built with eclipse:eclipse.
        +1 findbugs 0m 36s The patch does not introduce any new Findbugs (version 2.0.3) warnings.
        +1 mapreduce tests 0m 19s Tests passed in hadoop-mapreduce-client-shuffle.
            35m 30s  



        Subsystem Report/Notes
        Patch URL http://issues.apache.org/jira/secure/attachment/12731742/MAPREDUCE-5748.03.patch
        Optional Tests javadoc javac unit findbugs checkstyle
        git revision trunk / df36ad0
        hadoop-mapreduce-client-shuffle test log https://builds.apache.org/job/PreCommit-MAPREDUCE-Build/5697/artifact/patchprocess/testrun_hadoop-mapreduce-client-shuffle.txt
        Test Results https://builds.apache.org/job/PreCommit-MAPREDUCE-Build/5697/testReport/
        Java 1.7.0_55
        uname Linux asf905.gq1.ygridcore.net 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
        Console output https://builds.apache.org/job/PreCommit-MAPREDUCE-Build/5697/console

        This message was automatically generated.

        Show
        Hadoop QA added a comment - +1 overall Vote Subsystem Runtime Comment 0 pre-patch 14m 37s Pre-patch trunk compilation is healthy. +1 @author 0m 0s The patch does not contain any @author tags. +1 tests included 0m 0s The patch appears to include 1 new or modified test files. +1 javac 7m 31s There were no new javac warning messages. +1 javadoc 9m 31s There were no new javadoc warning messages. +1 release audit 0m 22s The applied patch does not increase the total number of release audit warnings. +1 checkstyle 0m 20s There were no new checkstyle issues. +1 whitespace 0m 0s The patch has no lines that end in whitespace. +1 install 1m 37s mvn install still works. +1 eclipse:eclipse 0m 33s The patch built with eclipse:eclipse. +1 findbugs 0m 36s The patch does not introduce any new Findbugs (version 2.0.3) warnings. +1 mapreduce tests 0m 19s Tests passed in hadoop-mapreduce-client-shuffle.     35m 30s   Subsystem Report/Notes Patch URL http://issues.apache.org/jira/secure/attachment/12731742/MAPREDUCE-5748.03.patch Optional Tests javadoc javac unit findbugs checkstyle git revision trunk / df36ad0 hadoop-mapreduce-client-shuffle test log https://builds.apache.org/job/PreCommit-MAPREDUCE-Build/5697/artifact/patchprocess/testrun_hadoop-mapreduce-client-shuffle.txt Test Results https://builds.apache.org/job/PreCommit-MAPREDUCE-Build/5697/testReport/ Java 1.7.0_55 uname Linux asf905.gq1.ygridcore.net 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 Console output https://builds.apache.org/job/PreCommit-MAPREDUCE-Build/5697/console This message was automatically generated.
        Hide
        Varun Saxena added a comment -

        Fixed checkstyle issue

        Show
        Varun Saxena added a comment - Fixed checkstyle issue
        Hide
        Hadoop QA added a comment -



        -1 overall



        Vote Subsystem Runtime Comment
        0 pre-patch 14m 34s Pre-patch trunk compilation is healthy.
        +1 @author 0m 0s The patch does not contain any @author tags.
        +1 tests included 0m 0s The patch appears to include 1 new or modified test files.
        +1 javac 7m 32s There were no new javac warning messages.
        +1 javadoc 9m 38s There were no new javadoc warning messages.
        +1 release audit 0m 22s The applied patch does not increase the total number of release audit warnings.
        -1 checkstyle 0m 20s The applied patch generated 1 new checkstyle issues (total was 60, now 61).
        -1 whitespace 0m 0s The patch has 8 line(s) that end in whitespace. Use git apply --whitespace=fix.
        +1 install 1m 36s mvn install still works.
        +1 eclipse:eclipse 0m 33s The patch built with eclipse:eclipse.
        +1 findbugs 0m 36s The patch does not introduce any new Findbugs (version 2.0.3) warnings.
        +1 mapreduce tests 0m 19s Tests passed in hadoop-mapreduce-client-shuffle.
            35m 34s  



        Subsystem Report/Notes
        Patch URL http://issues.apache.org/jira/secure/attachment/12731626/MAPREDUCE-5748.02.patch
        Optional Tests javadoc javac unit findbugs checkstyle
        git revision trunk / 02a4a22
        checkstyle https://builds.apache.org/job/PreCommit-MAPREDUCE-Build/5695/artifact/patchprocess/diffcheckstylehadoop-mapreduce-client-shuffle.txt
        whitespace https://builds.apache.org/job/PreCommit-MAPREDUCE-Build/5695/artifact/patchprocess/whitespace.txt
        hadoop-mapreduce-client-shuffle test log https://builds.apache.org/job/PreCommit-MAPREDUCE-Build/5695/artifact/patchprocess/testrun_hadoop-mapreduce-client-shuffle.txt
        Test Results https://builds.apache.org/job/PreCommit-MAPREDUCE-Build/5695/testReport/
        Java 1.7.0_55
        uname Linux asf905.gq1.ygridcore.net 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
        Console output https://builds.apache.org/job/PreCommit-MAPREDUCE-Build/5695/console

        This message was automatically generated.

        Show
        Hadoop QA added a comment - -1 overall Vote Subsystem Runtime Comment 0 pre-patch 14m 34s Pre-patch trunk compilation is healthy. +1 @author 0m 0s The patch does not contain any @author tags. +1 tests included 0m 0s The patch appears to include 1 new or modified test files. +1 javac 7m 32s There were no new javac warning messages. +1 javadoc 9m 38s There were no new javadoc warning messages. +1 release audit 0m 22s The applied patch does not increase the total number of release audit warnings. -1 checkstyle 0m 20s The applied patch generated 1 new checkstyle issues (total was 60, now 61). -1 whitespace 0m 0s The patch has 8 line(s) that end in whitespace. Use git apply --whitespace=fix. +1 install 1m 36s mvn install still works. +1 eclipse:eclipse 0m 33s The patch built with eclipse:eclipse. +1 findbugs 0m 36s The patch does not introduce any new Findbugs (version 2.0.3) warnings. +1 mapreduce tests 0m 19s Tests passed in hadoop-mapreduce-client-shuffle.     35m 34s   Subsystem Report/Notes Patch URL http://issues.apache.org/jira/secure/attachment/12731626/MAPREDUCE-5748.02.patch Optional Tests javadoc javac unit findbugs checkstyle git revision trunk / 02a4a22 checkstyle https://builds.apache.org/job/PreCommit-MAPREDUCE-Build/5695/artifact/patchprocess/diffcheckstylehadoop-mapreduce-client-shuffle.txt whitespace https://builds.apache.org/job/PreCommit-MAPREDUCE-Build/5695/artifact/patchprocess/whitespace.txt hadoop-mapreduce-client-shuffle test log https://builds.apache.org/job/PreCommit-MAPREDUCE-Build/5695/artifact/patchprocess/testrun_hadoop-mapreduce-client-shuffle.txt Test Results https://builds.apache.org/job/PreCommit-MAPREDUCE-Build/5695/testReport/ Java 1.7.0_55 uname Linux asf905.gq1.ygridcore.net 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 Console output https://builds.apache.org/job/PreCommit-MAPREDUCE-Build/5695/console This message was automatically generated.
        Hide
        Varun Saxena added a comment -

        Karthik Kambatla, which test case are you talking about ? The test added passes in local. Not sure why Jenkins took the old patch. Resubmitting the patch again to kick Jenkins

        Show
        Varun Saxena added a comment - Karthik Kambatla , which test case are you talking about ? The test added passes in local. Not sure why Jenkins took the old patch. Resubmitting the patch again to kick Jenkins
        Hide
        Karthik Kambatla added a comment -

        The test doesn't fail without the fix. Varun Saxena - could you look into it please?

        Show
        Karthik Kambatla added a comment - The test doesn't fail without the fix. Varun Saxena - could you look into it please?
        Hide
        Varun Saxena added a comment -

        Rebased patch of Hussein Baghdadi so that it can be taken for commit during Bug Bash

        Show
        Varun Saxena added a comment - Rebased patch of Hussein Baghdadi so that it can be taken for commit during Bug Bash
        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/12635637/0001-MAPREDUCE-5748-Potential-null-pointer-deference-in-S.patch
        Optional Tests javadoc javac unit findbugs checkstyle
        git revision trunk / f1a152c
        Console output https://builds.apache.org/job/PreCommit-MAPREDUCE-Build/5549/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/12635637/0001-MAPREDUCE-5748-Potential-null-pointer-deference-in-S.patch Optional Tests javadoc javac unit findbugs checkstyle git revision trunk / f1a152c Console output https://builds.apache.org/job/PreCommit-MAPREDUCE-Build/5549/console This message was automatically generated.
        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/12635637/0001-MAPREDUCE-5748-Potential-null-pointer-deference-in-S.patch
        Optional Tests javadoc javac unit findbugs checkstyle
        git revision trunk / f1a152c
        Console output https://builds.apache.org/job/PreCommit-MAPREDUCE-Build/5540/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/12635637/0001-MAPREDUCE-5748-Potential-null-pointer-deference-in-S.patch Optional Tests javadoc javac unit findbugs checkstyle git revision trunk / f1a152c Console output https://builds.apache.org/job/PreCommit-MAPREDUCE-Build/5540/console This message was automatically generated.
        Hide
        Hadoop QA added a comment -

        +1 overall. Here are the results of testing the latest attachment
        http://issues.apache.org/jira/secure/attachment/12635637/0001-MAPREDUCE-5748-Potential-null-pointer-deference-in-S.patch
        against trunk revision .

        +1 @author. The patch does not contain any @author tags.

        +1 tests included. The patch appears to include 1 new or modified test files.

        +1 javac. The applied patch does not increase the total number of javac compiler warnings.

        +1 javadoc. There were no new javadoc warning messages.

        +1 eclipse:eclipse. The patch built with eclipse:eclipse.

        +1 findbugs. The patch does not introduce any new Findbugs (version 1.3.9) warnings.

        +1 release audit. The applied patch does not increase the total number of release audit warnings.

        +1 core tests. The patch passed unit tests in hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-shuffle.

        +1 contrib tests. The patch passed contrib unit tests.

        Test results: https://builds.apache.org/job/PreCommit-MAPREDUCE-Build/4451//testReport/
        Console output: https://builds.apache.org/job/PreCommit-MAPREDUCE-Build/4451//console

        This message is automatically generated.

        Show
        Hadoop QA added a comment - +1 overall . Here are the results of testing the latest attachment http://issues.apache.org/jira/secure/attachment/12635637/0001-MAPREDUCE-5748-Potential-null-pointer-deference-in-S.patch against trunk revision . +1 @author . The patch does not contain any @author tags. +1 tests included . The patch appears to include 1 new or modified test files. +1 javac . The applied patch does not increase the total number of javac compiler warnings. +1 javadoc . There were no new javadoc warning messages. +1 eclipse:eclipse . The patch built with eclipse:eclipse. +1 findbugs . The patch does not introduce any new Findbugs (version 1.3.9) warnings. +1 release audit . The applied patch does not increase the total number of release audit warnings. +1 core tests . The patch passed unit tests in hadoop-mapreduce-project/hadoop-mapreduce-client/hadoop-mapreduce-client-shuffle. +1 contrib tests . The patch passed contrib unit tests. Test results: https://builds.apache.org/job/PreCommit-MAPREDUCE-Build/4451//testReport/ Console output: https://builds.apache.org/job/PreCommit-MAPREDUCE-Build/4451//console This message is automatically generated.
        Hide
        Ted Yu added a comment -

        +1 if tests pass.

        Thanks

        Show
        Ted Yu added a comment - +1 if tests pass. Thanks
        Hide
        Hussein Baghdadi added a comment -

        I tried to fix it. Kindly check the attached patched file.

        Show
        Hussein Baghdadi added a comment - I tried to fix it. Kindly check the attached patched file.
        Hide
        Hussein Baghdadi added a comment -

        Ok, sorry. My fault. You are right.

        Show
        Hussein Baghdadi added a comment - Ok, sorry. My fault. You are right.
        Hide
        Hussein Baghdadi added a comment -

        mapIds is already checked for null values:

        // line 472
        [code]
        if (mapIds == null || reduceQ == null || jobQ == null)

        { sendError(ctx, "Required param job, map and reduce", BAD_REQUEST); return; }

        [/code]

        Show
        Hussein Baghdadi added a comment - mapIds is already checked for null values: // line 472 [code] if (mapIds == null || reduceQ == null || jobQ == null) { sendError(ctx, "Required param job, map and reduce", BAD_REQUEST); return; } [/code]

          People

          • Assignee:
            Varun Saxena
            Reporter:
            Ted Yu
          • Votes:
            0 Vote for this issue
            Watchers:
            8 Start watching this issue

            Dates

            • Created:
              Updated:

              Development