You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@shindig.apache.org by Doug Davies <da...@oclc.org> on 2014/12/05 15:28:40 UTC
Review Request 28755: When making OAuth2 request the oauthApprovalUrl response shouldn’t be cached
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/28755/
-----------------------------------------------------------
Review request for shindig.
Bugs: SHINDIG-1984
https://issues.apache.org/jira/browse/SHINDIG-1984
Repository: shindig
Description
-------
When doing an OAUTH2 flow the first request to the service that returns the oauthApprovalUrl probably shouldn’t be cached or set in the staleResponse, because then it could possibly be used on the response for the ACTUAL request if it returns a 500. Thus an endless loop of display the approval url and making the service call.
Diffs
-----
trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/http/DefaultRequestPipeline.java 1642996
trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/http/DefaultRequestPipelineTest.java 1642996
Diff: https://reviews.apache.org/r/28755/diff/
Testing
-------
Thanks,
Doug Davies
Re: Review Request 28755: When making OAuth2 request the oauthApprovalUrl response shouldn’t be cached
Posted by Doug Davies <da...@oclc.org>.
> On Dec. 6, 2014, 2:10 a.m., Ryan Baxter wrote:
> > Doug is there any way to do this check in the OAuth2 code as opposed to in the request pipeline?
good idea. I'll investigate on Monday.
- Doug
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/28755/#review64154
-----------------------------------------------------------
On Dec. 5, 2014, 2:28 p.m., Doug Davies wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/28755/
> -----------------------------------------------------------
>
> (Updated Dec. 5, 2014, 2:28 p.m.)
>
>
> Review request for shindig.
>
>
> Bugs: SHINDIG-1984
> https://issues.apache.org/jira/browse/SHINDIG-1984
>
>
> Repository: shindig
>
>
> Description
> -------
>
> When doing an OAUTH2 flow the first request to the service that returns the oauthApprovalUrl probably shouldn’t be cached or set in the staleResponse, because then it could possibly be used on the response for the ACTUAL request if it returns a 500. Thus an endless loop of display the approval url and making the service call.
>
>
> Diffs
> -----
>
> trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/http/DefaultRequestPipeline.java 1642996
> trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/http/DefaultRequestPipelineTest.java 1642996
>
> Diff: https://reviews.apache.org/r/28755/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Doug Davies
>
>
Re: Review Request 28755: When making OAuth2 request the oauthApprovalUrl response shouldn’t be cached
Posted by Ryan Baxter <rb...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/28755/#review64154
-----------------------------------------------------------
Doug is there any way to do this check in the OAuth2 code as opposed to in the request pipeline?
- Ryan Baxter
On Dec. 5, 2014, 2:28 p.m., Doug Davies wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/28755/
> -----------------------------------------------------------
>
> (Updated Dec. 5, 2014, 2:28 p.m.)
>
>
> Review request for shindig.
>
>
> Bugs: SHINDIG-1984
> https://issues.apache.org/jira/browse/SHINDIG-1984
>
>
> Repository: shindig
>
>
> Description
> -------
>
> When doing an OAUTH2 flow the first request to the service that returns the oauthApprovalUrl probably shouldn’t be cached or set in the staleResponse, because then it could possibly be used on the response for the ACTUAL request if it returns a 500. Thus an endless loop of display the approval url and making the service call.
>
>
> Diffs
> -----
>
> trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/http/DefaultRequestPipeline.java 1642996
> trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/http/DefaultRequestPipelineTest.java 1642996
>
> Diff: https://reviews.apache.org/r/28755/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Doug Davies
>
>
Re: Review Request 28755: When making OAuth2 request the oauthApprovalUrl response shouldn’t be cached
Posted by Ryan Baxter <rb...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/28755/#review68003
-----------------------------------------------------------
Ship it!
Ship It!
- Ryan Baxter
On Dec. 8, 2014, 6:35 p.m., Doug Davies wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/28755/
> -----------------------------------------------------------
>
> (Updated Dec. 8, 2014, 6:35 p.m.)
>
>
> Review request for shindig.
>
>
> Bugs: SHINDIG-1984
> https://issues.apache.org/jira/browse/SHINDIG-1984
>
>
> Repository: shindig
>
>
> Description
> -------
>
> When doing an OAUTH2 flow the first request to the service that returns the oauthApprovalUrl probably shouldn’t be cached or set in the staleResponse, because then it could possibly be used on the response for the ACTUAL request if it returns a 500. Thus an endless loop of display the approval url and making the service call.
>
>
> Diffs
> -----
>
> trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/http/DefaultRequestPipeline.java 1642996
> trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/http/DefaultRequestPipelineTest.java 1642996
>
> Diff: https://reviews.apache.org/r/28755/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Doug Davies
>
>
Re: Review Request 28755: When making OAuth2 request the oauthApprovalUrl response shouldn’t be cached
Posted by Doug Davies <da...@oclc.org>.
> On Dec. 8, 2014, 6:39 p.m., Doug Davies wrote:
> >
Bump?
- Doug
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/28755/#review64254
-----------------------------------------------------------
On Dec. 8, 2014, 6:35 p.m., Doug Davies wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/28755/
> -----------------------------------------------------------
>
> (Updated Dec. 8, 2014, 6:35 p.m.)
>
>
> Review request for shindig.
>
>
> Bugs: SHINDIG-1984
> https://issues.apache.org/jira/browse/SHINDIG-1984
>
>
> Repository: shindig
>
>
> Description
> -------
>
> When doing an OAUTH2 flow the first request to the service that returns the oauthApprovalUrl probably shouldn’t be cached or set in the staleResponse, because then it could possibly be used on the response for the ACTUAL request if it returns a 500. Thus an endless loop of display the approval url and making the service call.
>
>
> Diffs
> -----
>
> trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/http/DefaultRequestPipeline.java 1642996
> trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/http/DefaultRequestPipelineTest.java 1642996
>
> Diff: https://reviews.apache.org/r/28755/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Doug Davies
>
>
Re: Review Request 28755: When making OAuth2 request the oauthApprovalUrl response shouldn’t be cached
Posted by Doug Davies <da...@oclc.org>.
> On Dec. 8, 2014, 6:39 p.m., Doug Davies wrote:
> > trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/http/DefaultRequestPipeline.java, line 302
> > <https://reviews.apache.org/r/28755/diff/2/?file=785542#file785542line302>
> >
> > Hmmm... maybe this should be an OR. If the request says to ignore OR if the respoinse says to ignore.
nevermind. it think it's right.
- Doug
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/28755/#review64254
-----------------------------------------------------------
On Dec. 8, 2014, 6:35 p.m., Doug Davies wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/28755/
> -----------------------------------------------------------
>
> (Updated Dec. 8, 2014, 6:35 p.m.)
>
>
> Review request for shindig.
>
>
> Bugs: SHINDIG-1984
> https://issues.apache.org/jira/browse/SHINDIG-1984
>
>
> Repository: shindig
>
>
> Description
> -------
>
> When doing an OAUTH2 flow the first request to the service that returns the oauthApprovalUrl probably shouldn’t be cached or set in the staleResponse, because then it could possibly be used on the response for the ACTUAL request if it returns a 500. Thus an endless loop of display the approval url and making the service call.
>
>
> Diffs
> -----
>
> trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/http/DefaultRequestPipeline.java 1642996
> trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/http/DefaultRequestPipelineTest.java 1642996
>
> Diff: https://reviews.apache.org/r/28755/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Doug Davies
>
>
Re: Review Request 28755: When making OAuth2 request the oauthApprovalUrl response shouldn’t be cached
Posted by Doug Davies <da...@oclc.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/28755/#review64254
-----------------------------------------------------------
trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/http/DefaultRequestPipeline.java
<https://reviews.apache.org/r/28755/#comment106890>
Hmmm... maybe this should be an OR. If the request says to ignore OR if the respoinse says to ignore.
- Doug Davies
On Dec. 8, 2014, 6:35 p.m., Doug Davies wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/28755/
> -----------------------------------------------------------
>
> (Updated Dec. 8, 2014, 6:35 p.m.)
>
>
> Review request for shindig.
>
>
> Bugs: SHINDIG-1984
> https://issues.apache.org/jira/browse/SHINDIG-1984
>
>
> Repository: shindig
>
>
> Description
> -------
>
> When doing an OAUTH2 flow the first request to the service that returns the oauthApprovalUrl probably shouldn’t be cached or set in the staleResponse, because then it could possibly be used on the response for the ACTUAL request if it returns a 500. Thus an endless loop of display the approval url and making the service call.
>
>
> Diffs
> -----
>
> trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/http/DefaultRequestPipeline.java 1642996
> trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/http/DefaultRequestPipelineTest.java 1642996
>
> Diff: https://reviews.apache.org/r/28755/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Doug Davies
>
>
Re: Review Request 28755: When making OAuth2 request the oauthApprovalUrl response shouldn’t be cached
Posted by Doug Davies <da...@oclc.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/28755/
-----------------------------------------------------------
(Updated Dec. 8, 2014, 6:35 p.m.)
Review request for shindig.
Changes
-------
I discovered that all oauth2 redirect calls are already setting strictNoCache. The RequestPipeline was not looking at this when deciding if to cache or not and it should be. So I added the check and now all oauth2 redirect calls (including the one that returns the approval url) is not cached. I tweaked the unit test to test the strictNoCache functionality instead of just the approval url.
Bugs: SHINDIG-1984
https://issues.apache.org/jira/browse/SHINDIG-1984
Repository: shindig
Description
-------
When doing an OAUTH2 flow the first request to the service that returns the oauthApprovalUrl probably shouldn’t be cached or set in the staleResponse, because then it could possibly be used on the response for the ACTUAL request if it returns a 500. Thus an endless loop of display the approval url and making the service call.
Diffs (updated)
-----
trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/http/DefaultRequestPipeline.java 1642996
trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/http/DefaultRequestPipelineTest.java 1642996
Diff: https://reviews.apache.org/r/28755/diff/
Testing
-------
Thanks,
Doug Davies