From f7105a2773048be8229b80ca74a889c9b15348b6 Mon Sep 17 00:00:00 2001
From: Thomas White <tomwhite@apache.org>
Date: Mon, 15 Nov 2010 17:01:23 +0000
Subject: [PATCH] HADOOP-6496. HttpServer sends wrong content-type for CSS
 files (and others). Contributed by Todd Lipcon.

git-svn-id: https://svn.apache.org/repos/asf/hadoop/common/trunk@1035353 13f79535-47bb-0310-9956-ffa450edef68
---
 CHANGES.txt                                   |  3 ++
 build.xml                                     |  7 ++--
 .../org/apache/hadoop/http/HttpServer.java    | 33 ++++++++++++++++---
 .../hadoop/http/HttpServerFunctionalTest.java | 10 ++----
 .../apache/hadoop/http/TestHttpServer.java    | 25 ++++++++++++++
 src/test/test-webapps/static/test.css         | 21 ++++++++++++
 src/test/test-webapps/test/.gitignore         | 14 ++++++++
 7 files changed, 99 insertions(+), 14 deletions(-)
 create mode 100644 src/test/test-webapps/static/test.css
 create mode 100644 src/test/test-webapps/test/.gitignore

diff --git a/CHANGES.txt b/CHANGES.txt
index 17c96c4f39..1b2cf168cb 100644
--- a/CHANGES.txt
+++ b/CHANGES.txt
@@ -328,6 +328,9 @@ Trunk (unreleased changes)
     HADOOP-6899 RawLocalFileSystem#setWorkingDir() does not work for relative names
      (Sanjay Radia)
 
+    HADOOP-6496. HttpServer sends wrong content-type for CSS files
+    (and others). (Todd Lipcon via tomwhite)
+
 Release 0.21.1 - Unreleased
 
   IMPROVEMENTS
diff --git a/build.xml b/build.xml
index 5418939b4a..5218a0d38c 100644
--- a/build.xml
+++ b/build.xml
@@ -79,7 +79,7 @@
   <property name="test.build.extraconf" value="${test.build.dir}/extraconf"/>
   <property name="test.build.javadoc" value="${test.build.dir}/docs/api"/>
   <property name="test.build.javadoc.dev" value="${test.build.dir}/docs/dev-api"/>
-  <property name="test.build.webapps" value="${build.dir}/webapps"/>
+  <property name="test.build.webapps" value="${build.dir}/test/webapps"/>
   <property name="test.include" value="Test*"/>
   <property name="test.classpath.id" value="test.classpath"/>
   <property name="test.output" value="no"/>
@@ -206,6 +206,7 @@
     <pathelement location="${test.build.extraconf}"/>
     <pathelement location="${test.core.build.classes}" />
     <pathelement location="${test.src.dir}"/>
+    <pathelement location="${test.build.dir}"/>
     <pathelement location="${build.dir}"/>
     <pathelement location="${build.examples}"/>
     <pathelement location="${build.tools}"/>
@@ -661,7 +662,9 @@
       <delete dir="@{test.dir}/data" />
       <mkdir dir="@{test.dir}/data" />
       <delete dir="${test.build.webapps}"/>
-      <mkdir dir="${test.build.webapps}/test" />
+      <copy todir="${test.build.webapps}">
+        <fileset dir="${test.src.dir}/test-webapps" includes="**/*" />
+      </copy>
       <delete dir="@{test.dir}/logs" />
       <mkdir dir="@{test.dir}/logs" />
       <copy file="${test.src.dir}/hadoop-policy.xml"
diff --git a/src/java/org/apache/hadoop/http/HttpServer.java b/src/java/org/apache/hadoop/http/HttpServer.java
index 5906b1fd44..ddab4552c8 100644
--- a/src/java/org/apache/hadoop/http/HttpServer.java
+++ b/src/java/org/apache/hadoop/http/HttpServer.java
@@ -58,9 +58,12 @@
 import org.apache.hadoop.security.Krb5AndCertsSslSocketConnector.MODE;
 import org.apache.hadoop.security.authorize.AccessControlList;
 import org.apache.hadoop.util.ReflectionUtils;
+import org.mortbay.io.Buffer;
 import org.mortbay.jetty.Connector;
 import org.mortbay.jetty.Handler;
+import org.mortbay.jetty.MimeTypes;
 import org.mortbay.jetty.Server;
+import org.mortbay.jetty.handler.ContextHandler;
 import org.mortbay.jetty.handler.ContextHandlerCollection;
 import org.mortbay.jetty.nio.SelectChannelConnector;
 import org.mortbay.jetty.security.SslSocketConnector;
