svn commit: r1064445 - in /shindig/trunk/java/gadgets/src: main/java/org/apache/shindig/gadgets/servlet/ProxyHandler.java test/java/org/apache/shindig/gadgets/servlet/ProxyHandlerTest.java

[li...@apache.org]

Author: lindner
Date: Fri Jan 28 03:19:33 2011
New Revision: 1064445

URL: http://svn.apache.org/viewvc?rev=1064445&view=rev
Log:
SHINDIG-1494 | Modified Patch from Mark Weitzel |  ProxyServlet special case for Flash files is not always effective

Modified:
    shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/ProxyHandler.java
    shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/servlet/ProxyHandlerTest.java

Modified: shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/ProxyHandler.java
URL: http://svn.apache.org/viewvc/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/ProxyHandler.java?rev=1064445&r1=1064444&r2=1064445&view=diff
==============================================================================
--- shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/ProxyHandler.java (original)
+++ shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/ProxyHandler.java Fri Jan 28 03:19:33 2011
@@ -24,6 +24,7 @@ import com.google.inject.Singleton;
 import com.google.inject.name.Named;
 
 import org.apache.commons.io.IOUtils;
+import org.apache.commons.lang.StringUtils;
 import org.apache.shindig.common.uri.Uri;
 import org.apache.shindig.gadgets.GadgetException;
 import org.apache.shindig.gadgets.http.HttpRequest;
@@ -141,8 +142,7 @@ public class ProxyHandler {
     // We're skipping the content disposition header for flash due to an issue with Flash player 10
     // This does make some sites a higher value phishing target, but this can be mitigated by
     // additional referer checks.
-    if (!"application/x-shockwave-flash".equalsIgnoreCase(results.getHeader("Content-Type")) &&
-        !"application/x-shockwave-flash".equalsIgnoreCase(response.getHeader("Content-Type"))) {
+    if (!isFlash(response.getHeader("Content-Type"), results.getHeader("Content-Type"))) {
       response.setHeader("Content-Disposition", "attachment;filename=p.txt");
     }
     if (results.getHeader("Content-Type") == null) {
@@ -150,6 +150,20 @@ public class ProxyHandler {
     }
   }
 
+  private static final String FLASH_CONTENT_TYPE = "application/x-shockwave-flash";
+
+  /**
+   * Test for presence of flash
+   *
+   * @param responseContentType the Content-Type header from the HttpResponseBuilder
+   * @param resultsContentType the Content-Type header from the HttpResponse
+   * @return true if either content type matches that of Flash
+   */
+  private boolean isFlash(String responseContentType, String resultsContentType) {
+    return StringUtils.startsWithIgnoreCase(responseContentType, FLASH_CONTENT_TYPE)
+        || StringUtils.startsWithIgnoreCase(resultsContentType, FLASH_CONTENT_TYPE);
+  }
+
   /**
    * Returns true in case the error encountered while rewriting the content
    * is recoverable. The rationale behind it is that errors should be thrown

Modified: shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/servlet/ProxyHandlerTest.java
URL: http://svn.apache.org/viewvc/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/servlet/ProxyHandlerTest.java?rev=1064445&r1=1064444&r2=1064445&view=diff
==============================================================================
--- shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/servlet/ProxyHandlerTest.java (original)
+++ shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/servlet/ProxyHandlerTest.java Fri Jan 28 03:19:33 2011
@@ -19,6 +19,7 @@
 package org.apache.shindig.gadgets.servlet;
 
 import com.google.common.base.Objects;
+import com.google.common.collect.ImmutableMap;
 import com.google.common.collect.Maps;
 
 import org.apache.shindig.common.EasyMockTestCase;
@@ -224,11 +225,20 @@ public class ProxyHandlerTest extends Ea
 
   @Test
   public void testNoContentDispositionForFlash() throws Exception {
+    assertNoContentDispositionForFlash("application/x-shockwave-flash");
+  }
+
+  @Test
+  public void testNoContentDispositionForFlashUtf8() throws Exception {
+    assertNoContentDispositionForFlash("application/x-shockwave-flash;charset=utf-8");
+  }
+
+  private void assertNoContentDispositionForFlash(String contentType) throws Exception {
     // Some headers may be blacklisted. These are OK.
     String url = "http://example.org/file.evil";
     String domain = "example.org";
-    Map<String, List<String>> headers = Maps.newHashMap();
-    headers.put("Content-Type", Arrays.asList("application/x-shockwave-flash"));
+    Map<String, List<String>> headers =
+        ImmutableMap.of("Content-Type", Arrays.asList(contentType));
 
     setupNoArgsProxyRequestMock(domain, url);
     expectGetAndReturnHeaders(url, headers);
@@ -237,7 +247,7 @@ public class ProxyHandlerTest extends Ea
     HttpResponse response = proxyHandler.fetch(request);
     verify();
 
-    assertEquals("application/x-shockwave-flash", response.getHeader("Content-Type"));
+    assertEquals(contentType, response.getHeader("Content-Type"));
     assertNull(response.getHeader("Content-Disposition"));
     assertTrue(rewriter.responseWasRewritten());
   }