Author: johnh Date: Wed Aug 18 17:28:57 2010 New Revision: 986806 URL: http://svn.apache.org/viewvc?rev=986806&view=rev Log: Small changes that Anupama made in this patch: http://codereview.appspot.com/1855044/show which got reverted in this cl: http://codereview.appspot.com/1696056/show.
Re-written patch (by me), after recent commit caused original patch at (http://codereview.appspot.com/1962045/) to not apply cleanly. Original patch provided by Gagan Singh and Anupama Dutta. 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=986806&r1=986805&r2=986806&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 Wed Aug 18 17:28:57 2010 @@ -20,9 +20,9 @@ package org.apache.shindig.gadgets.servl import com.google.inject.Inject; import com.google.inject.Singleton; - -import org.apache.commons.io.IOUtils; +import com.google.inject.name.Named; import org.apache.commons.lang.StringUtils; +import org.apache.commons.io.IOUtils; import org.apache.shindig.common.uri.Uri; import org.apache.shindig.gadgets.GadgetException; import org.apache.shindig.gadgets.http.HttpRequest; @@ -50,12 +50,16 @@ public class ProxyHandler { private final RequestPipeline requestPipeline; private final ResponseRewriterRegistry contentRewriterRegistry; + protected final boolean remapInternalServerError; @Inject public ProxyHandler(RequestPipeline requestPipeline, - ResponseRewriterRegistry contentRewriterRegistry) { + ResponseRewriterRegistry contentRewriterRegistry, + @Named("shindig.proxy.remapInternalServerError") + Boolean remapInternalServerError) { this.requestPipeline = requestPipeline; this.contentRewriterRegistry = contentRewriterRegistry; + this.remapInternalServerError = remapInternalServerError; } /** @@ -102,6 +106,7 @@ public class ProxyHandler { } HttpResponseBuilder response = new HttpResponseBuilder(results); + response.clearAllHeaders(); try { ServletUtil.setCachingHeaders(response, @@ -110,7 +115,7 @@ public class ProxyHandler { return ServletUtil.errorResponse(gex); } - UriUtils.copyResponseHeadersAndStatusCode(results, response, true, true, + UriUtils.copyResponseHeadersAndStatusCode(results, response, remapInternalServerError, true, DisallowedHeaders.CACHING_DIRECTIVES, // Proxy sets its own caching headers. DisallowedHeaders.CLIENT_STATE_DIRECTIVES, // Overridden or irrelevant to proxy. DisallowedHeaders.OUTPUT_TRANSFER_DIRECTIVES @@ -120,7 +125,7 @@ public class ProxyHandler { // in order to prevent those from overwriting the correct values. setResponseContentHeaders(response, results); - response.setHeader("Content-Type", getContentType(rcr, response)); + UriUtils.maybeRewriteContentType(rcr, response); // TODO: replace this with streaming APIs when ready ByteArrayOutputStream baos = new ByteArrayOutputStream(); @@ -129,24 +134,6 @@ public class ProxyHandler { return response.create(); } - private String getContentType(HttpRequest rcr, HttpResponseBuilder results) { - String contentType = results.getHeader("Content-Type"); - if (!StringUtils.isEmpty(rcr.getRewriteMimeType())) { - String requiredType = rcr.getRewriteMimeType(); - // Use a 'Vary' style check on the response - if (requiredType.endsWith("/*") && - !StringUtils.isEmpty(contentType)) { - requiredType = requiredType.substring(0, requiredType.length() - 2); - if (!contentType.toLowerCase().startsWith(requiredType.toLowerCase())) { - contentType = requiredType; - } - } else { - contentType = requiredType; - } - } - return contentType; - } - private void setResponseContentHeaders(HttpResponseBuilder response, HttpResponse results) { // 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 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=986806&r1=986805&r2=986806&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 Wed Aug 18 17:28:57 2010 @@ -61,7 +61,7 @@ public class ProxyHandlerTest extends Ea private ProxyUriManager.ProxyUri request; private final ProxyHandler proxyHandler - = new ProxyHandler(pipeline, rewriterRegistry); + = new ProxyHandler(pipeline, rewriterRegistry, true); private void expectGetAndReturnData(String url, byte[] data) throws Exception { HttpRequest req = new HttpRequest(Uri.parse(url)); @@ -108,6 +108,35 @@ public class ProxyHandlerTest extends Ea } @Test + public void testInvalidHeaderDropped() throws Exception { + String url = "http://example.org/mypage.html";; + String domain = "example.org"; + + setupProxyRequestMock(domain, url, true, -1, null, null); + + HttpRequest req = new HttpRequest(Uri.parse(url)) + .setIgnoreCache(true); + String contentType = "text/html; charset=UTF-8"; + HttpResponse resp = new HttpResponseBuilder() + .setResponseString("Hello") + .addHeader("Content-Type", contentType) + .addHeader("Content-Length", "200") // Disallowed header. + .addHeader(":", "someDummyValue") // Invalid header name. + .create(); + + expect(pipeline.execute(req)).andReturn(resp); + + replay(); + + HttpResponse recorder = proxyHandler.fetch(request); + + verify(); + assertNull(recorder.getHeader(":")); + assertNull(recorder.getHeader("Content-Length")); + assertEquals(recorder.getHeader("Content-Type"), contentType); + } + + @Test public void testLockedDomainEmbed() throws Exception { setupNoArgsProxyRequestMock("www.example.com", URL_ONE); expectGetAndReturnData(URL_ONE, DATA_ONE.getBytes()); @@ -269,7 +298,7 @@ public class ProxyHandlerTest extends Ea ResponseRewriterRegistry rewriterRegistry = new DefaultResponseRewriterRegistry( Arrays.<ResponseRewriter>asList(rewriter), null); - ProxyHandler proxyHandler = new ProxyHandler(pipeline, rewriterRegistry); + ProxyHandler proxyHandler = new ProxyHandler(pipeline, rewriterRegistry, true); request.setReturnOriginalContentOnError(true); HttpResponse recorder = proxyHandler.fetch(request); @@ -306,7 +335,7 @@ public class ProxyHandlerTest extends Ea ResponseRewriterRegistry rewriterRegistry = new DefaultResponseRewriterRegistry( Arrays.<ResponseRewriter>asList(rewriter), null); - ProxyHandler proxyHandler = new ProxyHandler(pipeline, rewriterRegistry); + ProxyHandler proxyHandler = new ProxyHandler(pipeline, rewriterRegistry, true); boolean exceptionCaught = false; try {
