-----------------------------------------------------------
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
>
>