svn commit: r910768 - in /shindig/trunk/java/gadgets/src: main/java/org/apache/shindig/gadgets/servlet/ test/java/org/apache/shindig/gadgets/servlet/

[jo...@apache.org]

Author: johnh
Date: Tue Feb 16 23:34:37 2010
New Revision: 910768

URL: http://svn.apache.org/viewvc?rev=910768&view=rev
Log:
Avoid NumberFormatException (RuntimeException) issues when parsing TTL/refresh values.


Modified:
    shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/ConcatProxyServlet.java
    shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/GadgetRenderingServlet.java
    shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/servlet/GadgetRenderingServletTest.java
    shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/servlet/MakeRequestHandlerTest.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/ConcatProxyServlet.java
URL: http://svn.apache.org/viewvc/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/ConcatProxyServlet.java?rev=910768&r1=910767&r2=910768&view=diff
==============================================================================
--- shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/ConcatProxyServlet.java (original)
+++ shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/ConcatProxyServlet.java Tue Feb 16 23:34:37 2010
@@ -75,8 +75,13 @@
 
     boolean ignoreCache = proxyHandler.getIgnoreCache(request);
     if (!ignoreCache && request.getParameter(ProxyBase.REFRESH_PARAM) != null) {
-        HttpUtil.setCachingHeaders(response, Integer.valueOf(request
+      try {
+        HttpUtil.setCachingHeaders(response, Integer.parseInt(request
             .getParameter(ProxyBase.REFRESH_PARAM)));
+      } catch (NumberFormatException e) {
+        // Ignore malform ttl:
+        HttpUtil.setNoCache(response);        
+      }
     } else {
       HttpUtil.setNoCache(response);
     }

Modified: shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/GadgetRenderingServlet.java
URL: http://svn.apache.org/viewvc/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/GadgetRenderingServlet.java?rev=910768&r1=910767&r2=910768&view=diff
==============================================================================
--- shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/GadgetRenderingServlet.java (original)
+++ shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/GadgetRenderingServlet.java Tue Feb 16 23:34:37 2010
@@ -22,6 +22,7 @@
 import org.apache.shindig.gadgets.GadgetContext;
 import org.apache.shindig.gadgets.UrlGenerator;
 import org.apache.shindig.gadgets.UrlValidationStatus;
+import org.apache.shindig.gadgets.http.BasicHttpFetcher;
 import org.apache.shindig.gadgets.http.HttpRequest;
 import org.apache.shindig.gadgets.render.Renderer;
 import org.apache.shindig.gadgets.render.RenderingResults;
@@ -29,8 +30,10 @@
 import com.google.inject.Inject;
 
 import org.apache.commons.lang.StringEscapeUtils;
+import org.apache.commons.lang.StringUtils;
 
 import java.io.IOException;
+import java.util.logging.Logger;
 
 import javax.servlet.http.HttpServletRequest;
 import javax.servlet.http.HttpServletResponse;
@@ -40,6 +43,9 @@
  */
 public class GadgetRenderingServlet extends InjectedServlet {
   static final int DEFAULT_CACHE_TTL = 60 * 5;
+
+  private static final Logger LOG = Logger.getLogger(GadgetRenderingServlet.class.getName());
+
   private Renderer renderer;
   private UrlGenerator urlGenerator;
 
@@ -81,8 +87,13 @@
           // with a query parameter.
           int ttl = DEFAULT_CACHE_TTL;
           String ttlStr = req.getParameter(ProxyBase.REFRESH_PARAM);
-          if (ttlStr != null) {
-            ttl = Integer.parseInt(ttlStr);
+          if (!StringUtils.isEmpty(ttlStr)) {
+            try {
+              ttl = Integer.parseInt(ttlStr);
+            } catch (NumberFormatException e) {
+              // Ignore malformed TTL value
+              LOG.info("Bad TTL value '" + ttlStr + "' was ignored");
+            }
           }
           HttpUtil.setCachingHeaders(resp, ttl, true);
         }
@@ -122,4 +133,4 @@
     return urlGenerator.validateIframeUrl(
         req.getRequestURL().append('?').append(req.getQueryString()).toString());
   }
-}
\ No newline at end of file
+}

Modified: shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/servlet/GadgetRenderingServletTest.java
URL: http://svn.apache.org/viewvc/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/servlet/GadgetRenderingServletTest.java?rev=910768&r1=910767&r2=910768&view=diff
==============================================================================
--- shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/servlet/GadgetRenderingServletTest.java (original)
+++ shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/servlet/GadgetRenderingServletTest.java Tue Feb 16 23:34:37 2010
@@ -69,6 +69,28 @@
   }
 
   @Test
