[ 
https://issues.apache.org/jira/browse/SHINDIG-1628?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13119503#comment-13119503
 ] 

[email protected] commented on SHINDIG-1628:
--------------------------------------------------------


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

Ship it!


I found two small nits that you could take or leave which I noted in the code 
-- other than that it looks good to me.


http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/HashLockedDomainService.java
<https://reviews.apache.org/r/2025/#comment5244>

    It might be good to log the exception here



http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/HashLockedDomainService.java
<https://reviews.apache.org/r/2025/#comment5245>

    You could consider using a StringBuilder here instead of a StringBuffer


- Jesse


On 2011-10-03 14:30:19, Dan Dumont wrote:
bq.  
bq.  -----------------------------------------------------------
bq.  This is an automatically generated e-mail. To reply, visit:
bq.  https://reviews.apache.org/r/2025/
bq.  -----------------------------------------------------------
bq.  
bq.  (Updated 2011-10-03 14:30:19)
bq.  
bq.  
bq.  Review request for shindig, Paul Lindner, johnfargo, Ryan Baxter, Jesse 
Ciancetta, and Stanton Sievers.
bq.  
bq.  
bq.  Summary
bq.  -------
bq.  
bq.  Sorry for the crazy diffs here.   Much stuff has moved around.
bq.  This is the cleanup part of the patch, I want a few good eyes first before 
I move on to the feature work.
bq.  
bq.  Some highlights:
bq.  
bq.  * org.apache.shindig.gadgets.uri.DefaultIframeUriManager
bq.  Nearly everything relating to locked domains has been moved to 
org.apache.shindig.gadgets.LockedDomainService
bq.  Removed validation on uri for locked domains from this class.   It was 
never actually used.
bq.  
bq.  * org.apache.shindig.gadgets.uri.ProxyUriBase
bq.  Removed check for INVALID_DOMAIN, nothing in the code paths leading there 
ever set that status.
bq.  
bq.  * org.apache.shindig.gadgets.uri.UriStatus
bq.  Removed INVALID_DOMAIN, it was not used anymore.  This class seems more 
focused on caching anyway.
bq.  
bq.  * org.apache.shindig.gadgets.HashLockedDomainService
bq.  Implemented new methods added to interface.  Renamed some methods for 
clarity and java convention.
bq.  Augmented some existing implementation from code that used to be in 
bq.  org.apache.shindig.gadgets.uri.DefaultIframeUriManager
bq.  
bq.  For documentation purposes:
bq.  I looked through what appears to be 3 proxies.  
bq.  * Content proxy - gadgets.io.getProxyUrl
bq.  * makeRequest proxy - gadgets.io.makeRequest
bq.  * RPC Proxy - osapi.http.get
bq.  
bq.  Content proxy denies all requests to a locked domain by default.  It's 
assumed that it's configured on a url that would ensure it is only used for 
things like image or sctipt tags, etc.
bq.  
bq.  makeRequest does not appear to do any locked domain checking to make sure 
the gadget is valid for the locked domain.  While it's reasonable to assume a 
malicious gadget will not use the locked domain url of another gadget, it's 
possible it could craft a request to the proxy on its own locked domain and 
forge the gadget passed in to appear as another gadget.  I'll be making changes 
to this proxy to include locked domain validation.
bq.  
bq.  RPC Proxy appears to be made from the container on behalf of a gadget, the 
gadget passed in should be legitimate.  I have not tried to make this request 
on a locked domain to see if the proxy will respond. (Gadget pretending to be 
the container making the request)
bq.  
bq.  
bq.  This addresses bug SHINDIG-1628.
bq.      https://issues.apache.org/jira/browse/SHINDIG-1628
bq.  
bq.  
bq.  Diffs
bq.  -----
bq.  
bq.    
http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/uri/UriManagerTestBase.java
 1178412 
bq.    
http://svn.apache.org/repos/asf/shindig/trunk/php/test/gadgets/ContainerConfigTest.php
 1178412 
bq.    
http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/uri/HashShaLockedDomainPrefixGeneratorTest.java
 1178412 
bq.    
http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/servlet/MakeRequestServletTest.java
 1178412 
bq.    
http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/servlet/ServletTestFixture.java
 1178412 
bq.    
http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/uri/DefaultIframeUriManagerTest.java
 1178412 
bq.    
http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/render/RendererTest.java
 1178412 
bq.    
http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/servlet/MakeRequestHandlerTest.java
 1178412 
bq.    
http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/servlet/MakeRequestHandler.java
 1178412 
bq.    
http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/uri/DefaultIframeUriManager.java
 1178412 
bq.    
http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/uri/HashShaLockedDomainPrefixGenerator.java
 1178412 
bq.    
http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/uri/LockedDomainPrefixGenerator.java
 1178412 
bq.    
http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/uri/ProxyUriBase.java
 1178412 
bq.    
http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/uri/UriStatus.java
 1178412 
bq.    
http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/HashLockedDomainServiceTest.java
 1178412 
bq.    
http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/oauth/GadgetOAuthCallbackGeneratorTest.java
 1178412 
bq.    
http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/GadgetException.java
 1178412 
bq.    
http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/HashLockedDomainService.java
 1178412 
bq.    
http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/LockedDomainService.java
 1178412 
bq.    
http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/oauth/GadgetOAuthCallbackGenerator.java
 1178412 
bq.    
http://svn.apache.org/repos/asf/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/render/Renderer.java
 1178412 
bq.    
http://svn.apache.org/repos/asf/shindig/trunk/content/samplecontainer/examples/SharedLockedDomainDemo2.xml
 PRE-CREATION 
bq.    
http://svn.apache.org/repos/asf/shindig/trunk/features/src/main/javascript/features/shared-script-frame/shared-script-frame-container.js
 1178412 
bq.    http://svn.apache.org/repos/asf/shindig/trunk/config/container.js 
1178412 
bq.    
http://svn.apache.org/repos/asf/shindig/trunk/content/samplecontainer/examples/SharedLockedDomainDemo1.xml
 PRE-CREATION 
bq.  
bq.  Diff: https://reviews.apache.org/r/2025/diff
bq.  
bq.  
bq.  Testing
bq.  -------
bq.  
bq.  
bq.  Thanks,
bq.  
bq.  Dan
bq.  
bq.


                
> Locked domain cleanup and shared-domain-locking feature
> -------------------------------------------------------
>
>                 Key: SHINDIG-1628
>                 URL: https://issues.apache.org/jira/browse/SHINDIG-1628
>             Project: Shindig
>          Issue Type: New Feature
>          Components: Java
>            Reporter: Dan Dumont
>         Attachments: upstream.patch
>
>


--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: 
https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira

        

Reply via email to