-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/6198/#review10203
-----------------------------------------------------------


Thanks for the changes.  Could you also clean up some of the duplicated code 
and move the comments over to the new method?
Something like this maybe:

  public GadgetSpec getGadgetSpec(GadgetContext context) throws GadgetException 
{
    Uri gadgetUri = getGadgetUri(context);
    if (RAW_GADGET_URI.equals(gadgetUri)) {
      try {
        String rawxml = context.getParameter(RAW_GADGETSPEC_XML_PARAM_NAME);
        return new GadgetSpec(gadgetUri, XmlUtil.parse(rawxml), rawxml);
      } catch (XmlException e) {
        throw new SpecParserException(e);
      }
    }

    Query query = new Query()
        .setSpecUri(gadgetUri)
        .setContainer(context.getContainer())
        .setGadgetUri(gadgetUri)
        .setIgnoreCache(context.getIgnoreCache());
    return super.getSpec(query);
  }

  public Uri getGadgetUri(GadgetContext context) throws GadgetException {
    String rawxml = context.getParameter(RAW_GADGETSPEC_XML_PARAM_NAME);
    if (rawxml != null) {
      // Set URI to a fixed, safe value (localhost), preventing a gadget 
rendered
      // via raw XML (eg. via POST) to be rendered on a locked domain of any 
other
      // gadget whose spec is hosted non-locally.
      return RAW_GADGET_URI;
    }
    return context.getUrl();
  }

Please also update the patch on the JIRA and grant license.

- Dan Dumont


On Aug. 9, 2012, 9:42 a.m., Marshall Shi wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/6198/
> -----------------------------------------------------------
> 
> (Updated Aug. 9, 2012, 9:42 a.m.)
> 
> 
> Review request for shindig, Ryan Baxter, Dan Dumont, and Stanton Sievers.
> 
> 
> Description
> -------
> 
> The gadgets/ifr endpoint will fetch the gadget xml first and then do the 
> white list check. It is consuming resources to fetch content when the gadget 
> is not allowed to render according to the gadget admin.
> The proposed fix is to move the white list check ahead of processing the 
> gadget xml. If the gadget is not allowed to show, an error message will be 
> returned before doing the content fetching. 
> 
> 
> This addresses bug shindig-1830.
>     https://issues.apache.org/jira/browse/shindig-1830
> 
> 
> Diffs
> -----
> 
>   
> http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/DefaultGadgetSpecFactory.java
>  1368808 
>   
> http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/GadgetSpecFactory.java
>  1368808 
>   
> http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/process/Processor.java
>  1368808 
>   
> http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/FakeGadgetSpecFactory.java
>  1368808 
>   
> http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/oauth2/MockUtils.java
>  1368808 
>   
> http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/process/ProcessorTest.java
>  1368808 
>   
> http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/rewrite/BaseRewriterTestCase.java
>  1368808 
>   
> http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/rewrite/RewriterTestBase.java
>  1368808 
> 
> Diff: https://reviews.apache.org/r/6198/diff/
> 
> 
> Testing
> -------
> 
> Done.
> 
> 
> Thanks,
> 
> Marshall Shi
> 
>

Reply via email to