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();