> On 2012-02-22 03:03:25, li xu wrote: > > LGTM. Thanks!
Sorry,missed previous comments. Second thought after reading it and checking a few web links, I agree more thoughts on "403" will be needed. The reason about "403" may not be token related...it's common to return "403" with a reason like "No directory Browsing" etc. - li ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/3987/#review5259 ----------------------------------------------------------- On 2012-02-21 21:40:31, Stanton Sievers wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/3987/ > ----------------------------------------------------------- > > (Updated 2012-02-21 21:40:31) > > > Review request for shindig, li xu and Adam Clarke. > > > Summary > ------- > > From JIRA: > If the url to which a gadget is doing a makeRequest doesn't exist, i.e., > returns a 404 to the Shindig server, the access token is being removed from > the OAuth2 Store. This functionality is implemented here: > org.apache.shindig.gadgets.oauth2.BasicOAuth2Request.fetchFromServer(OAuth2Accessor, > HttpRequest) > > fetchFromServer is checking only if the response code is 4xx, and if so, it > is removing the access token from the store. This seems right for 401 or 403 > return codes, perhaps, but not for 404. The behavior for an end user would > then be that they have to do the OAuth dance again next time the gadget tries > to access a resource. > > The proposal is to change the current implementation to look explicitly for > 401 or 403 response codes in fetchFromServer instead of looking for any 4xx. > > Any other recommendations on what the behavior should be are welcome. > > > This addresses bug SHINDIG-1711. > https://issues.apache.org/jira/browse/SHINDIG-1711 > > > Diffs > ----- > > > http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth2/BasicOAuth2Request.java > 1292022 > > Diff: https://reviews.apache.org/r/3987/diff > > > Testing > ------- > > Built and ran existing JUnits. > > > Thanks, > > Stanton > >