+  public void renderWithTtl() throws Exception {
+    servlet.setRenderer(renderer);
+    expect(renderer.render(isA(GadgetContext.class)))
+        .andReturn(RenderingResults.ok("working"));
+    expect(request.getParameter(ProxyBase.REFRESH_PARAM)).andReturn("120");
+    control.replay();
+    servlet.doGet(request, recorder);
+    assertEquals("private,max-age=120", recorder.getHeader("Cache-Control"));
+  }
+
+  @Test
+  public void renderWithBadTtl() throws Exception {
+    servlet.setRenderer(renderer);
+    expect(renderer.render(isA(GadgetContext.class)))
+        .andReturn(RenderingResults.ok("working"));
+    expect(request.getParameter(ProxyBase.REFRESH_PARAM)).andReturn("");
+    control.replay();
+    servlet.doGet(request, recorder);
+    assertEquals("private,max-age=300", recorder.getHeader("Cache-Control"));
+  }
+
+  @Test
   public void normalResponse() throws Exception {
     servlet.setRenderer(renderer);
     expect(renderer.render(isA(GadgetContext.class)))

Modified: shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/servlet/MakeRequestHandlerTest.java
URL: http://svn.apache.org/viewvc/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/servlet/MakeRequestHandlerTest.java?rev=910768&r1=910767&r2=910768&view=diff
==============================================================================
--- shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/servlet/MakeRequestHandlerTest.java (original)
+++ shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/servlet/MakeRequestHandlerTest.java Tue Feb 16 23:34:37 2010
@@ -146,6 +146,23 @@
   }
 
   @Test
+  public void testGetRequestWithBadTtl() throws Exception {
+    expect(request.getParameter(ProxyBase.REFRESH_PARAM)).andReturn("foo").anyTimes();
+
+    Capture<HttpRequest> requestCapture = new Capture<HttpRequest>();
+    expect(pipeline.execute(capture(requestCapture))).andReturn(new HttpResponse(RESPONSE_BODY));
+
+    replay();
+
+    handler.fetch(request, recorder);
+
+    HttpRequest httpRequest = requestCapture.getValue();
+    assertEquals(null, recorder.getHeader("Cache-Control"));
+    assertEquals(-1, httpRequest.getCacheTtl());
+  }
+
+
+  @Test
   public void testExplicitHeaders() throws Exception {
     String headerString = "X-Foo=bar&X-Bar=baz%20foo";
 

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=910768&r1=910767&r2=910768&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 Tue Feb 16 23:34:37 2010
@@ -203,6 +203,66 @@
     verify();
   }
 
+  /**
+   * Override HttpRequest equals to check for cache control fields
+   */
+  static class HttpRequestCache extends HttpRequest {
+    public HttpRequestCache(Uri uri) {
+      super(uri);
+    }
+
+    @Override
+    public boolean equals(Object obj) {
+      if (obj instanceof HttpRequest) {
+        HttpRequest req = (HttpRequest)obj;
+        if (req.getCacheTtl() != getCacheTtl() || req.getIgnoreCache() != getIgnoreCache()) {
+          return false;
+        }
+      }
+      return super.equals(obj);
+    }
+  }
+
+  @Test
+  public void testWithCache() throws Exception {
+    String url = "http://example.org/file.evil";
+    String domain = "example.org";
+
+    expect(lockedDomainService.isSafeForOpenProxy(domain)).andReturn(true).atLeastOnce();
+    setupProxyRequestMock(domain, url);
+    expect(request.getParameter(ProxyBase.REFRESH_PARAM)).andReturn("120").atLeastOnce();
+
+    HttpRequest req = new HttpRequestCache(Uri.parse(url)).setCacheTtl(120).setIgnoreCache(false);
+    HttpResponse resp = new HttpResponse("Hello");
+    expect(pipeline.execute(req)).andReturn(resp);
+
+    replay();
+
+    proxyHandler.fetch(request, recorder);
+
+    verify();
+  }
+
+  @Test
+  public void testWithBadTtl() throws Exception {
+    String url = "http://example.org/file.evil";
+    String domain = "example.org";
+
+    expect(lockedDomainService.isSafeForOpenProxy(domain)).andReturn(true).atLeastOnce();
+    setupProxyRequestMock(domain, url);
+    expect(request.getParameter(ProxyBase.REFRESH_PARAM)).andReturn("foo").atLeastOnce();
+
+    HttpRequest req = new HttpRequestCache(Uri.parse(url)).setCacheTtl(-1).setIgnoreCache(false);
+    HttpResponse resp = new HttpResponse("Hello");
+    expect(pipeline.execute(req)).andReturn(resp);
+
+    replay();
+
+    proxyHandler.fetch(request, recorder);
+
+    verify();
+  }
+
   @Test
   public void testXForwardedFor() throws Exception {
     String url = "http://example.org/";