You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@shindig.apache.org by Brian Lillie <br...@us.ibm.com> on 2011/09/01 18:38:04 UTC
Review Request: Gadget URI value incorrect on rewritten URLs and on Gadget
blacklist call
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/1692/
-----------------------------------------------------------
Review request for shindig.
Summary
-------
There are at least two cases where a URI is presented as a Gadget URI and is rather a URI based upon an incoming request.
1) When a resource such as a CSS file is loaded and has the links/URLs rewritten, the gadget parameter supplied on the rewritten link contains a URI associated with the resource requested, rather than the gadget associated with the request
2) When a request is made to the Gadget Blacklist, the URI parameter may not represent the intended gadget, but rather a random resource
With both of thse instances, the common pattern is that the DomWalker.makeGadget( HttpRequest ) is called, and the returned GadgetContext uses the request URI, rather than the Gadget URI.
Modify the DOMWalker makeGadget to prefer the use of the gadget URI to the request URI when constructing a Gadget/GadgetContext for use in rewriting or generating other requests
This addresses bug SHINDIG-1613.
https://issues.apache.org/jira/browse/SHINDIG-1613
Diffs
-----
http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/rewrite/DomWalker.java 1164090
http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/test/resources/org/apache/shindig/gadgets/rewrite/rewritebasic-expected.css 1164090
Diff: https://reviews.apache.org/r/1692/diff
Testing
-------
Modified rewrite junits to handle expected result
Thanks,
Brian
Re: Review Request: Gadget URI value incorrect on rewritten URLs and on
Gadget blacklist call
Posted by Dan Dumont <dd...@us.ibm.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/1692/#review8618
-----------------------------------------------------------
Hey Brian, what's the status of this review? Committed? Not needed?
- Dan Dumont
On Sept. 1, 2011, 4:38 p.m., Brian Lillie wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/1692/
> -----------------------------------------------------------
>
> (Updated Sept. 1, 2011, 4:38 p.m.)
>
>
> Review request for shindig.
>
>
> Description
> -------
>
> There are at least two cases where a URI is presented as a Gadget URI and is rather a URI based upon an incoming request.
>
> 1) When a resource such as a CSS file is loaded and has the links/URLs rewritten, the gadget parameter supplied on the rewritten link contains a URI associated with the resource requested, rather than the gadget associated with the request
> 2) When a request is made to the Gadget Blacklist, the URI parameter may not represent the intended gadget, but rather a random resource
>
> With both of thse instances, the common pattern is that the DomWalker.makeGadget( HttpRequest ) is called, and the returned GadgetContext uses the request URI, rather than the Gadget URI.
>
> Modify the DOMWalker makeGadget to prefer the use of the gadget URI to the request URI when constructing a Gadget/GadgetContext for use in rewriting or generating other requests
>
>
> This addresses bug SHINDIG-1613.
> https://issues.apache.org/jira/browse/SHINDIG-1613
>
>
> Diffs
> -----
>
> http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/rewrite/DomWalker.java 1164090
> http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/test/resources/org/apache/shindig/gadgets/rewrite/rewritebasic-expected.css 1164090
>
> Diff: https://reviews.apache.org/r/1692/diff/
>
>
> Testing
> -------
>
> Modified rewrite junits to handle expected result
>
>
> Thanks,
>
> Brian Lillie
>
>
Re: Review Request: Gadget URI value incorrect on rewritten URLs and on
Gadget blacklist call
Posted by Stanton Sievers <si...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/1692/#review1728
-----------------------------------------------------------
Ship it!
LGTM in theory. This code is used by all of the rewriters, so it's hard to wrap my head around potential ripple effect scenarios. I'll put my faith in the JUnits to catch any major problems introduced by making this change.
- Stanton
On 2011-09-01 16:38:04, Brian Lillie wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/1692/
> -----------------------------------------------------------
>
> (Updated 2011-09-01 16:38:04)
>
>
> Review request for shindig.
>
>
> Summary
> -------
>
> There are at least two cases where a URI is presented as a Gadget URI and is rather a URI based upon an incoming request.
>
> 1) When a resource such as a CSS file is loaded and has the links/URLs rewritten, the gadget parameter supplied on the rewritten link contains a URI associated with the resource requested, rather than the gadget associated with the request
> 2) When a request is made to the Gadget Blacklist, the URI parameter may not represent the intended gadget, but rather a random resource
>
> With both of thse instances, the common pattern is that the DomWalker.makeGadget( HttpRequest ) is called, and the returned GadgetContext uses the request URI, rather than the Gadget URI.
>
> Modify the DOMWalker makeGadget to prefer the use of the gadget URI to the request URI when constructing a Gadget/GadgetContext for use in rewriting or generating other requests
>
>
> This addresses bug SHINDIG-1613.
> https://issues.apache.org/jira/browse/SHINDIG-1613
>
>
> Diffs
> -----
>
> http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/rewrite/DomWalker.java 1164090
> http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/test/resources/org/apache/shindig/gadgets/rewrite/rewritebasic-expected.css 1164090
>
> Diff: https://reviews.apache.org/r/1692/diff
>
>
> Testing
> -------
>
> Modified rewrite junits to handle expected result
>
>
> Thanks,
>
> Brian
>
>