> 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? >
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. - Dan ----------------------------------------------------------- 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 > >