You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@shindig.apache.org by Adam Winer <aw...@google.com> on 2009/08/05 01:39:35 UTC

Re: svn commit: r801008 - in /incubator/shindig/trunk: config/ features/src/main/javascript/features/core.io/ features/src/main/javascript/features/core/ features/src/main/javascript/features/flash/ java/gadgets/src/main/java/org/apache/shindig/gadge

Moving the content-disposition setting from ProxyBase to ProxyHandler
effectively makes MakeRequestHandler an open proxy.  Sooo.... some
other fix is needed.

On Tue, Aug 4, 2009 at 3:48 PM, <jo...@apache.org> wrote:
> Author: johnh
> Date: Tue Aug  4 22:48:08 2009
> New Revision: 801008
>
> URL: http://svn.apache.org/viewvc?rev=801008&view=rev
> Log:
> * Adds support for gadgets.io.getCachedUrl(...) options to include {
> rewriteMime: "mimeType" }.
>
> * Uses this support in gadgets.flash.embedCachedFlash(...) to default mimeType
> to application/x-shockwave-flash, in order to force this mime-type in serving
> SWFs, even from servers that don't emit the proper headers for them (which
> breaks these on most all browsers)
>
> * Moves Content-Disposition removal logic for application/x-shockwave-flash
> content to ProxyHandler, since that's where HttpRequest, which has rewriteMime
> access, is read and processed.
>
> * Updates tests accordingly.
>
>
> Modified:
>    incubator/shindig/trunk/config/container.js
>    incubator/shindig/trunk/features/src/main/javascript/features/core.io/io.js
>    incubator/shindig/trunk/features/src/main/javascript/features/core/legacy.js
>    incubator/shindig/trunk/features/src/main/javascript/features/flash/flash.js
>    incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/ProxyBase.java
>    incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/ProxyHandler.java
>    incubator/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/servlet/ProxyBaseTest.java
>    incubator/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/servlet/ProxyHandlerTest.java
>
> Modified: incubator/shindig/trunk/config/container.js
> URL: http://svn.apache.org/viewvc/incubator/shindig/trunk/config/container.js?rev=801008&r1=801007&r2=801008&view=diff
> ==============================================================================
> --- incubator/shindig/trunk/config/container.js (original)
> +++ incubator/shindig/trunk/config/container.js Tue Aug  4 22:48:08 2009
> @@ -96,7 +96,7 @@
>  "gadgets.features" : {
>   "core.io" : {
>     // Note: /proxy is an open proxy. Be careful how you expose this!
> -    "proxyUrl" : "http://%host%/gadgets/proxy?refresh=%refresh%&url=%url%",
> +    "proxyUrl" : "http://%host%/gadgets/proxy?refresh=%refresh%&url=%url%&%rewriteMime%",
>     "jsonProxyUrl" : "http://%host%/gadgets/makeRequest"
>   },
>   "views" : {
>
> Modified: incubator/shindig/trunk/features/src/main/javascript/features/core.io/io.js
> URL: http://svn.apache.org/viewvc/incubator/shindig/trunk/features/src/main/javascript/features/core.io/io.js?rev=801008&r1=801007&r2=801008&view=diff
> ==============================================================================
> --- incubator/shindig/trunk/features/src/main/javascript/features/core.io/io.js (original)
> +++ incubator/shindig/trunk/features/src/main/javascript/features/core.io/io.js Tue Aug  4 22:48:08 2009
> @@ -470,12 +470,15 @@
>
>       var urlParams = gadgets.util.getUrlParameters();
>
> +      var rewriteMimeParam =
> +          params.rewriteMime ? "rewriteMime=" + encodeURIComponent(params.rewriteMime) : "";
>       return config.proxyUrl.replace("%url%", encodeURIComponent(url)).
>           replace("%host%", document.location.host).
>           replace("%rawurl%", url).
>           replace("%refresh%", encodeURIComponent(refresh)).
>           replace("%gadget%", encodeURIComponent(urlParams.url)).
> -          replace("%container%", encodeURIComponent(urlParams.container || urlParams.synd));
> +          replace("%container%", encodeURIComponent(urlParams.container || urlParams.synd)).
> +          replace("%rewriteMime%", rewriteMimeParam);
>     }
>   };
>  }();
>
> Modified: incubator/shindig/trunk/features/src/main/javascript/features/core/legacy.js
> URL: http://svn.apache.org/viewvc/incubator/shindig/trunk/features/src/main/javascript/features/core/legacy.js?rev=801008&r1=801007&r2=801008&view=diff
> ==============================================================================
> --- incubator/shindig/trunk/features/src/main/javascript/features/core/legacy.js (original)
> +++ incubator/shindig/trunk/features/src/main/javascript/features/core/legacy.js Tue Aug  4 22:48:08 2009
> @@ -124,9 +124,10 @@
>  }
>
>  function _IG_GetCachedUrl(url, opt_params) {
> -  var params = { 'REFRESH_INTERVAL': 3600 };
> -  if (opt_params && opt_params.refreshInterval) {
> -    params['REFRESH_INTERVAL'] = opt_params.refreshInterval;
> +  var params = opt_params || {};
> +  params['REFRESH_INTERVAL'] = 3600;
> +  if (params.refreshInterval) {
> +    params['REFRESH_INTERVAL'] = params.refreshInterval;
>   }
>   return gadgets.io.getProxyUrl(url, params);
>  }
>
> Modified: incubator/shindig/trunk/features/src/main/javascript/features/flash/flash.js
> URL: http://svn.apache.org/viewvc/incubator/shindig/trunk/features/src/main/javascript/features/flash/flash.js?rev=801008&r1=801007&r2=801008&view=diff
> ==============================================================================
> --- incubator/shindig/trunk/features/src/main/javascript/features/flash/flash.js (original)
> +++ incubator/shindig/trunk/features/src/main/javascript/features/flash/flash.js Tue Aug  4 22:48:08 2009
> @@ -190,7 +190,7 @@
>  */
>  gadgets.flash.embedCachedFlash = function() {
>   var args = Array.prototype.slice.call(arguments);
> -  args[0] = gadgets.io.getProxyUrl(args[0]);
> +  args[0] = gadgets.io.getProxyUrl(args[0], { rewriteMime: "application/x-shockwave-flash" });
>   return gadgets.flash.embedFlash.apply(this, args);
>  };
>
>
> Modified: incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/ProxyBase.java
> URL: http://svn.apache.org/viewvc/incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/ProxyBase.java?rev=801008&r1=801007&r2=801008&view=diff
> ==============================================================================
> --- incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/ProxyBase.java (original)
> +++ incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/ProxyBase.java Tue Aug  4 22:48:08 2009
> @@ -131,12 +131,6 @@
>       refreshInterval = Math.max(60 * 60, (int)(results.getCacheTtl() / 1000L));
>     }
>     HttpUtil.setCachingHeaders(response, refreshInterval);
> -    // 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"))) {
> -      response.setHeader("Content-Disposition", "attachment;filename=p.txt");
> -    }
>   }
>
>   /**
>
> Modified: incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/ProxyHandler.java
> URL: http://svn.apache.org/viewvc/incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/ProxyHandler.java?rev=801008&r1=801007&r2=801008&view=diff
> ==============================================================================
> --- incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/ProxyHandler.java (original)
> +++ incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/ProxyHandler.java Tue Aug  4 22:48:08 2009
> @@ -153,21 +153,30 @@
>       }
>     }
>
> +    String responseType = results.getHeader("Content-Type");
>     if (!StringUtils.isEmpty(rcr.getRewriteMimeType())) {
>       String requiredType = rcr.getRewriteMimeType();
> -      String responseType = results.getHeader("Content-Type");
>       // Use a 'Vary' style check on the response
>       if (requiredType.endsWith("/*") &&
>           !StringUtils.isEmpty(responseType)) {
>         requiredType = requiredType.substring(0, requiredType.length() - 2);
>         if (!responseType.toLowerCase().startsWith(requiredType.toLowerCase())) {
>           response.setContentType(requiredType);
> +          responseType = requiredType;
>         }
>       } else {
>         response.setContentType(requiredType);
> +        responseType = requiredType;
>       }
>     }
>
> +    // 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(responseType)) {
> +      response.setHeader("Content-Disposition", "attachment;filename=p.txt");
> +    }
> +
>     if (results.getHttpStatusCode() != HttpResponse.SC_OK) {
>       response.sendError(results.getHttpStatusCode());
>     }
>
> Modified: incubator/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/servlet/ProxyBaseTest.java
> URL: http://svn.apache.org/viewvc/incubator/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/servlet/ProxyBaseTest.java?rev=801008&r1=801007&r2=801008&view=diff
> ==============================================================================
> --- incubator/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/servlet/ProxyBaseTest.java (original)
> +++ incubator/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/servlet/ProxyBaseTest.java Tue Aug  4 22:48:08 2009
> @@ -146,7 +146,6 @@
>     // Just verify that they were set. Specific values are configurable.
>     assertNotNull("Expires header not set", recorder.getHeader("Expires"));
>     assertNotNull("Cache-Control header not set", recorder.getHeader("Cache-Control"));
> -    assertEquals("attachment;filename=p.txt", recorder.getHeader("Content-Disposition"));
>   }
>
>   public void testSetResponseHeadersForFlash() throws Exception {
> @@ -161,8 +160,6 @@
>     // Just verify that they were set. Specific values are configurable.
>     assertNotNull("Expires header not set", recorder.getHeader("Expires"));
>     assertNotNull("Cache-Control header not set", recorder.getHeader("Cache-Control"));
> -    assertNull("Content-Disposition header set for flash",
> -        recorder.getHeader("Content-Disposition"));
>   }
>
>   public void testSetResponseHeadersNoCache() throws Exception {
> @@ -179,7 +176,6 @@
>     assertNotNull("Expires header not set", recorder.getHeader("Expires"));
>     assertEquals("no-cache", recorder.getHeader("Pragma"));
>     assertEquals("no-cache", recorder.getHeader("Cache-Control"));
> -    assertEquals("attachment;filename=p.txt", recorder.getHeader("Content-Disposition"));
>   }
>
>   public void testSetResponseHeadersForceParam() throws Exception {
> @@ -190,7 +186,6 @@
>     proxy.setResponseHeaders(request, recorder, results);
>
>     HttpUtilTest.checkCacheControlHeaders(HttpUtilTest.testStartTime, recorder, 30, false);
> -    assertEquals("attachment;filename=p.txt", recorder.getHeader("Content-Disposition"));
>   }
>
>   public void testSetResponseHeadersForceParamInvalid() throws Exception {
>
> Modified: incubator/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/servlet/ProxyHandlerTest.java
> URL: http://svn.apache.org/viewvc/incubator/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/servlet/ProxyHandlerTest.java?rev=801008&r1=801007&r2=801008&view=diff
> ==============================================================================
> --- incubator/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/servlet/ProxyHandlerTest.java (original)
> +++ incubator/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/servlet/ProxyHandlerTest.java Tue Aug  4 22:48:08 2009
> @@ -87,6 +87,7 @@
>     verify();
>
>     assertEquals(DATA_ONE, recorder.getResponseAsString());
> +    assertEquals("attachment;filename=p.txt", recorder.getHeader("Content-Disposition"));
>     assertTrue(rewriter.responseWasRewritten());
>   }
>
> @@ -108,6 +109,7 @@
>     assertEquals(Uri.parse(URL_ONE), httpRequest.getValue().getUri());
>
>     assertEquals(DATA_ONE, recorder.getResponseAsString());
> +    assertEquals("attachment;filename=p.txt", recorder.getHeader("Content-Disposition"));
>     assertTrue(rewriter.responseWasRewritten());
>   }
>
> @@ -143,6 +145,26 @@
>
>     assertEquals(contentType, recorder.getHeader("Content-Type"));
>     assertEquals(magicGarbage, recorder.getHeader("X-Magic-Garbage"));
> +    assertEquals("attachment;filename=p.txt", recorder.getHeader("Content-Disposition"));
> +    assertTrue(rewriter.responseWasRewritten());
> +  }
> +
> +  public void testFlashGetsNoContentDisposition() throws Exception {
> +    String url = "http://example.org/swiff.swf";
> +    String domain = "example.org";
> +    String contentType = "application/x-shockwave-flash";
> +    Map<String, List<String>> headers = Maps.newHashMap();
> +    headers.put("Content-Type", Arrays.asList(contentType));
> +
> +    expect(lockedDomainService.isSafeForOpenProxy(domain)).andReturn(true).atLeastOnce();
> +    setupProxyRequestMock(domain, url);
> +    expectGetAndReturnHeaders(url, headers);
> +
> +    replay();
> +
> +    proxyHandler.fetch(request, recorder);
> +    assertEquals(contentType, recorder.getHeader("Content-Type"));
> +    assertNull("Content-disposition set for flash", recorder.getHeader("Content-Disposition"));
>     assertTrue(rewriter.responseWasRewritten());
>   }
>
>
>
>

Re: svn commit: r801008 - in /incubator/shindig/trunk: config/ features/src/main/javascript/features/core.io/ features/src/main/javascript/features/core/ features/src/main/javascript/features/flash/ java/gadgets/src/main/java/org/apache/shindig/gadge

Posted by John Hjelmstad <fa...@google.com>.
You're right, thanks for the catch. That's a fairly unappealing oversight.
In some respects I
prefer to set Content-Disposition in both ProxyHandler and
MakeRequestHandler, but on consideration I think it's better the logic
be
restored to ProxyBase.

Fix: http://codereview.appspot.com/100050

On Tue, Aug 4, 2009 at 4:39 PM, Adam Winer <aw...@google.com> wrote:

> Moving the content-disposition setting from ProxyBase to ProxyHandler
> effectively makes MakeRequestHandler an open proxy.  Sooo.... some
> other fix is needed.
>
> On Tue, Aug 4, 2009 at 3:48 PM, <jo...@apache.org> wrote:
> > Author: johnh
> > Date: Tue Aug  4 22:48:08 2009
> > New Revision: 801008
> >
> > URL: http://svn.apache.org/viewvc?rev=801008&view=rev
> > Log:
> > * Adds support for gadgets.io.getCachedUrl(...) options to include {
> > rewriteMime: "mimeType" }.
> >
> > * Uses this support in gadgets.flash.embedCachedFlash(...) to default
> mimeType
> > to application/x-shockwave-flash, in order to force this mime-type in
> serving
> > SWFs, even from servers that don't emit the proper headers for them
> (which
> > breaks these on most all browsers)
> >
> > * Moves Content-Disposition removal logic for
> application/x-shockwave-flash
> > content to ProxyHandler, since that's where HttpRequest, which has
> rewriteMime
> > access, is read and processed.
> >
> > * Updates tests accordingly.
> >
> >
> > Modified:
> >    incubator/shindig/trunk/config/container.js
> >    incubator/shindig/trunk/features/src/main/javascript/features/
> core.io/io.js
> >
>  incubator/shindig/trunk/features/src/main/javascript/features/core/legacy.js
> >
>  incubator/shindig/trunk/features/src/main/javascript/features/flash/flash.js
> >
>  incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/ProxyBase.java
> >
>  incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/ProxyHandler.java
> >
>  incubator/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/servlet/ProxyBaseTest.java
> >
>  incubator/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/servlet/ProxyHandlerTest.java
> >
> > Modified: incubator/shindig/trunk/config/container.js
> > URL:
> http://svn.apache.org/viewvc/incubator/shindig/trunk/config/container.js?rev=801008&r1=801007&r2=801008&view=diff
> >
> ==============================================================================
> > --- incubator/shindig/trunk/config/container.js (original)
> > +++ incubator/shindig/trunk/config/container.js Tue Aug  4 22:48:08 2009
> > @@ -96,7 +96,7 @@
> >  "gadgets.features" : {
> >   "core.io" : {
> >     // Note: /proxy is an open proxy. Be careful how you expose this!
> > -    "proxyUrl" : "http://
> %host%/gadgets/proxy?refresh=%refresh%&url=%url%",
> > +    "proxyUrl" : "http://
> %host%/gadgets/proxy?refresh=%refresh%&url=%url%&%rewriteMime%",
> >     "jsonProxyUrl" : "http://%host%/gadgets/makeRequest"
> >   },
> >   "views" : {
> >
> > Modified: incubator/shindig/trunk/features/src/main/javascript/features/
> core.io/io.js
> > URL:
> http://svn.apache.org/viewvc/incubator/shindig/trunk/features/src/main/javascript/features/core.io/io.js?rev=801008&r1=801007&r2=801008&view=diff
> >
> ==============================================================================
> > --- incubator/shindig/trunk/features/src/main/javascript/features/
> core.io/io.js (original)
> > +++ incubator/shindig/trunk/features/src/main/javascript/features/
> core.io/io.js Tue Aug  4 22:48:08 2009
> > @@ -470,12 +470,15 @@
> >
> >       var urlParams = gadgets.util.getUrlParameters();
> >
> > +      var rewriteMimeParam =
> > +          params.rewriteMime ? "rewriteMime=" +
> encodeURIComponent(params.rewriteMime) : "";
> >       return config.proxyUrl.replace("%url%", encodeURIComponent(url)).
> >           replace("%host%", document.location.host).
> >           replace("%rawurl%", url).
> >           replace("%refresh%", encodeURIComponent(refresh)).
> >           replace("%gadget%", encodeURIComponent(urlParams.url)).
> > -          replace("%container%", encodeURIComponent(urlParams.container
> || urlParams.synd));
> > +          replace("%container%", encodeURIComponent(urlParams.container
> || urlParams.synd)).
> > +          replace("%rewriteMime%", rewriteMimeParam);
> >     }
> >   };
> >  }();
> >
> > Modified:
> incubator/shindig/trunk/features/src/main/javascript/features/core/legacy.js
> > URL:
> http://svn.apache.org/viewvc/incubator/shindig/trunk/features/src/main/javascript/features/core/legacy.js?rev=801008&r1=801007&r2=801008&view=diff
> >
> ==============================================================================
> > ---
> incubator/shindig/trunk/features/src/main/javascript/features/core/legacy.js
> (original)
> > +++
> incubator/shindig/trunk/features/src/main/javascript/features/core/legacy.js
> Tue Aug  4 22:48:08 2009
> > @@ -124,9 +124,10 @@
> >  }
> >
> >  function _IG_GetCachedUrl(url, opt_params) {
> > -  var params = { 'REFRESH_INTERVAL': 3600 };
> > -  if (opt_params && opt_params.refreshInterval) {
> > -    params['REFRESH_INTERVAL'] = opt_params.refreshInterval;
> > +  var params = opt_params || {};
> > +  params['REFRESH_INTERVAL'] = 3600;
> > +  if (params.refreshInterval) {
> > +    params['REFRESH_INTERVAL'] = params.refreshInterval;
> >   }
> >   return gadgets.io.getProxyUrl(url, params);
> >  }
> >
> > Modified:
> incubator/shindig/trunk/features/src/main/javascript/features/flash/flash.js
> > URL:
> http://svn.apache.org/viewvc/incubator/shindig/trunk/features/src/main/javascript/features/flash/flash.js?rev=801008&r1=801007&r2=801008&view=diff
> >
> ==============================================================================
> > ---
> incubator/shindig/trunk/features/src/main/javascript/features/flash/flash.js
> (original)
> > +++
> incubator/shindig/trunk/features/src/main/javascript/features/flash/flash.js
> Tue Aug  4 22:48:08 2009
> > @@ -190,7 +190,7 @@
> >  */
> >  gadgets.flash.embedCachedFlash = function() {
> >   var args = Array.prototype.slice.call(arguments);
> > -  args[0] = gadgets.io.getProxyUrl(args[0]);
> > +  args[0] = gadgets.io.getProxyUrl(args[0], { rewriteMime:
> "application/x-shockwave-flash" });
> >   return gadgets.flash.embedFlash.apply(this, args);
> >  };
> >
> >
> > Modified:
> incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/ProxyBase.java
> > URL:
> http://svn.apache.org/viewvc/incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/ProxyBase.java?rev=801008&r1=801007&r2=801008&view=diff
> >
> ==============================================================================
> > ---
> incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/ProxyBase.java
> (original)
> > +++
> incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/ProxyBase.java
> Tue Aug  4 22:48:08 2009
> > @@ -131,12 +131,6 @@
> >       refreshInterval = Math.max(60 * 60, (int)(results.getCacheTtl() /
> 1000L));
> >     }
> >     HttpUtil.setCachingHeaders(response, refreshInterval);
> > -    // 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")))
> {
> > -      response.setHeader("Content-Disposition",
> "attachment;filename=p.txt");
> > -    }
> >   }
> >
> >   /**
> >
> > Modified:
> incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/ProxyHandler.java
> > URL:
> http://svn.apache.org/viewvc/incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/ProxyHandler.java?rev=801008&r1=801007&r2=801008&view=diff
> >
> ==============================================================================
> > ---
> incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/ProxyHandler.java
> (original)
> > +++
> incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/ProxyHandler.java
> Tue Aug  4 22:48:08 2009
> > @@ -153,21 +153,30 @@
> >       }
> >     }
> >
> > +    String responseType = results.getHeader("Content-Type");
> >     if (!StringUtils.isEmpty(rcr.getRewriteMimeType())) {
> >       String requiredType = rcr.getRewriteMimeType();
> > -      String responseType = results.getHeader("Content-Type");
> >       // Use a 'Vary' style check on the response
> >       if (requiredType.endsWith("/*") &&
> >           !StringUtils.isEmpty(responseType)) {
> >         requiredType = requiredType.substring(0, requiredType.length() -
> 2);
> >         if
> (!responseType.toLowerCase().startsWith(requiredType.toLowerCase())) {
> >           response.setContentType(requiredType);
> > +          responseType = requiredType;
> >         }
> >       } else {
> >         response.setContentType(requiredType);
> > +        responseType = requiredType;
> >       }
> >     }
> >
> > +    // 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(responseType))
> {
> > +      response.setHeader("Content-Disposition",
> "attachment;filename=p.txt");
> > +    }
> > +
> >     if (results.getHttpStatusCode() != HttpResponse.SC_OK) {
> >       response.sendError(results.getHttpStatusCode());
> >     }
> >
> > Modified:
> incubator/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/servlet/ProxyBaseTest.java
> > URL:
> http://svn.apache.org/viewvc/incubator/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/servlet/ProxyBaseTest.java?rev=801008&r1=801007&r2=801008&view=diff
> >
> ==============================================================================
> > ---
> incubator/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/servlet/ProxyBaseTest.java
> (original)
> > +++
> incubator/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/servlet/ProxyBaseTest.java
> Tue Aug  4 22:48:08 2009
> > @@ -146,7 +146,6 @@
> >     // Just verify that they were set. Specific values are configurable.
> >     assertNotNull("Expires header not set",
> recorder.getHeader("Expires"));
> >     assertNotNull("Cache-Control header not set",
> recorder.getHeader("Cache-Control"));
> > -    assertEquals("attachment;filename=p.txt",
> recorder.getHeader("Content-Disposition"));
> >   }
> >
> >   public void testSetResponseHeadersForFlash() throws Exception {
> > @@ -161,8 +160,6 @@
> >     // Just verify that they were set. Specific values are configurable.
> >     assertNotNull("Expires header not set",
> recorder.getHeader("Expires"));
> >     assertNotNull("Cache-Control header not set",
> recorder.getHeader("Cache-Control"));
> > -    assertNull("Content-Disposition header set for flash",
> > -        recorder.getHeader("Content-Disposition"));
> >   }
> >
> >   public void testSetResponseHeadersNoCache() throws Exception {
> > @@ -179,7 +176,6 @@
> >     assertNotNull("Expires header not set",
> recorder.getHeader("Expires"));
> >     assertEquals("no-cache", recorder.getHeader("Pragma"));
> >     assertEquals("no-cache", recorder.getHeader("Cache-Control"));
> > -    assertEquals("attachment;filename=p.txt",
> recorder.getHeader("Content-Disposition"));
> >   }
> >
> >   public void testSetResponseHeadersForceParam() throws Exception {
> > @@ -190,7 +186,6 @@
> >     proxy.setResponseHeaders(request, recorder, results);
> >
> >     HttpUtilTest.checkCacheControlHeaders(HttpUtilTest.testStartTime,
> recorder, 30, false);
> > -    assertEquals("attachment;filename=p.txt",
> recorder.getHeader("Content-Disposition"));
> >   }
> >
> >   public void testSetResponseHeadersForceParamInvalid() throws Exception
> {
> >
> > Modified:
> incubator/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/servlet/ProxyHandlerTest.java
> > URL:
> http://svn.apache.org/viewvc/incubator/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/servlet/ProxyHandlerTest.java?rev=801008&r1=801007&r2=801008&view=diff
> >
> ==============================================================================
> > ---
> incubator/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/servlet/ProxyHandlerTest.java
> (original)
> > +++
> incubator/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/servlet/ProxyHandlerTest.java
> Tue Aug  4 22:48:08 2009
> > @@ -87,6 +87,7 @@
> >     verify();
> >
> >     assertEquals(DATA_ONE, recorder.getResponseAsString());
> > +    assertEquals("attachment;filename=p.txt",
> recorder.getHeader("Content-Disposition"));
> >     assertTrue(rewriter.responseWasRewritten());
> >   }
> >
> > @@ -108,6 +109,7 @@
> >     assertEquals(Uri.parse(URL_ONE), httpRequest.getValue().getUri());
> >
> >     assertEquals(DATA_ONE, recorder.getResponseAsString());
> > +    assertEquals("attachment;filename=p.txt",
> recorder.getHeader("Content-Disposition"));
> >     assertTrue(rewriter.responseWasRewritten());
> >   }
> >
> > @@ -143,6 +145,26 @@
> >
> >     assertEquals(contentType, recorder.getHeader("Content-Type"));
> >     assertEquals(magicGarbage, recorder.getHeader("X-Magic-Garbage"));
> > +    assertEquals("attachment;filename=p.txt",
> recorder.getHeader("Content-Disposition"));
> > +    assertTrue(rewriter.responseWasRewritten());
> > +  }
> > +
> > +  public void testFlashGetsNoContentDisposition() throws Exception {
> > +    String url = "http://example.org/swiff.swf";
> > +    String domain = "example.org";
> > +    String contentType = "application/x-shockwave-flash";
> > +    Map<String, List<String>> headers = Maps.newHashMap();
> > +    headers.put("Content-Type", Arrays.asList(contentType));
> > +
> > +
>  expect(lockedDomainService.isSafeForOpenProxy(domain)).andReturn(true).atLeastOnce();
> > +    setupProxyRequestMock(domain, url);
> > +    expectGetAndReturnHeaders(url, headers);
> > +
> > +    replay();
> > +
> > +    proxyHandler.fetch(request, recorder);
> > +    assertEquals(contentType, recorder.getHeader("Content-Type"));
> > +    assertNull("Content-disposition set for flash",
> recorder.getHeader("Content-Disposition"));
> >     assertTrue(rewriter.responseWasRewritten());
> >   }
> >
> >
> >
> >
>