@@ -750,6 +753,7 @@ public void doGet(HttpServletRequest request, HttpServletResponse response)
    * all of the servlets resistant to cross-site scripting attacks.
    */
   public static class QuotingInputFilter implements Filter {
+    private FilterConfig config;
 
     public static class RequestQuoter extends HttpServletRequestWrapper {
       private final HttpServletRequest rawRequest;
@@ -837,6 +841,7 @@ public String getServerName() {
 
     @Override
     public void init(FilterConfig config) throws ServletException {
+      this.config = config;
     }
 
     @Override
@@ -850,11 +855,29 @@ public void doFilter(ServletRequest request,
                          ) throws IOException, ServletException {
       HttpServletRequestWrapper quoted = 
         new RequestQuoter((HttpServletRequest) request);
-      final HttpServletResponse httpResponse = (HttpServletResponse) response;
-      // set the default to UTF-8 so that we don't need to worry about IE7
-      // choosing to interpret the special characters as UTF-7
-      httpResponse.setContentType("text/html;charset=utf-8");
-      chain.doFilter(quoted, response);
+      HttpServletResponse httpResponse = (HttpServletResponse) response;
+
+      String mime = inferMimeType(request);
+      if (mime == null || mime.equals("text/html")) {
+        // no extension or HTML with unspecified encoding, we want to
+        // force HTML with utf-8 encoding
+        // This is to avoid the following security issue:
+        // http://openmya.hacker.jp/hasegawa/security/utf7cs.html
+        httpResponse.setContentType("text/html; charset=utf-8");
+      }
+      chain.doFilter(quoted, httpResponse);
+    }
+
+    /**
+     * Infer the mime type for the response based on the extension of the request
+     * URI. Returns null if unknown.
+     */
+    private String inferMimeType(ServletRequest request) {
+      String path = ((HttpServletRequest)request).getRequestURI();
+      ContextHandler.SContext sContext = (ContextHandler.SContext)config.getServletContext();
+      MimeTypes mimes = sContext.getContextHandler().getMimeTypes();
+      Buffer mimeBuffer = mimes.getMimeByExtension(path);
+      return (mimeBuffer == null) ? null : mimeBuffer.toString();
     }
 
   }
diff --git a/src/test/core/org/apache/hadoop/http/HttpServerFunctionalTest.java b/src/test/core/org/apache/hadoop/http/HttpServerFunctionalTest.java
index e5822551b3..07688137d5 100644
--- a/src/test/core/org/apache/hadoop/http/HttpServerFunctionalTest.java
+++ b/src/test/core/org/apache/hadoop/http/HttpServerFunctionalTest.java
@@ -36,7 +36,7 @@ public class HttpServerFunctionalTest extends Assert {
   /** JVM property for the webapp test dir : {@value} */
   public static final String TEST_BUILD_WEBAPPS = "test.build.webapps";
   /** expected location of the test.build.webapps dir: {@value} */
-  private static final String BUILD_WEBAPPS_DIR = "build/webapps";
+  private static final String BUILD_WEBAPPS_DIR = "build/test/webapps";
   
   /** name of the test webapp: {@value} */
   private static final String TEST = "test";
@@ -78,13 +78,9 @@ public static HttpServer createTestServer(Configuration conf)
   protected static void prepareTestWebapp() {
     String webapps = System.getProperty(TEST_BUILD_WEBAPPS, BUILD_WEBAPPS_DIR);
     File testWebappDir = new File(webapps +
-        File.pathSeparator + TEST);
+        File.separatorChar + TEST);
     if (!testWebappDir.exists()) {
-      assertTrue("Unable to create the test dir " + testWebappDir,
-          testWebappDir.mkdirs());
-    } else {
-      assertTrue("Not a directory " + testWebappDir,
-          testWebappDir.isDirectory());
+      fail("Test webapp dir " + testWebappDir + " missing");
     }
   }
 
diff --git a/src/test/core/org/apache/hadoop/http/TestHttpServer.java b/src/test/core/org/apache/hadoop/http/TestHttpServer.java
index c2c4194c40..71ceebcc85 100644
--- a/src/test/core/org/apache/hadoop/http/TestHttpServer.java
+++ b/src/test/core/org/apache/hadoop/http/TestHttpServer.java
@@ -19,6 +19,7 @@
 
 import java.io.IOException;
 import java.io.PrintStream;
+import java.net.URLConnection;
 import java.net.HttpURLConnection;
 import java.net.URL;
 import java.util.Arrays;
@@ -167,6 +168,30 @@ public void run() {
                  readOutput(new URL(baseUrl, "/echomap?a=b&c<=d&a=>")));
   }
 
+  @Test public void testContentTypes() throws Exception {
+    // Static CSS files should have text/css
+    URL cssUrl = new URL(baseUrl, "/static/test.css");
+    HttpURLConnection conn = (HttpURLConnection)cssUrl.openConnection();
+    conn.connect();
+    assertEquals(200, conn.getResponseCode());
+    assertEquals("text/css", conn.getContentType());
+
+    // Servlets should have text/html with proper encoding
+    URL servletUrl = new URL(baseUrl, "/echo?a=b");
+    conn = (HttpURLConnection)servletUrl.openConnection();
+    conn.connect();
+    assertEquals(200, conn.getResponseCode());
+    assertEquals("text/html; charset=utf-8", conn.getContentType());
+
+    // We should ignore parameters for mime types - ie a parameter
+    // ending in .css should not change mime type
+    servletUrl = new URL(baseUrl, "/echo?a=b.css");
+    conn = (HttpURLConnection)servletUrl.openConnection();
+    conn.connect();
+    assertEquals(200, conn.getResponseCode());
+    assertEquals("text/html; charset=utf-8", conn.getContentType());
+  }
+
   /**
    * Dummy filter that mimics as an authentication filter. Obtains user identity
    * from the request parameter user.name. Wraps around the request so that
diff --git a/src/test/test-webapps/static/test.css b/src/test/test-webapps/static/test.css
new file mode 100644
index 0000000000..ae4382869a
--- /dev/null
+++ b/src/test/test-webapps/static/test.css
@@ -0,0 +1,21 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+/**
+ * Test CSS file for content type handling - empty, since we just check
+ * returned content type!
+ */
diff --git a/src/test/test-webapps/test/.gitignore b/src/test/test-webapps/test/.gitignore
new file mode 100644
index 0000000000..ae1e83eeb3
--- /dev/null
+++ b/src/test/test-webapps/test/.gitignore
@@ -0,0 +1,14 @@
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements.  See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License.  You may obtain a copy of the License at
+#
+#     http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.