You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@shindig.apache.org by jo...@apache.org on 2009/10/05 21:24:37 UTC

svn commit: r821978 - in /incubator/shindig/trunk/java/gadgets/src: main/java/org/apache/shindig/gadgets/ main/java/org/apache/shindig/gadgets/render/ main/java/org/apache/shindig/gadgets/servlet/ test/java/org/apache/shindig/gadgets/render/ test/java/...

Author: johnh
Date: Mon Oct  5 19:24:36 2009
New Revision: 821978

URL: http://svn.apache.org/viewvc?rev=821978&view=rev
Log:
This patch implements validation of the v= param used by the gadget rendering
and JS servlets to version and thus aggressively cache their content.

The patch introduces two new validation methods to UrlGenerator, one for
IframeUrls and one for JsUrls. Each is sufficiently generic that other
configuration mismatches could be validated in them as well. This does bloat
UrlGenerator a bit, which gives stronger reason to split it into separate URL
generation interfaces (gadget, js) sooner than later.

DefaultUrlGenerator is updated with JS validation using the jsChecksum already
available to it. The Iframe validation method is implemented with existing
semantics: trust v= if it exists. Subclasses may improve on this by doing
something like @Injecting a request-scoped Gadget object (directly or otherwise)
and validating the spec checksum directly.

JsServlet/GadgetRenderingServlet updated to avoid caching altogether in the case
of invalid/stale URLs.

Existing tests updated; it would be nice, at least during refactoring, to
improve tests to cover v= cases thoroughly.


Added:
    incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/UrlValidationStatus.java
Modified:
    incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/DefaultUrlGenerator.java
    incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/UrlGenerator.java
    incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/render/RenderingGadgetRewriter.java
    incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/GadgetRenderingServlet.java
    incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/JsServlet.java
    incubator/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/render/RenderingGadgetRewriterTest.java
    incubator/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/servlet/GadgetRenderingServletTest.java
    incubator/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/servlet/JsonRpcHandlerTest.java

Modified: incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/DefaultUrlGenerator.java
URL: http://svn.apache.org/viewvc/incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/DefaultUrlGenerator.java?rev=821978&r1=821977&r2=821978&view=diff
==============================================================================
--- incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/DefaultUrlGenerator.java (original)
+++ incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/DefaultUrlGenerator.java Mon Oct  5 19:24:36 2009
@@ -114,6 +114,18 @@
        .append("&debug=").append(context.getDebug() ? "1" : "0");
     return buf.toString();
   }
+  
+  public UrlValidationStatus validateJsUrl(String url) {
+    Uri uri = Uri.parse(url);
+    String checksum = uri.getQueryParameter("v");
+    if (checksum != null) {
+      if (checksum.equals(jsChecksum)) {
+        return UrlValidationStatus.VALID_VERSIONED;
+      }
+      return UrlValidationStatus.INVALID;
+    }
+    return UrlValidationStatus.VALID_UNVERSIONED;
+  }
 
   public String getIframeUrl(Gadget gadget) {
     GadgetContext context = gadget.getContext();
@@ -170,6 +182,15 @@
 
     return uri.toString();
   }
