> On Feb. 26, 2013, 12:22 p.m., Stanton Sievers wrote:
> > Is there accompanying Java code to do the replacement of %scheme%? The
> > only place I see this is happening is for authorization, token, and gadget
> > URIs in the
> > org.apache.shindig.gadgets.oauth2.persistence.sample.JSONOAuth2Persister.
> > I don't see anything that would handle this for the redirect and callback
> > URIs.
>
> Dan Dumont wrote:
> On second glance, Stanton brings up a good point. Also, the scheme
> should probably include the colon(:).
>
> Dan Dumont wrote:
> Could you please fill out the "Testing Done" section of the review to
> clarify if any testing was done to make sure this patch works as expected?
>
> Zhi Hong Yang wrote:
> "the scheme should probably include the colon(:).", if true, all codes
> as following should be changed :
> gadgetUri.replace("%scheme%", this.authority.getScheme()); to be
> gadgetUri.replace("%scheme%", this.authority.getScheme()+":");
> that's good idea?
>
>
> Dan Dumont wrote:
> Actually, disregard that comment about the scheme. The URI api does not
> include the colon as part of the scheme, so this looks good as it is. Please
> address Stanton's other concerns.
when shindig deploy in https env, if the urls in property file(i.e.
shindig.signing.global-callback-url,shindig.oauth2.global-redirect-uri) is
hardcoded as "http", it will cause issues.
the review code is updated, please take a look, thanks.
- Zhi Hong
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/9324/#review17091
-----------------------------------------------------------
On Feb. 6, 2013, 5:51 a.m., Zhi Hong Yang wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/9324/
> -----------------------------------------------------------
>
> (Updated Feb. 6, 2013, 5:51 a.m.)
>
>
> Review request for shindig, Ryan Baxter, Dan Dumont, Stanton Sievers, and
> Rich Thompson.
>
>
> Description
> -------
>
> in shindig.properties, the scheme is hardcoded in following setting:
> shindig.signing.global-callback-url=http://%authority%%contextRoot%/gadgets/oauthcallback
> shindig.oauth2.global-redirect-uri=http://%authority%%contextRoot%/gadgets/oauth2callback
> should be fixed as :
> shindig.signing.global-callback-url=%scheme%://%authority%%contextRoot%/gadgets/oauthcallback
> shindig.oauth2.global-redirect-uri=%scheme%://%authority%%contextRoot%/gadgets/oauth2callback
>
>
> Diffs
> -----
>
>
> http://svn.apache.org/repos/asf/shindig/trunk/java/common/conf/shindig.properties
> 1442845
>
> Diff: https://reviews.apache.org/r/9324/diff/
>
>
> Testing
> -------
>
>
> Thanks,
>
> Zhi Hong Yang
>
>
