Hadoop Common
  1. Hadoop Common
  2. HADOOP-6496

HttpServer sends wrong content-type for CSS files (and others)

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: 0.21.0, 0.22.0
    • Fix Version/s: 1.2.0, 0.22.0, 1-win
    • Component/s: None
    • Labels:
      None
    • Hadoop Flags:
      Reviewed

      Description

      CSS files are send as text/html causing problems if the HTML page is rendered in standards mode. The HDFS interface for example still works because it is rendered in quirks mode, the HBase interface doesn't work because it is rendered in standards mode. See HBASE-2110 for more details.

      I've had a quick look at HttpServer but I'm too unfamiliar with it to see the problem. I think this started happening with HADOOP-6441 which would lead me to believe that the filter is called for every request and not only *.jsp and *.html. I'd consider this a bug but I don't know enough about this to provide a fix.

      1. hadoop-6496.txt
        8 kB
        Todd Lipcon
      2. hadoop-6496.txt
        10 kB
        Todd Lipcon
      3. HADOOP-6496.branch-1.1.backport.patch
        8 kB
        Ivan Mitic
      4. HADOOP-6496.branch-1.1.backport.2.patch
        5 kB
        Ivan Mitic

        Issue Links

          Activity

          Hide
          Matt Foley added a comment -

          Closed upon release of Hadoop 1.2.0.

          Show
          Matt Foley added a comment - Closed upon release of Hadoop 1.2.0.
          Hide
          Suresh Srinivas added a comment -

          I committed the patch to branch-1 and branch-1-win. Thank you Ivan for porting the patch.

          Show
          Suresh Srinivas added a comment - I committed the patch to branch-1 and branch-1-win. Thank you Ivan for porting the patch.
          Hide
          Suresh Srinivas added a comment -

          Thanks for the explanation. +1 for the branch-1 patch.

          Show
          Suresh Srinivas added a comment - Thanks for the explanation. +1 for the branch-1 patch.
          Hide
          Ivan Mitic added a comment -

          Thanks for reviewing Suresh.

          the branch-1.1 patch is missing the addition of file test.css, changes to build.xml, HttpServerFunctionalTest.java. Is that intentional?

          HttpServerFunctionalTest does not exist in branch-1, that is why I don't have any changes around it.
          Other changes (build.xml/test.css) are only related to adding a new test.css test file. However, that introduced unittest regressions in branch-1, since Jetty ended up resolving resources against the local test path build/test/webapps. Specifically, test.classpath would have build/test/webapps before build/webapps causing many tests dependent on files under build/webapps to fail, since HttpServer binds to an incorrect path.

          As an alternative, I used an existing /static/hadoop.css test file. This way we have both the fix and a test covering the new functionality. Let me know if this makes sense or if you have other proposals.

          Show
          Ivan Mitic added a comment - Thanks for reviewing Suresh. the branch-1.1 patch is missing the addition of file test.css, changes to build.xml, HttpServerFunctionalTest.java. Is that intentional? HttpServerFunctionalTest does not exist in branch-1, that is why I don't have any changes around it. Other changes (build.xml/test.css) are only related to adding a new test.css test file. However, that introduced unittest regressions in branch-1, since Jetty ended up resolving resources against the local test path build/test/webapps . Specifically, test.classpath would have build/test/webapps before build/webapps causing many tests dependent on files under build/webapps to fail, since HttpServer binds to an incorrect path. As an alternative, I used an existing /static/hadoop.css test file. This way we have both the fix and a test covering the new functionality. Let me know if this makes sense or if you have other proposals.
          Hide
          Suresh Srinivas added a comment -

          Ivan, the branch-1.1 patch is missing the addition of file test.css, changes to build.xml, HttpServerFunctionalTest.java. Is that intentional?

          Show
          Suresh Srinivas added a comment - Ivan, the branch-1.1 patch is missing the addition of file test.css, changes to build.xml, HttpServerFunctionalTest.java. Is that intentional?
          Hide
          Ivan Mitic added a comment -

          Attaching the patch for branch-1.1.

          Show
          Ivan Mitic added a comment - Attaching the patch for branch-1.1.
          Hide
          Ivan Mitic added a comment -

          Reopening for branch 1.1 backport.

          Show
          Ivan Mitic added a comment - Reopening for branch 1.1 backport.
          Hide
          Ivan Mitic added a comment -

          Assigning to myself to prepare the backport patch. The initial backport patch I attached introduced unit test regression, will post a new patch soon.

          Show
          Ivan Mitic added a comment - Assigning to myself to prepare the backport patch. The initial backport patch I attached introduced unit test regression, will post a new patch soon.
          Hide
          Ivan Mitic added a comment -

          We need to backport this to branch-1.1 as it blocks the fix for HADOOP-7827. Attaching the patch.

          Please see comments on HADOOP-7827 for additional details.

          Show
          Ivan Mitic added a comment - We need to backport this to branch-1.1 as it blocks the fix for HADOOP-7827 . Attaching the patch. Please see comments on HADOOP-7827 for additional details.
          Hide
          Hudson added a comment -

          Integrated in Hadoop-Common-trunk #514 (See https://hudson.apache.org/hudson/job/Hadoop-Common-trunk/514/)
          HADOOP-6496. HttpServer sends wrong content-type for CSS files (and others). Contributed by Todd Lipcon.

          Show
          Hudson added a comment - Integrated in Hadoop-Common-trunk #514 (See https://hudson.apache.org/hudson/job/Hadoop-Common-trunk/514/ ) HADOOP-6496 . HttpServer sends wrong content-type for CSS files (and others). Contributed by Todd Lipcon.
          Hide
          Hudson added a comment -

          Integrated in Hadoop-Common-trunk-Commit #426 (See https://hudson.apache.org/hudson/job/Hadoop-Common-trunk-Commit/426/)
          HADOOP-6496. HttpServer sends wrong content-type for CSS files (and others). Contributed by Todd Lipcon.

          Show
          Hudson added a comment - Integrated in Hadoop-Common-trunk-Commit #426 (See https://hudson.apache.org/hudson/job/Hadoop-Common-trunk-Commit/426/ ) HADOOP-6496 . HttpServer sends wrong content-type for CSS files (and others). Contributed by Todd Lipcon.
          Hide
          Tom White added a comment -

          I've just committed this. Thanks Todd!

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

          +1 looks good.

          Show
          Tom White added a comment - +1 looks good.
          Hide
          Hadoop QA added a comment -

          +1 overall. Here are the results of testing the latest attachment
          http://issues.apache.org/jira/secure/attachment/12458948/hadoop-6496.txt
          against trunk revision 1031743.

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

          +1 tests included. The patch appears to include 15 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.

          +1 system test framework. The patch passed system test framework compile.

          Test results: https://hudson.apache.org/hudson/job/PreCommit-HADOOP-Build/76//testReport/
          Findbugs warnings: https://hudson.apache.org/hudson/job/PreCommit-HADOOP-Build/76//artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html
          Console output: https://hudson.apache.org/hudson/job/PreCommit-HADOOP-Build/76//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/12458948/hadoop-6496.txt against trunk revision 1031743. +1 @author. The patch does not contain any @author tags. +1 tests included. The patch appears to include 15 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. +1 system test framework. The patch passed system test framework compile. Test results: https://hudson.apache.org/hudson/job/PreCommit-HADOOP-Build/76//testReport/ Findbugs warnings: https://hudson.apache.org/hudson/job/PreCommit-HADOOP-Build/76//artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html Console output: https://hudson.apache.org/hudson/job/PreCommit-HADOOP-Build/76//console This message is automatically generated.
          Hide
          Todd Lipcon added a comment -

          Add license headers to the two new files

          Show
          Todd Lipcon added a comment - Add license headers to the two new files
          Hide
          Hadoop QA added a comment -

          -1 overall. Here are the results of testing the latest attachment
          http://issues.apache.org/jira/secure/attachment/12455942/hadoop-6496.txt
          against trunk revision 1031422.

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

          +1 tests included. The patch appears to include 15 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 generated 3 release audit warnings (more than the trunk's current 1 warnings).

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

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

          +1 system test framework. The patch passed system test framework compile.

          Test results: https://hudson.apache.org/hudson/job/PreCommit-HADOOP-Build/63//testReport/
          Release audit warnings: https://hudson.apache.org/hudson/job/PreCommit-HADOOP-Build/63//artifact/trunk/patchprocess/patchReleaseAuditProblems.txt
          Findbugs warnings: https://hudson.apache.org/hudson/job/PreCommit-HADOOP-Build/63//artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html
          Console output: https://hudson.apache.org/hudson/job/PreCommit-HADOOP-Build/63//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/12455942/hadoop-6496.txt against trunk revision 1031422. +1 @author. The patch does not contain any @author tags. +1 tests included. The patch appears to include 15 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 generated 3 release audit warnings (more than the trunk's current 1 warnings). +1 core tests. The patch passed core unit tests. +1 contrib tests. The patch passed contrib unit tests. +1 system test framework. The patch passed system test framework compile. Test results: https://hudson.apache.org/hudson/job/PreCommit-HADOOP-Build/63//testReport/ Release audit warnings: https://hudson.apache.org/hudson/job/PreCommit-HADOOP-Build/63//artifact/trunk/patchprocess/patchReleaseAuditProblems.txt Findbugs warnings: https://hudson.apache.org/hudson/job/PreCommit-HADOOP-Build/63//artifact/trunk/build/test/findbugs/newPatchFindbugsWarnings.html Console output: https://hudson.apache.org/hudson/job/PreCommit-HADOOP-Build/63//console This message is automatically generated.
          Hide
          Steve Loughran added a comment -

          Thinking about this some more

          1. the HttpServer could add a basic set of mime mappings
          2. We could use the test/ webapp of HADOOP-6461 to stick some html, xml, css, txt content into the test classpath of -common
          3. then test w/ HtmlUnit. A test-time only dependency. I've used HtmlUnit before, very nice.
          Show
          Steve Loughran added a comment - Thinking about this some more the HttpServer could add a basic set of mime mappings We could use the test/ webapp of HADOOP-6461 to stick some html, xml, css, txt content into the test classpath of -common then test w/ HtmlUnit. A test-time only dependency. I've used HtmlUnit before, very nice.
          Hide
          Steve Loughran added a comment -

          Looking at my Jetty code, I see this code to set mime mappings.
          public void addMimeMapping(String extension, String mimeType)

          { log.info("Adding mime mapping " + extension + " maps to " + mimeType); MimeTypes mimes = getServletContext().getMimeTypes(); mimes.addMimeMapping(extension, mimeType); }
          
          

          Maybe the filter could look for text/html and text/plain content types in the response and only change the encoding value if it matches these types.

          Show
          Steve Loughran added a comment - Looking at my Jetty code, I see this code to set mime mappings. public void addMimeMapping(String extension, String mimeType) { log.info("Adding mime mapping " + extension + " maps to " + mimeType); MimeTypes mimes = getServletContext().getMimeTypes(); mimes.addMimeMapping(extension, mimeType); } Maybe the filter could look for text/html and text/plain content types in the response and only change the encoding value if it matches these types.
          Hide
          Owen O'Malley added a comment -

          There was a security concern about the encoding being unspecified resulting in some browsers (namely older versions of IE) doing some bad unquoting, so HADOOP-6441 set the default content type to text/html. Unfortunately, Jetty doesn't seem to let me set a default for the encoding and so I needed to set the content type to "text/html;charset=utf-8". I guess we could have the filter look at the name of the request and special case it for urls that end in ".css". Is there a better approach?

          Show
          Owen O'Malley added a comment - There was a security concern about the encoding being unspecified resulting in some browsers (namely older versions of IE) doing some bad unquoting, so HADOOP-6441 set the default content type to text/html. Unfortunately, Jetty doesn't seem to let me set a default for the encoding and so I needed to set the content type to "text/html;charset=utf-8". I guess we could have the filter look at the name of the request and special case it for urls that end in ".css". Is there a better approach?

            People

            • Assignee:
              Ivan Mitic
              Reporter:
              Lars Francke
            • Votes:
              0 Vote for this issue
              Watchers:
              6 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development