Hadoop Common
  1. Hadoop Common
  2. HADOOP-6443

Serialization classes accept invalid metadata

    Details

    • Type: Improvement Improvement
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 0.21.0
    • Component/s: io
    • Labels:
      None

      Description

      The SerializationBase.accept() methods of several serialization implementations use incorrect metadata when determining whether they are the correct serializer for the user's metadata.

      1. HADOOP-6443.3.patch
        12 kB
        Aaron Kimball
      2. HADOOP-6443.2.patch
        7 kB
        Aaron Kimball
      3. HADOOP-6443.patch
        4 kB
        Aaron Kimball

        Issue Links

          Activity

          Hide
          Hudson added a comment -

          Integrated in Hadoop-Common-trunk #209 (See http://hudson.zones.apache.org/hudson/job/Hadoop-Common-trunk/209/)
          . Serialization classes accept invalid metadata. Contributed by Aaron Kimball.

          Show
          Hudson added a comment - Integrated in Hadoop-Common-trunk #209 (See http://hudson.zones.apache.org/hudson/job/Hadoop-Common-trunk/209/ ) . Serialization classes accept invalid metadata. Contributed by Aaron Kimball.
          Hide
          Hudson added a comment -

          Integrated in Hadoop-Common-trunk-Commit #132 (See http://hudson.zones.apache.org/hudson/job/Hadoop-Common-trunk-Commit/132/)
          . Serialization classes accept invalid metadata. Contributed by Aaron Kimball.

          Show
          Hudson added a comment - Integrated in Hadoop-Common-trunk-Commit #132 (See http://hudson.zones.apache.org/hudson/job/Hadoop-Common-trunk-Commit/132/ ) . Serialization classes accept invalid metadata. Contributed by Aaron Kimball.
          Hide
          Tom White added a comment -

          I've just committed this. Thanks Aaron!

          Show
          Tom White added a comment - I've just committed this. Thanks Aaron!
          Hide
          Hadoop QA added a comment -

          +1 overall. Here are the results of testing the latest attachment
          http://issues.apache.org/jira/secure/attachment/12429238/HADOOP-6443.3.patch
          against trunk revision 894706.

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

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

          +1 javadoc. The javadoc tool did not generate any warning messages.

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

          +1 findbugs. The patch does not introduce any new Findbugs warnings.

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

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

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

          Test results: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-h4.grid.sp2.yahoo.net/246/testReport/
          Findbugs warnings: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-h4.grid.sp2.yahoo.net/246/artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html
          Checkstyle results: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-h4.grid.sp2.yahoo.net/246/artifact/trunk/build/test/checkstyle-errors.html
          Console output: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-h4.grid.sp2.yahoo.net/246/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/12429238/HADOOP-6443.3.patch against trunk revision 894706. +1 @author. The patch does not contain any @author tags. +1 tests included. The patch appears to include 6 new or modified tests. +1 javadoc. The javadoc tool did not generate any warning messages. +1 javac. The applied patch does not increase the total number of javac compiler warnings. +1 findbugs. The patch does not introduce any new Findbugs warnings. +1 release audit. The applied patch does not increase the total number of release audit warnings. +1 core tests. The patch passed core unit tests. +1 contrib tests. The patch passed contrib unit tests. Test results: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-h4.grid.sp2.yahoo.net/246/testReport/ Findbugs warnings: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-h4.grid.sp2.yahoo.net/246/artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html Checkstyle results: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-h4.grid.sp2.yahoo.net/246/artifact/trunk/build/test/checkstyle-errors.html Console output: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-h4.grid.sp2.yahoo.net/246/console This message is automatically generated.
          Hide
          Aaron Kimball added a comment -

          factored.

          Show
          Aaron Kimball added a comment - factored.
          Hide
          Tom White added a comment -

          +1

          A nit: the logic for checking for SERIALIZATION_KEY is repeated in three places. Can you factor it out into a method?

          Show
          Tom White added a comment - +1 A nit: the logic for checking for SERIALIZATION_KEY is repeated in three places. Can you factor it out into a method?
          Hide
          Hadoop QA added a comment -

          +1 overall. Here are the results of testing the latest attachment
          http://issues.apache.org/jira/secure/attachment/12428232/HADOOP-6443.2.patch
          against trunk revision 891132.

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

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

          +1 javadoc. The javadoc tool did not generate any warning messages.

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

          +1 findbugs. The patch does not introduce any new Findbugs warnings.

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

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

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

          Test results: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-h4.grid.sp2.yahoo.net/212/testReport/
          Findbugs warnings: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-h4.grid.sp2.yahoo.net/212/artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html
          Checkstyle results: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-h4.grid.sp2.yahoo.net/212/artifact/trunk/build/test/checkstyle-errors.html
          Console output: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-h4.grid.sp2.yahoo.net/212/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/12428232/HADOOP-6443.2.patch against trunk revision 891132. +1 @author. The patch does not contain any @author tags. +1 tests included. The patch appears to include 6 new or modified tests. +1 javadoc. The javadoc tool did not generate any warning messages. +1 javac. The applied patch does not increase the total number of javac compiler warnings. +1 findbugs. The patch does not introduce any new Findbugs warnings. +1 release audit. The applied patch does not increase the total number of release audit warnings. +1 core tests. The patch passed core unit tests. +1 contrib tests. The patch passed contrib unit tests. Test results: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-h4.grid.sp2.yahoo.net/212/testReport/ Findbugs warnings: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-h4.grid.sp2.yahoo.net/212/artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html Checkstyle results: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-h4.grid.sp2.yahoo.net/212/artifact/trunk/build/test/checkstyle-errors.html Console output: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-h4.grid.sp2.yahoo.net/212/console This message is automatically generated.
          Hide
          Aaron Kimball added a comment -

          Added some unit test cases for this.

          Show
          Aaron Kimball added a comment - Added some unit test cases for this.
          Hide
          Hadoop QA added a comment -

          -1 overall. Here are the results of testing the latest attachment
          http://issues.apache.org/jira/secure/attachment/12428120/HADOOP-6443.patch
          against trunk revision 890964.

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

          -1 tests included. The patch doesn't appear to include any new or modified tests.
          Please justify why no new tests are needed for this patch.
          Also please list what manual steps were performed to verify this patch.

          +1 javadoc. The javadoc tool did not generate any warning messages.

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

          +1 findbugs. The patch does not introduce any new Findbugs warnings.

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

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

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

          Test results: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-h1.grid.sp2.yahoo.net/30/testReport/
          Findbugs warnings: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-h1.grid.sp2.yahoo.net/30/artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html
          Checkstyle results: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-h1.grid.sp2.yahoo.net/30/artifact/trunk/build/test/checkstyle-errors.html
          Console output: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-h1.grid.sp2.yahoo.net/30/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/12428120/HADOOP-6443.patch against trunk revision 890964. +1 @author. The patch does not contain any @author tags. -1 tests included. The patch doesn't appear to include any new or modified tests. Please justify why no new tests are needed for this patch. Also please list what manual steps were performed to verify this patch. +1 javadoc. The javadoc tool did not generate any warning messages. +1 javac. The applied patch does not increase the total number of javac compiler warnings. +1 findbugs. The patch does not introduce any new Findbugs warnings. +1 release audit. The applied patch does not increase the total number of release audit warnings. +1 core tests. The patch passed core unit tests. +1 contrib tests. The patch passed contrib unit tests. Test results: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-h1.grid.sp2.yahoo.net/30/testReport/ Findbugs warnings: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-h1.grid.sp2.yahoo.net/30/artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html Checkstyle results: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-h1.grid.sp2.yahoo.net/30/artifact/trunk/build/test/checkstyle-errors.html Console output: http://hudson.zones.apache.org/hudson/job/Hadoop-Patch-h1.grid.sp2.yahoo.net/30/console This message is automatically generated.
          Hide
          Aaron Kimball added a comment -

          Submitting straightforward patch for this; this was written in conjunction with HADOOP-6323 but was split into a separate patch in my workflow. This code is tested via the serialization instantiations in MAPREDUCE-1126 (e.g., TestJavaSerialization, TestAvroSerialization, etc in the MAPREDUCE project).

          Show
          Aaron Kimball added a comment - Submitting straightforward patch for this; this was written in conjunction with HADOOP-6323 but was split into a separate patch in my workflow. This code is tested via the serialization instantiations in MAPREDUCE-1126 (e.g., TestJavaSerialization, TestAvroSerialization, etc in the MAPREDUCE project).
          Hide
          Aaron Kimball added a comment -

          patch providing this behavior.

          Show
          Aaron Kimball added a comment - patch providing this behavior.
          Hide
          Aaron Kimball added a comment -

          The accept methods of the serializers check whether SERIALIZATION_CLASS equals the current class name. If so, they accept unconditionally, even though they contain further (unreachable) logic to ensure that they contain the requisite metadata (e.g., an avro schema or a class name).

          The check should work in the opposite direction: if the user specifies SERIALIZATION_CLASS and it does not equal the current class name, then the serialization should reject the metadata. If the SERIALIZATION_CLASS equals the current class name, or is simply unset, then the serialization should inspect the remainder of the metadata to ensure that it meets the criteria associated with the current serialization.

          Show
          Aaron Kimball added a comment - The accept methods of the serializers check whether SERIALIZATION_CLASS equals the current class name. If so, they accept unconditionally, even though they contain further (unreachable) logic to ensure that they contain the requisite metadata (e.g., an avro schema or a class name). The check should work in the opposite direction: if the user specifies SERIALIZATION_CLASS and it does not equal the current class name, then the serialization should reject the metadata. If the SERIALIZATION_CLASS equals the current class name, or is simply unset, then the serialization should inspect the remainder of the metadata to ensure that it meets the criteria associated with the current serialization.

            People

            • Assignee:
              Aaron Kimball
              Reporter:
              Aaron Kimball
            • Votes:
              0 Vote for this issue
              Watchers:
              4 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development