You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@shindig.apache.org by Marshall Shi <sh...@cn.ibm.com> on 2012/08/01 04:54:48 UTC
Re: Review Request: Do whitelist check before consuming resources fetching
content from the gadget URI
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/6198/
-----------------------------------------------------------
(Updated Aug. 1, 2012, 2:54 a.m.)
Review request for shindig, Ryan Baxter, Dan Dumont, and Stanton Sievers.
Changes
-------
Fixed white space problem raised by Ryan.
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/process/Processor.java 1363665
Diff: https://reviews.apache.org/r/6198/diff/
Testing (updated)
-------
Done.
Thanks,
Marshall Shi
Re: Review Request: Do whitelist check before consuming resources fetching
content from the gadget URI
Posted by Dan Dumont <dd...@us.ibm.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/6198/#review10020
-----------------------------------------------------------
http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/DefaultGadgetSpecFactory.java
<https://reviews.apache.org/r/6198/#comment21226>
Like this.
http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/DefaultGadgetSpecFactory.java
<https://reviews.apache.org/r/6198/#comment21225>
This method should incorporate a similar check like above to return the correct URI if the gadget is rawxml.
- Dan Dumont
On Aug. 5, 2012, 1:37 a.m., Marshall Shi wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/6198/
> -----------------------------------------------------------
>
> (Updated Aug. 5, 2012, 1:37 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
>
>
Re: Review Request: Do whitelist check before consuming resources fetching
content from the gadget URI
Posted by Dan Dumont <dd...@us.ibm.com>.
-----------------------------------------------------------
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
>
>
Re: Review Request: Do whitelist check before consuming resources fetching
content from the gadget URI
Posted by Ryan Baxter <rb...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/6198/#review10264
-----------------------------------------------------------
Ship it!
Ship It!
- Ryan Baxter
On Aug. 14, 2012, 5:50 a.m., Marshall Shi wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/6198/
> -----------------------------------------------------------
>
> (Updated Aug. 14, 2012, 5:50 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 1372695
> http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/GadgetSpecFactory.java 1368912
> http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/process/Processor.java 1368912
> http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/FakeGadgetSpecFactory.java 1368912
> http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/oauth2/MockUtils.java 1368912
> http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/process/ProcessorTest.java 1368912
> http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/rewrite/BaseRewriterTestCase.java 1368912
> http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/rewrite/RewriterTestBase.java 1368912
>
> Diff: https://reviews.apache.org/r/6198/diff/
>
>
> Testing
> -------
>
> Done.
>
>
> Thanks,
>
> Marshall Shi
>
>
Re: Review Request: Do whitelist check before consuming resources fetching
content from the gadget URI
Posted by Dan Dumont <dd...@us.ibm.com>.
> On Aug. 14, 2012, 2:07 p.m., Dan Dumont wrote:
> > Committed r1372888
> >
Please close the review.
- Dan
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/6198/#review10273
-----------------------------------------------------------
On Aug. 14, 2012, 5:50 a.m., Marshall Shi wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/6198/
> -----------------------------------------------------------
>
> (Updated Aug. 14, 2012, 5:50 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 1372695
> http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/GadgetSpecFactory.java 1368912
> http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/process/Processor.java 1368912
> http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/FakeGadgetSpecFactory.java 1368912
> http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/oauth2/MockUtils.java 1368912
> http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/process/ProcessorTest.java 1368912
> http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/rewrite/BaseRewriterTestCase.java 1368912
> http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/rewrite/RewriterTestBase.java 1368912
>
> Diff: https://reviews.apache.org/r/6198/diff/
>
>
> Testing
> -------
>
> Done.
>
>
> Thanks,
>
> Marshall Shi
>
>
Re: Review Request: Do whitelist check before consuming resources fetching
content from the gadget URI
Posted by Dan Dumont <dd...@us.ibm.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/6198/#review10273
-----------------------------------------------------------
Ship it!
Committed r1372888
- Dan Dumont
On Aug. 14, 2012, 5:50 a.m., Marshall Shi wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/6198/
> -----------------------------------------------------------
>
> (Updated Aug. 14, 2012, 5:50 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 1372695
> http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/GadgetSpecFactory.java 1368912
> http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/process/Processor.java 1368912
> http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/FakeGadgetSpecFactory.java 1368912
> http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/oauth2/MockUtils.java 1368912
> http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/process/ProcessorTest.java 1368912
> http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/rewrite/BaseRewriterTestCase.java 1368912
> http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/rewrite/RewriterTestBase.java 1368912
>
> Diff: https://reviews.apache.org/r/6198/diff/
>
>
> Testing
> -------
>
> Done.
>
>
> Thanks,
>
> Marshall Shi
>
>
Re: Review Request: Do whitelist check before consuming resources fetching
content from the gadget URI
Posted by Marshall Shi <sh...@cn.ibm.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/6198/
-----------------------------------------------------------
(Updated Aug. 14, 2012, 5:50 a.m.)
Review request for shindig, Ryan Baxter, Dan Dumont, and Stanton Sievers.
Changes
-------
Remove duplicated logic as suggested by Dan.
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 (updated)
-----
http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/DefaultGadgetSpecFactory.java 1372695
http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/GadgetSpecFactory.java 1368912
http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/process/Processor.java 1368912
http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/FakeGadgetSpecFactory.java 1368912
http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/oauth2/MockUtils.java 1368912
http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/process/ProcessorTest.java 1368912
http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/rewrite/BaseRewriterTestCase.java 1368912
http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/rewrite/RewriterTestBase.java 1368912
Diff: https://reviews.apache.org/r/6198/diff/
Testing
-------
Done.
Thanks,
Marshall Shi
Re: Review Request: Do whitelist check before consuming resources fetching
content from the gadget URI
Posted by Marshall Shi <sh...@cn.ibm.com>.
-----------------------------------------------------------
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.
Changes
-------
Update patch according to Dan's comment.
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 (updated)
-----
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
Re: Review Request: Do whitelist check before consuming resources fetching
content from the gadget URI
Posted by Marshall Shi <sh...@cn.ibm.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/6198/
-----------------------------------------------------------
(Updated Aug. 5, 2012, 1:37 a.m.)
Review request for shindig, Ryan Baxter, Dan Dumont, and Stanton Sievers.
Changes
-------
Add a new method getGadgetUri in GadgetSpecFactory interface, so Processor can use this method to get the gadget uri instead of get the uri from context directly.
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 (updated)
-----
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
Re: Review Request: Do whitelist check before consuming resources fetching
content from the gadget URI
Posted by Marshall Shi <sh...@cn.ibm.com>.
> On Aug. 1, 2012, 6:04 p.m., Dan Dumont wrote:
> > Ship It!
>
> Dan Dumont wrote:
> Please update the patch in the JIRA with your latest changes.
patch updated in the JIRA.
- Marshall
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/6198/#review9699
-----------------------------------------------------------
On Aug. 1, 2012, 2:54 a.m., Marshall Shi wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/6198/
> -----------------------------------------------------------
>
> (Updated Aug. 1, 2012, 2:54 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/process/Processor.java 1363665
>
> Diff: https://reviews.apache.org/r/6198/diff/
>
>
> Testing
> -------
>
> Done.
>
>
> Thanks,
>
> Marshall Shi
>
>
Re: Review Request: Do whitelist check before consuming resources fetching
content from the gadget URI
Posted by Dan Dumont <dd...@us.ibm.com>.
> On Aug. 1, 2012, 6:04 p.m., Dan Dumont wrote:
> > Ship It!
Please update the patch in the JIRA with your latest changes.
- Dan
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/6198/#review9699
-----------------------------------------------------------
On Aug. 1, 2012, 2:54 a.m., Marshall Shi wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/6198/
> -----------------------------------------------------------
>
> (Updated Aug. 1, 2012, 2:54 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/process/Processor.java 1363665
>
> Diff: https://reviews.apache.org/r/6198/diff/
>
>
> Testing
> -------
>
> Done.
>
>
> Thanks,
>
> Marshall Shi
>
>
Re: Review Request: Do whitelist check before consuming resources fetching
content from the gadget URI
Posted by Dan Dumont <dd...@us.ibm.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/6198/#review9699
-----------------------------------------------------------
Ship it!
Ship It!
- Dan Dumont
On Aug. 1, 2012, 2:54 a.m., Marshall Shi wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/6198/
> -----------------------------------------------------------
>
> (Updated Aug. 1, 2012, 2:54 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/process/Processor.java 1363665
>
> Diff: https://reviews.apache.org/r/6198/diff/
>
>
> Testing
> -------
>
> Done.
>
>
> Thanks,
>
> Marshall Shi
>
>
Re: Review Request: Do whitelist check before consuming resources fetching
content from the gadget URI
Posted by Dan Dumont <dd...@us.ibm.com>.
> On Aug. 2, 2012, 1:13 p.m., Dan Dumont wrote:
> > This patch might need a bit more work. I'll need to talk to Ryan about this method:
> > public GadgetSpec getGadgetSpec(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.
> >
> > It looks like we'd be altering the whitelist check for rawxml gadgets with this change.
I think it might be a good idea to put a getGadgetUri(GadgetContext context) in the org.apache.shindig.gadgets.GadgetSpecFactory interface.
This can implement the current logic found in getGadgetSpec(GadgetContext context) for deriving the gadget uri.
You can then use that instead of trying to get it from the context directly.
- Dan
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/6198/#review9749
-----------------------------------------------------------
On Aug. 1, 2012, 2:54 a.m., Marshall Shi wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/6198/
> -----------------------------------------------------------
>
> (Updated Aug. 1, 2012, 2:54 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/process/Processor.java 1363665
>
> Diff: https://reviews.apache.org/r/6198/diff/
>
>
> Testing
> -------
>
> Done.
>
>
> Thanks,
>
> Marshall Shi
>
>
Re: Review Request: Do whitelist check before consuming resources fetching
content from the gadget URI
Posted by Ryan Baxter <rb...@gmail.com>.
> On Aug. 2, 2012, 1:13 p.m., Dan Dumont wrote:
> > This patch might need a bit more work. I'll need to talk to Ryan about this method:
> > public GadgetSpec getGadgetSpec(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.
> >
> > It looks like we'd be altering the whitelist check for rawxml gadgets with this change.
>
> Dan Dumont wrote:
> I think it might be a good idea to put a getGadgetUri(GadgetContext context) in the org.apache.shindig.gadgets.GadgetSpecFactory interface.
> This can implement the current logic found in getGadgetSpec(GadgetContext context) for deriving the gadget uri.
>
> You can then use that instead of trying to get it from the context directly.
Good catch Dan!
- Ryan
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/6198/#review9749
-----------------------------------------------------------
On Aug. 1, 2012, 2:54 a.m., Marshall Shi wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/6198/
> -----------------------------------------------------------
>
> (Updated Aug. 1, 2012, 2:54 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/process/Processor.java 1363665
>
> Diff: https://reviews.apache.org/r/6198/diff/
>
>
> Testing
> -------
>
> Done.
>
>
> Thanks,
>
> Marshall Shi
>
>
Re: Review Request: Do whitelist check before consuming resources fetching
content from the gadget URI
Posted by Dan Dumont <dd...@us.ibm.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/6198/#review9749
-----------------------------------------------------------
This patch might need a bit more work. I'll need to talk to Ryan about this method:
public GadgetSpec getGadgetSpec(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.
It looks like we'd be altering the whitelist check for rawxml gadgets with this change.
- Dan Dumont
On Aug. 1, 2012, 2:54 a.m., Marshall Shi wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/6198/
> -----------------------------------------------------------
>
> (Updated Aug. 1, 2012, 2:54 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/process/Processor.java 1363665
>
> Diff: https://reviews.apache.org/r/6198/diff/
>
>
> Testing
> -------
>
> Done.
>
>
> Thanks,
>
> Marshall Shi
>
>