> On 2012-03-02 12:52:49, Stanton Sievers wrote:
> > Hi Henry,
> > 
> > You should also remove the logic in 
> > org.apache.shindig.auth.BlobCrypterSecurityToken.fromToken(SecurityToken) 
> > where we eat the UnsupportedOperationException when calling getActiveUrl(). 
> >  
> > 
> > I'm also looking at 
> > org.apache.shindig.gadgets.oauth.GadgetOAuthCallbackGenerator.checkGadgetCanRender(SecurityToken,
> >  OAuthArguments, OAuthResponseParams) and it seems like a value of "null" 
> > here is going to throw other exceptions when create a Uri.parse.  I'm 
> > wondering if we can handle "null" more gracefully here... although we 
> > really weren't handling UnsupportedOperationException gracefully here 
> > either.
> > 
> > Do you know why the exception was being thrown in the first place?  Looking 
> > at the rest of the APIs in SecurityToken that one strikes me as odd.

Thanks for the BlobCrypterSecurityToken.fromToken change. 
 
Yeah, I am aware of the checkGadgetCanRender call. I wouldnt worry about it tho 
bc most of times the UrlParameterAuthenticationHandler should do its job to 
inject the right activeUrl to the token.


- Henry


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/4135/#review5556
-----------------------------------------------------------


On 2012-03-01 23:38:41, Henry Saputra wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/4135/
> -----------------------------------------------------------
> 
> (Updated 2012-03-01 23:38:41)
> 
> 
> Review request for shindig.
> 
> 
> Summary
> -------
> 
> Simplify the SecurityToken.getActiveUrl implementation and definition.
> 
> If not set simply return null should be suffice.
> 
> 
> This addresses bug SHINDIG-1719.
>     https://issues.apache.org/jira/browse/SHINDIG-1719
> 
> 
> Diffs
> -----
> 
>   
> trunk/java/common/src/main/java/org/apache/shindig/auth/BasicSecurityToken.java
>  1295758 
>   
> trunk/java/common/src/main/java/org/apache/shindig/auth/BlobCrypterSecurityToken.java
>  1295758 
>   trunk/java/common/src/main/java/org/apache/shindig/auth/SecurityToken.java 
> 1295758 
> 
> Diff: https://reviews.apache.org/r/4135/diff
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Henry
> 
>

Reply via email to