+  
+  public UrlValidationStatus validateIframeUrl(String url) {
+    // Naive implementation: assume that the URL is valid always; versioned if v= present.
+    Uri uri = Uri.parse(url);
+    if (uri.getQueryParameter("v") != null) {
+      return UrlValidationStatus.VALID_VERSIONED;
+    }
+    return UrlValidationStatus.VALID_UNVERSIONED;
+  }
 
   public String getGadgetDomainOAuthCallback(String container, String gadgetHost) {
     String callback = oauthCallbackUriTemplates.get(container);

Modified: incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/UrlGenerator.java
URL: http://svn.apache.org/viewvc/incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/UrlGenerator.java?rev=821978&r1=821977&r2=821978&view=diff
==============================================================================
--- incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/UrlGenerator.java (original)
+++ incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/UrlGenerator.java Mon Oct  5 19:24:36 2009
@@ -18,16 +18,31 @@
  */
 package org.apache.shindig.gadgets;
 
-import com.google.inject.ImplementedBy;
-
 import java.util.Collection;
 
+import com.google.inject.ImplementedBy;
+
 /**
  * Generates urls for various public entrypoints
  */
 @ImplementedBy(DefaultUrlGenerator.class)
 public interface UrlGenerator {
   /**
+   * Generates iframe urls for meta data service.
+   * Use this rather than generating your own urls by hand.
+   *
+   * @return The generated iframe url.
+   */
+  String getIframeUrl(Gadget gadget);
+  
+  /**
+   * Validate gadget rendering URL.
+   * 
+   * @return Status of the rendered URL.
+   */
+  UrlValidationStatus validateIframeUrl(String url);
+  
+  /**
    * @param features The list of features that js is needed for.
    * @return The url for the bundled javascript that includes all referenced feature libraries.
    */
@@ -39,14 +54,15 @@
    * @return The bundled js parameter for type=url gadgets.
    */
   String getBundledJsParam(Collection<String> features, GadgetContext context);
-
+  
   /**
-   * Generates iframe urls for meta data service.
-   * Use this rather than generating your own urls by hand.
-   *
-   * @return The generated iframe url.
+   * Validates the inbound URL, for use by serving code for caching and redirection purposes.
+   * As an example, a JS URL with invalid/stale v= checksum may either be patched up or nullified.
+   * 
+   * @param url JS URL
+   * @return Validated equivalent of the inbound URL, or null if not a valid JS URL.
    */
-  String getIframeUrl(Gadget gadget);
+  UrlValidationStatus validateJsUrl(String url);
   
   /**
    * @return the oauthcallback URL on the gadget domain.  The returned URL may be absolute or

Added: incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/UrlValidationStatus.java
URL: http://svn.apache.org/viewvc/incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/UrlValidationStatus.java?rev=821978&view=auto
==============================================================================
--- incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/UrlValidationStatus.java (added)
+++ incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/UrlValidationStatus.java Mon Oct  5 19:24:36 2009
@@ -0,0 +1,25 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.shindig.gadgets;
+
+public enum UrlValidationStatus {
+  VALID_VERSIONED,
+  VALID_UNVERSIONED,
+  INVALID
+}

Modified: incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/render/RenderingGadgetRewriter.java
URL: http://svn.apache.org/viewvc/incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/render/RenderingGadgetRewriter.java?rev=821978&r1=821977&r2=821978&view=diff
==============================================================================
--- incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/render/RenderingGadgetRewriter.java (original)
+++ incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/render/RenderingGadgetRewriter.java Mon Oct  5 19:24:36 2009
@@ -224,7 +224,7 @@
     // gather the libraries we'll need to generate the forced libs
     if (forcedLibs == null || forcedLibs.length() == 0) {
       // Don't bother making a mutable copy if the list is empty
-      forced = (defaultForcedLibs.isEmpty()) ? defaultForcedLibs :  Sets.newTreeSet(defaultForcedLibs);
+      forced = (defaultForcedLibs.isEmpty()) ? defaultForcedLibs : Sets.newTreeSet(defaultForcedLibs);
     } else {
       forced = Sets.newTreeSet(Arrays.asList(forcedLibs.split(":")));
     }

Modified: incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/GadgetRenderingServlet.java
URL: http://svn.apache.org/viewvc/incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/GadgetRenderingServlet.java?rev=821978&r1=821977&r2=821978&view=diff
==============================================================================
--- incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/GadgetRenderingServlet.java (original)
+++ incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/GadgetRenderingServlet.java Mon Oct  5 19:24:36 2009
@@ -20,6 +20,8 @@
 import org.apache.shindig.common.servlet.HttpUtil;
 import org.apache.shindig.common.servlet.InjectedServlet;
 import org.apache.shindig.gadgets.GadgetContext;
+import org.apache.shindig.gadgets.UrlGenerator;
+import org.apache.shindig.gadgets.UrlValidationStatus;
 import org.apache.shindig.gadgets.http.HttpRequest;
 import org.apache.shindig.gadgets.render.Renderer;
 import org.apache.shindig.gadgets.render.RenderingResults;
@@ -39,13 +41,20 @@
 public class GadgetRenderingServlet extends InjectedServlet {
   static final int DEFAULT_CACHE_TTL = 60 * 5;
   private Renderer renderer;
+  private UrlGenerator urlGenerator;
 
   @Inject
   public void setRenderer(Renderer renderer) {
     this.renderer = renderer;
   }
+  
+  @Inject
+  public void setUrlGenerator(UrlGenerator gadgetUrlGenerator) {
+    this.urlGenerator = gadgetUrlGenerator;
+  }
 
-  private void render(HttpServletRequest req, HttpServletResponse resp) throws IOException {
+  private void render(HttpServletRequest req, HttpServletResponse resp, UrlValidationStatus urlstatus)
+      throws IOException {
     if (req.getHeader(HttpRequest.DOS_PREVENTION_HEADER) != null) {
       // Refuse to render for any request that came from us.
       // TODO: Is this necessary for any other type of request? Rendering seems to be the only one
@@ -61,9 +70,10 @@
     RenderingResults results = renderer.render(context);
     switch (results.getStatus()) {
       case OK:
-        if (context.getIgnoreCache()) {
+        if (context.getIgnoreCache() ||
+            urlstatus == UrlValidationStatus.INVALID) {
           HttpUtil.setCachingHeaders(resp, 0);
-        } else if (req.getParameter("v") != null) {
+        } else if (urlstatus == UrlValidationStatus.VALID_VERSIONED) {
           // Versioned files get cached indefinitely
           HttpUtil.setCachingHeaders(resp, true);
         } else {
@@ -92,17 +102,23 @@
     // If an If-Modified-Since header is ever provided, we always say
     // not modified. This is because when there actually is a change,
     // cache busting should occur.
+    UrlValidationStatus urlstatus = getUrlStatus(req);
     if (req.getHeader("If-Modified-Since") != null &&
         !"1".equals(req.getParameter("nocache")) &&
-        req.getParameter("v") != null) {
+        urlstatus == UrlValidationStatus.VALID_VERSIONED) {
       resp.setStatus(HttpServletResponse.SC_NOT_MODIFIED);
       return;
     }
-    render(req, resp);
+    render(req, resp, urlstatus);
   }
 
   @Override
   protected void doPost(HttpServletRequest req, HttpServletResponse resp) throws IOException {
-    render(req, resp);
+    render(req, resp, getUrlStatus(req));
+  }
+  
+  private UrlValidationStatus getUrlStatus(HttpServletRequest req) {
+    return urlGenerator.validateIframeUrl(
+        req.getRequestURL().append('?').append(req.getQueryString()).toString());
   }
-}
+}
\ No newline at end of file

Modified: incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/JsServlet.java
URL: http://svn.apache.org/viewvc/incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/JsServlet.java?rev=821978&r1=821977&r2=821978&view=diff
==============================================================================
--- incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/JsServlet.java (original)
+++ incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/JsServlet.java Mon Oct  5 19:24:36 2009
@@ -26,6 +26,8 @@
 import org.apache.shindig.gadgets.GadgetFeatureRegistry;
 import org.apache.shindig.gadgets.JsLibrary;
 import org.apache.shindig.gadgets.RenderingContext;
+import org.apache.shindig.gadgets.UrlGenerator;
+import org.apache.shindig.gadgets.UrlValidationStatus;
 
 import com.google.inject.Inject;
 
@@ -47,6 +49,12 @@
   public void setRegistry(GadgetFeatureRegistry registry) {
     this.registry = registry;
   }
+  
+  private UrlGenerator urlGenerator;
+  @Inject
+  public void setUrlGenerator(UrlGenerator urlGenerator) {
+    this.urlGenerator = urlGenerator;
+  }
 
   @Override
   protected void doGet(HttpServletRequest req, HttpServletResponse resp)
@@ -54,8 +62,10 @@
     // If an If-Modified-Since header is ever provided, we always say
     // not modified. This is because when there actually is a change,
     // cache busting should occur.
+    UrlValidationStatus vstatus = urlGenerator.validateJsUrl(
+        req.getRequestURL().append('?').append(req.getQueryString()).toString());
     if (req.getHeader("If-Modified-Since") != null &&
-        req.getParameter("v") != null) {
+        vstatus == UrlValidationStatus.VALID_VERSIONED) {
       resp.setStatus(HttpServletResponse.SC_NOT_MODIFIED);
       return;
     }
@@ -107,12 +117,20 @@
       return;
     }
 
-    if (req.getParameter("v") != null) {
-      // Versioned files get cached indefinitely
-      HttpUtil.setCachingHeaders(resp, !isProxyCacheable);
-    } else {
-      // Unversioned files get cached for 1 hour.
-      HttpUtil.setCachingHeaders(resp, 60 * 60, !isProxyCacheable);
+    switch (vstatus) {
+      case VALID_VERSIONED:
+        // Versioned files get cached indefinitely
+        HttpUtil.setCachingHeaders(resp, !isProxyCacheable);
+        break;
+      case VALID_UNVERSIONED:
+        // Unversioned files get cached for 1 hour.
+        HttpUtil.setCachingHeaders(resp, 60 * 60, !isProxyCacheable);
+        break;
+      case INVALID:
+        // URL is invalid in some way, likely version mismatch.
+        // Indicate no-cache forcing subsequent requests to regenerate URLs.
+        HttpUtil.setNoCache(resp);
+        break;
     }
     resp.setContentType("text/javascript; charset=utf-8");
     byte[] response = jsData.toString().getBytes("UTF-8");

Modified: incubator/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/render/RenderingGadgetRewriterTest.java
URL: http://svn.apache.org/viewvc/incubator/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/render/RenderingGadgetRewriterTest.java?rev=821978&r1=821977&r2=821978&view=diff
==============================================================================
--- incubator/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/render/RenderingGadgetRewriterTest.java (original)
+++ incubator/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/render/RenderingGadgetRewriterTest.java Mon Oct  5 19:24:36 2009
@@ -37,6 +37,7 @@
 import org.apache.shindig.gadgets.GadgetFeatureRegistry;
 import org.apache.shindig.gadgets.JsLibrary;
 import org.apache.shindig.gadgets.UrlGenerator;
+import org.apache.shindig.gadgets.UrlValidationStatus;
 import org.apache.shindig.gadgets.parse.GadgetHtmlParser;
 import org.apache.shindig.gadgets.parse.ParseModule;
 import org.apache.shindig.gadgets.preload.PreloadException;
@@ -751,10 +752,18 @@
     public String getBundledJsParam(Collection<String> features, GadgetContext context) {
       throw new UnsupportedOperationException();
     }
+    
+    public UrlValidationStatus validateJsUrl(String url) {
+      throw new UnsupportedOperationException();
+    }
 
     public String getIframeUrl(Gadget gadget) {
       throw new UnsupportedOperationException();
     }
+    
+    public UrlValidationStatus validateIframeUrl(String url) {
+      throw new UnsupportedOperationException();
+    }
 
     public String getBundledJsUrl(Collection<String> features, GadgetContext context) {
       return "/js/" + Join.join(":", features);

Modified: incubator/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/servlet/GadgetRenderingServletTest.java
URL: http://svn.apache.org/viewvc/incubator/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/servlet/GadgetRenderingServletTest.java?rev=821978&r1=821977&r2=821978&view=diff
==============================================================================
--- incubator/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/servlet/GadgetRenderingServletTest.java (original)
+++ incubator/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/servlet/GadgetRenderingServletTest.java Mon Oct  5 19:24:36 2009
@@ -25,12 +25,15 @@
 
 import org.apache.shindig.common.servlet.HttpServletResponseRecorder;
 import org.apache.shindig.gadgets.GadgetContext;
+import org.apache.shindig.gadgets.UrlGenerator;
+import org.apache.shindig.gadgets.UrlValidationStatus;
 import org.apache.shindig.gadgets.http.HttpRequest;
 import org.apache.shindig.gadgets.render.Renderer;
 import org.apache.shindig.gadgets.render.RenderingResults;
 
 import org.easymock.IMocksControl;
 import org.easymock.classextension.EasyMock;
+import org.junit.Before;
 import org.junit.Test;
 
 import javax.servlet.http.HttpServletRequest;
@@ -46,7 +49,15 @@
   private final Renderer renderer = control.createMock(Renderer.class);
   public final HttpServletResponseRecorder recorder = new HttpServletResponseRecorder(response);
   private final GadgetRenderingServlet servlet = new GadgetRenderingServlet();
-
+  private final UrlGenerator urlGenerator = control.createMock(UrlGenerator.class);
+  
+  @Before
+  public void setUpUrlGenerator() {
+    expect(urlGenerator.validateIframeUrl(isA(String.class))).andReturn(UrlValidationStatus.VALID_UNVERSIONED);
+    expect(request.getRequestURL()).andReturn(new StringBuffer("http://foo.com"));
+    expect(request.getQueryString()).andReturn("?q=a");
+    servlet.setUrlGenerator(urlGenerator);
+  }
 
   @Test
   public void dosHeaderRejected() throws Exception {

Modified: incubator/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/servlet/JsonRpcHandlerTest.java
URL: http://svn.apache.org/viewvc/incubator/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/servlet/JsonRpcHandlerTest.java?rev=821978&r1=821977&r2=821978&view=diff
==============================================================================
--- incubator/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/servlet/JsonRpcHandlerTest.java (original)
+++ incubator/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/servlet/JsonRpcHandlerTest.java Mon Oct  5 19:24:36 2009
@@ -28,6 +28,7 @@
 import org.apache.shindig.gadgets.GadgetContext;
 import org.apache.shindig.gadgets.GadgetException;
 import org.apache.shindig.gadgets.UrlGenerator;
+import org.apache.shindig.gadgets.UrlValidationStatus;
 import org.apache.shindig.gadgets.process.ProcessingException;
 import org.apache.shindig.gadgets.process.Processor;
 import org.apache.shindig.gadgets.spec.GadgetSpec;
@@ -269,6 +270,10 @@
     public String getBundledJsUrl(Collection<String> features, GadgetContext context) {
       throw new UnsupportedOperationException();
     }
+    
+    public UrlValidationStatus validateJsUrl(String jsUrl) {
+      throw new UnsupportedOperationException();
+    }
 
     public String getIframeUrl(Gadget gadget) {
       if (throwRandomFault) {
@@ -276,6 +281,10 @@
       }
       return iframeUrl;
     }
+    
+    public UrlValidationStatus validateIframeUrl(String url) {
+      throw new UnsupportedOperationException();
+    }
 
     public String getGadgetDomainOAuthCallback(String container, String gadgetHost) {
       throw new UnsupportedOperationException();