Hadoop HDFS
  1. Hadoop HDFS
  2. HDFS-3516

Check content-type in WebHdfsFileSystem

    Details

    • Type: Improvement Improvement
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 1.1.0, 2.0.2-alpha
    • Component/s: hdfs-client, webhdfs
    • Labels:
      None
    • Hadoop Flags:
      Reviewed

      Description

      WebHdfsFileSystem currently tries to parse the response as json. It may be a good idea to check the content-type before parsing it.

      1. h3516_20120609_b-1.patch
        5 kB
        Tsz Wo Nicholas Sze
      2. h3516_20120609.patch
        5 kB
        Tsz Wo Nicholas Sze
      3. h3516_20120608.patch
        5 kB
        Tsz Wo Nicholas Sze
      4. h3516_20120607.patch
        4 kB
        Tsz Wo Nicholas Sze

        Activity

        Tsz Wo Nicholas Sze created issue -
        Hide
        Tsz Wo Nicholas Sze added a comment -

        h3516_20120607.patch: check content-type.

        Will add a test.

        Show
        Tsz Wo Nicholas Sze added a comment - h3516_20120607.patch: check content-type. Will add a test.
        Tsz Wo Nicholas Sze made changes -
        Field Original Value New Value
        Attachment h3516_20120607.patch [ 12531335 ]
        Tsz Wo Nicholas Sze made changes -
        Attachment h3516_20120607.patch [ 12531336 ]
        Tsz Wo Nicholas Sze made changes -
        Attachment h3516_20120607.patch [ 12531335 ]
        Hide
        Daryn Sharp added a comment -

        Once a test is added, looks good. Seems a tiny bit awkward to pass the content-type to parseJson but otoh it avoids redundancy.

        Show
        Daryn Sharp added a comment - Once a test is added, looks good. Seems a tiny bit awkward to pass the content-type to parseJson but otoh it avoids redundancy.
        Hide
        Tsz Wo Nicholas Sze added a comment -

        h3516_20120608.patch: adds a test.

        Show
        Tsz Wo Nicholas Sze added a comment - h3516_20120608.patch: adds a test.
        Tsz Wo Nicholas Sze made changes -
        Attachment h3516_20120608.patch [ 12531495 ]
        Tsz Wo Nicholas Sze made changes -
        Status Open [ 1 ] Patch Available [ 10002 ]
        Hide
        Hadoop QA added a comment -

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

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

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

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

        +1 javadoc. The javadoc tool did not generate any 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 failed these unit tests in hadoop-hdfs-project/hadoop-hdfs:

        org.apache.hadoop.hdfs.web.TestFSMainOperationsWebHdfs
        org.apache.hadoop.hdfs.security.TestDelegationToken
        org.apache.hadoop.fs.TestFcHdfsSymlink

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

        Test results: https://builds.apache.org/job/PreCommit-HDFS-Build/2626//testReport/
        Console output: https://builds.apache.org/job/PreCommit-HDFS-Build/2626//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/12531495/h3516_20120608.patch against trunk revision . +1 @author. The patch does not contain any @author tags. +1 tests included. The patch appears to include 2 new or modified test files. +1 javac. The applied patch does not increase the total number of javac compiler warnings. +1 javadoc. The javadoc tool did not generate any 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 failed these unit tests in hadoop-hdfs-project/hadoop-hdfs: org.apache.hadoop.hdfs.web.TestFSMainOperationsWebHdfs org.apache.hadoop.hdfs.security.TestDelegationToken org.apache.hadoop.fs.TestFcHdfsSymlink +1 contrib tests. The patch passed contrib unit tests. Test results: https://builds.apache.org/job/PreCommit-HDFS-Build/2626//testReport/ Console output: https://builds.apache.org/job/PreCommit-HDFS-Build/2626//console This message is automatically generated.
        Hide
        Tsz Wo Nicholas Sze added a comment -

        h3516_20120609.patch: fixes some bugs.

        Show
        Tsz Wo Nicholas Sze added a comment - h3516_20120609.patch: fixes some bugs.
        Tsz Wo Nicholas Sze made changes -
        Attachment h3516_20120609.patch [ 12531588 ]
        Hide
        Hadoop QA added a comment -

        +1 overall. Here are the results of testing the latest attachment
        http://issues.apache.org/jira/secure/attachment/12531588/h3516_20120609.patch
        against trunk revision .

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

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

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

        +1 javadoc. The javadoc tool did not generate any 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-hdfs-project/hadoop-hdfs.

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

        Test results: https://builds.apache.org/job/PreCommit-HDFS-Build/2632//testReport/
        Console output: https://builds.apache.org/job/PreCommit-HDFS-Build/2632//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/12531588/h3516_20120609.patch against trunk revision . +1 @author. The patch does not contain any @author tags. +1 tests included. The patch appears to include 2 new or modified test files. +1 javac. The applied patch does not increase the total number of javac compiler warnings. +1 javadoc. The javadoc tool did not generate any 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-hdfs-project/hadoop-hdfs. +1 contrib tests. The patch passed contrib unit tests. Test results: https://builds.apache.org/job/PreCommit-HDFS-Build/2632//testReport/ Console output: https://builds.apache.org/job/PreCommit-HDFS-Build/2632//console This message is automatically generated.
        Hide
        Daryn Sharp added a comment -

        It's unclear to me whether it can be assumed there is an error stream if there is an unexpected return code. The docs seem unclear about how to determine if an error occurred other than to check if the error stream is null. Perhaps it makes sense to always get the error stream, but if null, fallback to the output stream? Thoughts?

        Show
        Daryn Sharp added a comment - It's unclear to me whether it can be assumed there is an error stream if there is an unexpected return code. The docs seem unclear about how to determine if an error occurred other than to check if the error stream is null. Perhaps it makes sense to always get the error stream, but if null, fallback to the output stream? Thoughts?
        Hide
        Tsz Wo Nicholas Sze added a comment -

        For errors, the Java library sends the error message using error stream. So using error stream works well in practice. Also, this is implementation details and not to do with the WebHDFS API spec. (I guess you mean WebHDFS spec by docs. Or do you mean Java API doc?)

        I think it does not make sense to fall back to input stream (I believe you mean input stream by output stream) since the error messages won't be there. Say, there is funny bug that the op expects 307 TEMPORARY_REDIRECT but the server returns 200 OK with a JSON or HTML response. Then, it does not make sense to print out the response from input steam as error message.

        Show
        Tsz Wo Nicholas Sze added a comment - For errors, the Java library sends the error message using error stream. So using error stream works well in practice. Also, this is implementation details and not to do with the WebHDFS API spec. (I guess you mean WebHDFS spec by docs. Or do you mean Java API doc?) I think it does not make sense to fall back to input stream (I believe you mean input stream by output stream) since the error messages won't be there. Say, there is funny bug that the op expects 307 TEMPORARY_REDIRECT but the server returns 200 OK with a JSON or HTML response. Then, it does not make sense to print out the response from input steam as error message.
        Hide
        Daryn Sharp added a comment -

        +1 Sounds reasonable.

        Show
        Daryn Sharp added a comment - +1 Sounds reasonable.
        Hide
        Hudson added a comment -

        Integrated in Hadoop-Mapreduce-trunk-Commit #2406 (See https://builds.apache.org/job/Hadoop-Mapreduce-trunk-Commit/2406/)
        HDFS-3516. Check content-type in WebHdfsFileSystem. (Revision 1353800)

        Result = FAILURE
        szetszwo : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1353800
        Files :

        • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt
        • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/web/WebHdfsFileSystem.java
        • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/web/TestWebHdfsFileSystemContract.java
        • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/web/WebHdfsTestUtil.java
        Show
        Hudson added a comment - Integrated in Hadoop-Mapreduce-trunk-Commit #2406 (See https://builds.apache.org/job/Hadoop-Mapreduce-trunk-Commit/2406/ ) HDFS-3516 . Check content-type in WebHdfsFileSystem. (Revision 1353800) Result = FAILURE szetszwo : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1353800 Files : /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/web/WebHdfsFileSystem.java /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/web/TestWebHdfsFileSystemContract.java /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/web/WebHdfsTestUtil.java
        Hide
        Tsz Wo Nicholas Sze added a comment -

        I have committed this.

        Show
        Tsz Wo Nicholas Sze added a comment - I have committed this.
        Tsz Wo Nicholas Sze made changes -
        Status Patch Available [ 10002 ] Resolved [ 5 ]
        Hadoop Flags Reviewed [ 10343 ]
        Fix Version/s 2.0.1-alpha [ 12321440 ]
        Resolution Fixed [ 1 ]
        Hide
        Hudson added a comment -

        Integrated in Hadoop-Hdfs-trunk-Commit #2457 (See https://builds.apache.org/job/Hadoop-Hdfs-trunk-Commit/2457/)
        HDFS-3516. Check content-type in WebHdfsFileSystem. (Revision 1353800)

        Result = SUCCESS
        szetszwo : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1353800
        Files :

        • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt
        • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/web/WebHdfsFileSystem.java
        • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/web/TestWebHdfsFileSystemContract.java
        • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/web/WebHdfsTestUtil.java
        Show
        Hudson added a comment - Integrated in Hadoop-Hdfs-trunk-Commit #2457 (See https://builds.apache.org/job/Hadoop-Hdfs-trunk-Commit/2457/ ) HDFS-3516 . Check content-type in WebHdfsFileSystem. (Revision 1353800) Result = SUCCESS szetszwo : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1353800 Files : /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/web/WebHdfsFileSystem.java /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/web/TestWebHdfsFileSystemContract.java /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/web/WebHdfsTestUtil.java
        Hide
        Hudson added a comment -

        Integrated in Hadoop-Common-trunk-Commit #2388 (See https://builds.apache.org/job/Hadoop-Common-trunk-Commit/2388/)
        HDFS-3516. Check content-type in WebHdfsFileSystem. (Revision 1353800)

        Result = SUCCESS
        szetszwo : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1353800
        Files :

        • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt
        • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/web/WebHdfsFileSystem.java
        • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/web/TestWebHdfsFileSystemContract.java
        • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/web/WebHdfsTestUtil.java
        Show
        Hudson added a comment - Integrated in Hadoop-Common-trunk-Commit #2388 (See https://builds.apache.org/job/Hadoop-Common-trunk-Commit/2388/ ) HDFS-3516 . Check content-type in WebHdfsFileSystem. (Revision 1353800) Result = SUCCESS szetszwo : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1353800 Files : /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/web/WebHdfsFileSystem.java /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/web/TestWebHdfsFileSystemContract.java /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/web/WebHdfsTestUtil.java
        Hide
        Tsz Wo Nicholas Sze added a comment -

        h3516_20120609_b-1.patch: for branch-1.

        Show
        Tsz Wo Nicholas Sze added a comment - h3516_20120609_b-1.patch: for branch-1.
        Tsz Wo Nicholas Sze made changes -
        Attachment h3516_20120609_b-1.patch [ 12533461 ]
        Hide
        Tsz Wo Nicholas Sze added a comment -

        Committed also to branch-1 and branch-1.1.

        Show
        Tsz Wo Nicholas Sze added a comment - Committed also to branch-1 and branch-1.1.
        Tsz Wo Nicholas Sze made changes -
        Fix Version/s 1.1.0 [ 12317959 ]
        Hide
        Hudson added a comment -

        Integrated in Hadoop-Hdfs-trunk #1088 (See https://builds.apache.org/job/Hadoop-Hdfs-trunk/1088/)
        HDFS-3516. Check content-type in WebHdfsFileSystem. (Revision 1353800)

        Result = FAILURE
        szetszwo : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1353800
        Files :

        • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt
        • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/web/WebHdfsFileSystem.java
        • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/web/TestWebHdfsFileSystemContract.java
        • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/web/WebHdfsTestUtil.java
        Show
        Hudson added a comment - Integrated in Hadoop-Hdfs-trunk #1088 (See https://builds.apache.org/job/Hadoop-Hdfs-trunk/1088/ ) HDFS-3516 . Check content-type in WebHdfsFileSystem. (Revision 1353800) Result = FAILURE szetszwo : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1353800 Files : /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/web/WebHdfsFileSystem.java /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/web/TestWebHdfsFileSystemContract.java /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/web/WebHdfsTestUtil.java
        Hide
        Hudson added a comment -

        Integrated in Hadoop-Mapreduce-trunk #1121 (See https://builds.apache.org/job/Hadoop-Mapreduce-trunk/1121/)
        HDFS-3516. Check content-type in WebHdfsFileSystem. (Revision 1353800)

        Result = FAILURE
        szetszwo : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1353800
        Files :

        • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt
        • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/web/WebHdfsFileSystem.java
        • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/web/TestWebHdfsFileSystemContract.java
        • /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/web/WebHdfsTestUtil.java
        Show
        Hudson added a comment - Integrated in Hadoop-Mapreduce-trunk #1121 (See https://builds.apache.org/job/Hadoop-Mapreduce-trunk/1121/ ) HDFS-3516 . Check content-type in WebHdfsFileSystem. (Revision 1353800) Result = FAILURE szetszwo : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1353800 Files : /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/web/WebHdfsFileSystem.java /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/web/TestWebHdfsFileSystemContract.java /hadoop/common/trunk/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/web/WebHdfsTestUtil.java
        Hide
        Hudson added a comment -

        Integrated in Hadoop-Hdfs-0.23-Build #318 (See https://builds.apache.org/job/Hadoop-Hdfs-0.23-Build/318/)
        svn merge -c 1353800 FIXES: HDFS-3516. Check content-type in WebHdfsFileSystem. (Revision 1363000)

        Result = SUCCESS
        daryn : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1363000
        Files :

        • /hadoop/common/branches/branch-0.23/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt
        • /hadoop/common/branches/branch-0.23/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/web/WebHdfsFileSystem.java
        • /hadoop/common/branches/branch-0.23/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/web/TestWebHdfsFileSystemContract.java
        • /hadoop/common/branches/branch-0.23/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/web/WebHdfsTestUtil.java
        Show
        Hudson added a comment - Integrated in Hadoop-Hdfs-0.23-Build #318 (See https://builds.apache.org/job/Hadoop-Hdfs-0.23-Build/318/ ) svn merge -c 1353800 FIXES: HDFS-3516 . Check content-type in WebHdfsFileSystem. (Revision 1363000) Result = SUCCESS daryn : http://svn.apache.org/viewcvs.cgi/?root=Apache-SVN&view=rev&rev=1363000 Files : /hadoop/common/branches/branch-0.23/hadoop-hdfs-project/hadoop-hdfs/CHANGES.txt /hadoop/common/branches/branch-0.23/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/hdfs/web/WebHdfsFileSystem.java /hadoop/common/branches/branch-0.23/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/web/TestWebHdfsFileSystemContract.java /hadoop/common/branches/branch-0.23/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/web/WebHdfsTestUtil.java
        Arun C Murthy made changes -
        Fix Version/s 2.0.2-alpha [ 12322472 ]
        Fix Version/s 2.1.0-alpha [ 12321440 ]
        Arun C Murthy made changes -
        Status Resolved [ 5 ] Closed [ 6 ]
        Vinayakumar B made changes -
        Component/s webhdfs [ 12319200 ]

          People

          • Assignee:
            Tsz Wo Nicholas Sze
            Reporter:
            Tsz Wo Nicholas Sze
          • Votes:
            0 Vote for this issue
            Watchers:
            4 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development