shindig-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From hsapu...@apache.org
Subject svn commit: r1149614 - in /shindig/trunk/java/gadgets/src: main/java/org/apache/shindig/gadgets/servlet/ProxyHandler.java main/java/org/apache/shindig/gadgets/servlet/ProxyServlet.java test/java/org/apache/shindig/gadgets/servlet/ProxyServletTest.java
Date Fri, 22 Jul 2011 14:27:38 GMT
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());
   }



Mime
View raw message