Author: hsaputra Date: Fri Jul 22 14:27:37 2011 New Revision: 1149614 URL: http://svn.apache.org/viewvc?rev=1149614&view=rev Log: SHINDIG-1555 | Patch from Michael Brockhurst | Shindig's ProxyServlet should implement doPost
CR: https://reviews.apache.org/r/991/ Modified: shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/ProxyHandler.java shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/ProxyServlet.java shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/servlet/ProxyServletTest.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=1149614&r1=1149613&r2=1149614&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 Jul 22 14:27:37 2011 @@ -25,6 +25,7 @@ import com.google.inject.name.Named; import org.apache.commons.io.IOUtils; import org.apache.commons.lang.StringUtils; +import org.apache.shindig.common.Nullable; import org.apache.shindig.common.uri.Uri; import org.apache.shindig.gadgets.GadgetBlacklist; import org.apache.shindig.gadgets.GadgetException; @@ -40,8 +41,10 @@ import org.apache.shindig.gadgets.uri.Pr import org.apache.shindig.gadgets.uri.UriUtils; import org.apache.shindig.gadgets.uri.UriUtils.DisallowedHeaders; +import java.io.ByteArrayInputStream; import java.io.ByteArrayOutputStream; import java.io.IOException; +import java.io.InputStream; /** * Handles open proxy requests. @@ -53,16 +56,14 @@ public class ProxyHandler { protected final boolean remapInternalServerError; private final GadgetBlacklist gadgetBlacklist; private final Integer longLivedRefreshSec; + private static final String POST = "POST"; @Inject public ProxyHandler(RequestPipeline requestPipeline, - @RewriterRegistry(rewriteFlow = RewriteFlow.DEFAULT) - ResponseRewriterRegistry contentRewriterRegistry, - @Named("shindig.proxy.remapInternalServerError") - Boolean remapInternalServerError, - GadgetBlacklist gadgetBlacklist, - @Named("org.apache.shindig.gadgets.servlet.longLivedRefreshSec") - int longLivedRefreshSec) { + @RewriterRegistry(rewriteFlow = RewriteFlow.DEFAULT) ResponseRewriterRegistry contentRewriterRegistry, + @Named("shindig.proxy.remapInternalServerError") Boolean remapInternalServerError, + GadgetBlacklist gadgetBlacklist, + @Named("org.apache.shindig.gadgets.servlet.longLivedRefreshSec") int longLivedRefreshSec) { this.requestPipeline = requestPipeline; this.contentRewriterRegistry = contentRewriterRegistry; this.remapInternalServerError = remapInternalServerError; @@ -72,35 +73,47 @@ public class ProxyHandler { /** * Generate a remote content request based on the parameters sent from the client. + * @param uriCtx + * @param tgt + * @param postBody */ - private HttpRequest buildHttpRequest( - ProxyUriManager.ProxyUri uriCtx, Uri tgt) throws GadgetException { + private HttpRequest buildHttpRequest(ProxyUriManager.ProxyUri uriCtx, Uri tgt, @Nullable String postBody) + throws GadgetException, IOException { ServletUtil.validateUrl(tgt); HttpRequest req = uriCtx.makeHttpRequest(tgt); req.setRewriteMimeType(uriCtx.getRewriteMimeType()); + if (postBody != null) { + req.setMethod(POST); + // convert String into InputStream + req.setPostBody(new ByteArrayInputStream(postBody.getBytes())); + } return req; } - public HttpResponse fetch(ProxyUriManager.ProxyUri proxyUri) + public HttpResponse fetch(ProxyUriManager.ProxyUri proxyUri) throws IOException, GadgetException { + return fetch(proxyUri, null); + } + + public HttpResponse fetch(ProxyUriManager.ProxyUri proxyUri, @Nullable String postBody) throws IOException, GadgetException { - HttpRequest rcr = buildHttpRequest(proxyUri, proxyUri.getResource()); + HttpRequest rcr = buildHttpRequest(proxyUri, proxyUri.getResource(), postBody); if (rcr == null) { throw new GadgetException(GadgetException.Code.INVALID_PARAMETER, - "No url parameter in request", HttpResponse.SC_BAD_REQUEST); + "No url parameter in request", HttpResponse.SC_BAD_REQUEST); } if (rcr.getGadget() != null && gadgetBlacklist.isBlacklisted(rcr.getGadget())) { throw new GadgetException(GadgetException.Code.BLACKLISTED_GADGET, - "The requested content is unavailable", HttpResponse.SC_FORBIDDEN); + "The requested content is unavailable", HttpResponse.SC_FORBIDDEN); } - + HttpResponse results = requestPipeline.execute(rcr); if (results.isError()) { // Error: try the fallback. Particularly useful for proxied images. Uri fallbackUri = proxyUri.getFallbackUri(); if (fallbackUri != null) { - HttpRequest fallbackRcr = buildHttpRequest(proxyUri, fallbackUri); + HttpRequest fallbackRcr = buildHttpRequest(proxyUri, fallbackUri, null); results = requestPipeline.execute(fallbackRcr); } } @@ -113,7 +126,7 @@ public class ProxyHandler { // set to "true" or the error is irrecoverable from. if (!proxyUri.shouldReturnOrigOnErr() || !isRecoverable(results)) { throw new GadgetException(GadgetException.Code.INTERNAL_SERVER_ERROR, e, - e.getHttpStatusCode()); + e.getHttpStatusCode()); } } } @@ -122,18 +135,16 @@ public class ProxyHandler { response.clearAllHeaders(); try { - ServletUtil.setCachingHeaders(response, - proxyUri.translateStatusRefresh(longLivedRefreshSec, (int) (results.getCacheTtl() / 1000)), - false); + ServletUtil.setCachingHeaders(response, proxyUri.translateStatusRefresh(longLivedRefreshSec, + (int) (results.getCacheTtl() / 1000)), false); } catch (GadgetException gex) { return ServletUtil.errorResponse(gex); } 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 - ); + DisallowedHeaders.CACHING_DIRECTIVES, // Proxy sets its own caching headers. + DisallowedHeaders.CLIENT_STATE_DIRECTIVES, // Overridden or irrelevant to proxy. + DisallowedHeaders.OUTPUT_TRANSFER_DIRECTIVES); // Set Content-Type and Content-Disposition. Do this after copy results headers, // in order to prevent those from overwriting the correct values. @@ -164,28 +175,29 @@ public class ProxyHandler { /** * Test for presence of flash - * - * @param responseContentType the Content-Type header from the HttpResponseBuilder - * @param resultsContentType the Content-Type header from the HttpResponse + * + * @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); + || 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 - * only in case of serious grave errors (defined to be un recoverable). - * It should always be preferred to handle errors and return the original - * content at least. - * - * @param results The result of rewriting. + * Returns true in case the error encountered while rewriting the content is recoverable. The + * rationale behind it is that errors should be thrown only in case of serious grave errors + * (defined to be un recoverable). It should always be preferred to handle errors and return the + * original content at least. + * + * @param results + * The result of rewriting. * @return True if the error is recoverable, false otherwise. */ public boolean isRecoverable(HttpResponse results) { - return !(Strings.isNullOrEmpty(results.getResponseAsString()) && - results.getHeaders() == null); + return !(Strings.isNullOrEmpty(results.getResponseAsString()) && results.getHeaders() == null); } } Modified: shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/ProxyServlet.java URL: http://svn.apache.org/viewvc/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/ProxyServlet.java?rev=1149614&r1=1149613&r2=1149614&view=diff ============================================================================== --- shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/ProxyServlet.java (original) +++ shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/ProxyServlet.java Fri Jul 22 14:27:37 2011 @@ -27,7 +27,9 @@ import org.apache.shindig.gadgets.Locked import org.apache.shindig.gadgets.http.HttpResponse; import org.apache.shindig.gadgets.uri.ProxyUriManager; +import java.io.BufferedReader; import java.io.IOException; +import java.io.InputStreamReader; import java.util.logging.Level; import java.util.logging.Logger; @@ -37,16 +39,15 @@ import javax.servlet.http.HttpServletRes import com.google.inject.Inject; /** - * Handles open proxy requests (used in rewriting and for URLs returned by - * gadgets.io.getProxyUrl). + * Handles open proxy requests (used in rewriting and for URLs returned by gadgets.io.getProxyUrl). */ public class ProxyServlet extends InjectedServlet { private static final long serialVersionUID = 9085050443492307723L; - - //class name for logging purpose + + // class name for logging purpose private static final String classname = ProxyServlet.class.getName(); - private static final Logger LOG = Logger.getLogger(classname,MessageKeys.MESSAGES); - + private static final Logger LOG = Logger.getLogger(classname, MessageKeys.MESSAGES); + private transient ProxyUriManager proxyUriManager; private transient LockedDomainService lockedDomainService; private transient ProxyHandler proxyHandler; @@ -56,13 +57,13 @@ public class ProxyServlet extends Inject checkInitialized(); this.proxyHandler = proxyHandler; } - + @Inject public void setProxyUriManager(ProxyUriManager proxyUriManager) { checkInitialized(); this.proxyUriManager = proxyUriManager; } - + @Inject public void setLockedDomainService(LockedDomainService lockedDomainService) { checkInitialized(); @@ -72,13 +73,25 @@ public class ProxyServlet extends Inject @Override protected void doGet(HttpServletRequest request, HttpServletResponse servletResponse) throws IOException { + processRequest(request, servletResponse); + } + + @Override + protected void doPost(HttpServletRequest request, HttpServletResponse servletResponse) + throws IOException { + processRequest(request, servletResponse); + } + + private void processRequest(HttpServletRequest request, HttpServletResponse servletResponse) + throws IOException { if (request.getHeader("If-Modified-Since") != null) { servletResponse.setStatus(HttpServletResponse.SC_NOT_MODIFIED); return; } Uri reqUri = new UriBuilder(request).toUri(); - HttpResponse response = null; + + HttpResponse response; try { // Parse request uri: ProxyUriManager.ProxyUri proxyUri = proxyUriManager.process(reqUri); @@ -89,21 +102,49 @@ public class ProxyServlet extends Inject // Force embedded images and the like to their own domain to avoid XSS // in gadget domains. Uri resourceUri = proxyUri.getResource(); - String msg = "Embed request for url " + - (resourceUri != null ? resourceUri.toString() : "n/a") + " made to wrong domain " + host; + String msg = "Embed request for url " + (resourceUri != null ? resourceUri.toString() : "n/a") + + " made to wrong domain " + host; if (LOG.isLoggable(Level.INFO)) { - LOG.logp(Level.INFO, classname, "doGet", MessageKeys.EMBEDED_IMG_WRONG_DOMAIN, new Object[] {resourceUri != null ? resourceUri.toString() : "n/a", host}); + LOG.logp(Level.INFO, classname, "processRequest", MessageKeys.EMBEDED_IMG_WRONG_DOMAIN, + new Object[] { resourceUri != null ? resourceUri.toString() : "n/a", host }); } throw new GadgetException(GadgetException.Code.INVALID_PARAMETER, msg, - HttpResponse.SC_BAD_REQUEST); + HttpResponse.SC_BAD_REQUEST); + } + if ("POST".equalsIgnoreCase(request.getMethod())) { + StringBuffer buffer = getPOSTContent(request); + response = proxyHandler.fetch(proxyUri, buffer.toString()); + } else { + response = proxyHandler.fetch(proxyUri); } - - response = proxyHandler.fetch(proxyUri); } catch (GadgetException e) { response = ServletUtil.errorResponse(new GadgetException(e.getCode(), e.getMessage(), HttpServletResponse.SC_BAD_REQUEST)); } - + ServletUtil.copyToServletResponseAndOverrideCacheHeaders(response, servletResponse); } + + private StringBuffer getPOSTContent(HttpServletRequest request) throws IOException { + // Convert POST content from request to a string + StringBuffer buffer = new StringBuffer(); + BufferedReader reader = null; + try { + reader = new BufferedReader(new InputStreamReader(request.getInputStream())); + int letter = 0; + while ((letter = reader.read()) != -1) { + buffer.append((char) letter); + } + reader.close(); + } catch (IOException e) { + LOG.logp(Level.WARNING, classname, "getPOSTContent", "Caught exception while reading POST body:" + + e.getMessage()); + } finally { + if (reader != null) { + reader.close(); + reader = null; + } + } + return buffer; + } } Modified: shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/servlet/ProxyServletTest.java URL: http://svn.apache.org/viewvc/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/servlet/ProxyServletTest.java?rev=1149614&r1=1149613&r2=1149614&view=diff ============================================================================== --- shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/servlet/ProxyServletTest.java (original) +++ shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/servlet/ProxyServletTest.java Fri Jul 22 14:27:37 2011 @@ -21,6 +21,10 @@ package org.apache.shindig.gadgets.servl import static junitx.framework.StringAssert.assertContains; import static org.easymock.EasyMock.expect; +import java.io.ByteArrayInputStream; +import java.io.IOException; +import java.io.InputStream; + import org.apache.shindig.common.uri.Uri; import org.apache.shindig.gadgets.GadgetException; import org.apache.shindig.gadgets.LockedDomainService; @@ -29,19 +33,36 @@ import org.apache.shindig.gadgets.uri.Pr import org.junit.Before; import org.junit.Test; +import javax.servlet.ServletInputStream; import javax.servlet.http.HttpServletResponse; /** * Tests for ProxyServlet. - * + * * Tests are trivial; real tests are in ProxyHandlerTest. */ public class ProxyServletTest extends ServletTestFixture { private static final Uri REQUEST_URL = Uri.parse("http://example.org/file";); - private static final String BASIC_SYNTAX_URL - = "http://opensocial.org/proxy?foo=bar&url="; + REQUEST_URL; + private static final String BASIC_SYNTAX_URL = "http://opensocial.org/proxy?foo=bar&url="; + + REQUEST_URL; private static final String RESPONSE_BODY = "Hello, world!"; private static final String ERROR_MESSAGE = "Broken!"; + private static final String POST_CONTENT = "my post stuff"; + private static final String POST_METHOD = "POST"; + + private ServletInputStream postContentStream = new ServletInputStream() { + InputStream is = new ByteArrayInputStream(POST_CONTENT.getBytes()); + @Override + public int read() throws IOException { + return is.read(); + } + + @Override + public void close() throws IOException { + is.close(); + } + + }; private final ProxyUriManager proxyUriManager = mock(ProxyUriManager.class); private final LockedDomainService lockedDomainService = mock(LockedDomainService.class); @@ -62,6 +83,7 @@ public class ProxyServletTest extends Se private void setupRequest(String str, boolean ldSafe) throws Exception { Uri uri = Uri.parse(str); + expect(request.getScheme()).andReturn(uri.getScheme()); expect(request.getServerName()).andReturn(uri.getAuthority()); expect(request.getServerPort()).andReturn(80); @@ -80,7 +102,7 @@ public class ProxyServletTest extends Se @Test public void testIfModifiedSinceAlwaysReturnsEarly() throws Exception { expect(request.getHeader("If-Modified-Since")).andReturn("Yes, this is an invalid header"); - + replay(); servlet.doGet(request, recorder); verify(); @@ -93,7 +115,7 @@ public class ProxyServletTest extends Se public void testDoGetNormal() throws Exception { setupRequest(BASIC_SYNTAX_URL); expect(proxyHandler.fetch(proxyUri)).andReturn(new HttpResponse(RESPONSE_BODY)); - + replay(); servlet.doGet(request, recorder); verify(); @@ -105,7 +127,7 @@ public class ProxyServletTest extends Se public void testDoGetHttpError() throws Exception { setupRequest(BASIC_SYNTAX_URL); expect(proxyHandler.fetch(proxyUri)).andReturn(HttpResponse.notFound()); - + replay(); servlet.doGet(request, recorder); verify(); @@ -117,8 +139,8 @@ public class ProxyServletTest extends Se public void testDoGetException() throws Exception { setupRequest(BASIC_SYNTAX_URL); expect(proxyHandler.fetch(proxyUri)).andThrow( - new GadgetException(GadgetException.Code.FAILED_TO_RETRIEVE_CONTENT, ERROR_MESSAGE)); - + new GadgetException(GadgetException.Code.FAILED_TO_RETRIEVE_CONTENT, ERROR_MESSAGE)); + replay(); servlet.doGet(request, recorder); verify(); @@ -126,15 +148,71 @@ public class ProxyServletTest extends Se assertEquals(HttpServletResponse.SC_BAD_REQUEST, recorder.getHttpStatusCode()); assertContains(ERROR_MESSAGE, recorder.getResponseAsString()); } - + @Test public void testDoGetNormalWithLockedDomainUnsafe() throws Exception { setupRequest(BASIC_SYNTAX_URL, false); - + replay(); servlet.doGet(request, recorder); verify(); - + + assertEquals(HttpServletResponse.SC_BAD_REQUEST, recorder.getHttpStatusCode()); + assertContains("wrong domain", recorder.getResponseAsString()); + } + + @Test + public void testDoPostNormal() throws Exception { + setupRequest(BASIC_SYNTAX_URL); + expect(request.getInputStream()).andReturn(postContentStream); + expect(request.getMethod()).andReturn(POST_METHOD); + expect(proxyHandler.fetch(proxyUri, POST_CONTENT)).andReturn(new HttpResponse(RESPONSE_BODY)); + + replay(); + servlet.doPost(request, recorder); + verify(); + + assertResponseOk(HttpResponse.SC_OK, RESPONSE_BODY); + } + + @Test + public void testDoPostHttpError() throws Exception { + setupRequest(BASIC_SYNTAX_URL); + expect(proxyHandler.fetch(proxyUri, POST_CONTENT)).andReturn(HttpResponse.notFound()); + expect(request.getMethod()).andReturn(POST_METHOD); + expect(request.getInputStream()).andReturn(postContentStream); + + replay(); + servlet.doPost(request, recorder); + verify(); + + assertResponseOk(HttpResponse.SC_NOT_FOUND, ""); + } + + @Test + public void testDoPostException() throws Exception { + setupRequest(BASIC_SYNTAX_URL); + expect(request.getInputStream()).andReturn(postContentStream); + expect(request.getMethod()).andReturn(POST_METHOD); + expect(proxyHandler.fetch(proxyUri, POST_CONTENT)).andThrow( + new GadgetException(GadgetException.Code.FAILED_TO_RETRIEVE_CONTENT, ERROR_MESSAGE)); + + replay(); + servlet.doPost(request, recorder); + verify(); + + assertEquals(HttpServletResponse.SC_BAD_REQUEST, recorder.getHttpStatusCode()); + assertContains(ERROR_MESSAGE, recorder.getResponseAsString()); + } + + @Test + public void testDoPostNormalWithLockedDomainUnsafe() throws Exception { + setupRequest(BASIC_SYNTAX_URL, false); + + replay(); + servlet.doGet(request, recorder); + verify(); + assertEquals(HttpServletResponse.SC_BAD_REQUEST, recorder.getHttpStatusCode()); assertContains("wrong domain", recorder.getResponseAsString()); }
