----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/1480/#review1428 -----------------------------------------------------------
Ship it! LGTM with small nit. http://svn.apache.org/repos/asf/shindig/trunk/java/common/src/test/java/org/apache/shindig/auth/BlobCrypterSecurityTokenCodecTest.java <https://reviews.apache.org/r/1480/#comment3293> Update the comment to indicate input argument is location of the file. - Henry On 2011-08-12 17:17:30, Jesse Ciancetta wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/1480/ > ----------------------------------------------------------- > > (Updated 2011-08-12 17:17:30) > > > Review request for shindig. > > > Summary > ------- > > Patch to enable loading security token key from either an absolute filesystem > reference (as it does currently) or from the classpath. > > After I finished writing the patch I went to create a JIRA ticket for it in > the Shindig project and realized I should search to see if one already > existed -- and sure enough I found one, complete with a patch which is quite > similar to mine! :) > > It looks like the issues with the previous patch were formatting issues and > the fact that the previous patch removed the extensibility hook in > BlobCrypterSecurityTokenCodec (the loadCrypterFromFile method allowing > implementers to plug in their own BlobCrypter) -- luckily this patch does not > suffer from either or those issues (at least I don't think it does!). > > It should be noted however that the loadCrypterFromFile method *has* been > renamed to loadCrypter (breaking backwards compatibility with the existing > API), however since we're moving from a 2.X to a 3.X version this would seem > acceptable. > > > This addresses bugs RAVE-173 and SHINDIG-811. > https://issues.apache.org/jira/browse/RAVE-173 > https://issues.apache.org/jira/browse/SHINDIG-811 > > > Diffs > ----- > > > http://svn.apache.org/repos/asf/shindig/trunk/java/common/src/main/java/org/apache/shindig/auth/BlobCrypterSecurityTokenCodec.java > 1036287 > > http://svn.apache.org/repos/asf/shindig/trunk/java/common/src/main/java/org/apache/shindig/common/crypto/BasicBlobCrypter.java > 1067589 > > http://svn.apache.org/repos/asf/shindig/trunk/java/common/src/test/java/org/apache/shindig/auth/BlobCrypterSecurityTokenCodecTest.java > 1036287 > > http://svn.apache.org/repos/asf/shindig/trunk/java/common/src/test/java/org/apache/shindig/auth/DefaultSecurityTokenCodecTest.java > 1036287 > > Diff: https://reviews.apache.org/r/1480/diff > > > Testing > ------- > > All tests are passing and additional testing has been done in Tomcat with the > different loading mechanisms (absolute file reference and classpath > reference). > > > Thanks, > > Jesse > >
