You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@shindig.apache.org by Jesse Ciancetta <jc...@mitre.org> on 2011/08/12 19:17:30 UTC
Review Request: Enable loading security token key from either an absolute
filesystem reference or from the classpath
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/1480/
-----------------------------------------------------------
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
Re: Review Request: Enable loading security token key from either an
absolute filesystem reference or from the classpath
Posted by Henry Saputra <hs...@apache.org>.
-----------------------------------------------------------
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
>
>
RE: Review Request: Enable loading security token key from either
an absolute filesystem reference or from the classpath
Posted by "Ciancetta, Jesse E." <jc...@mitre.org>.
Thanks Henry and Paul!
I've gone ahead and closed out the review request as submitted.
>-----Original Message-----
>From: Henry Saputra [mailto:henry.saputra@gmail.com]
>Sent: Friday, August 12, 2011 10:55 PM
>To: dev@shindig.apache.org
>Subject: Re: Review Request: Enable loading security token key from either an
>absolute filesystem reference or from the classpath
>
>Cool, thanks Paul.
>
>Closing SHINDIG-811 with this rev.
>
>- Henry
>
>On Fri, Aug 12, 2011 at 6:41 PM, Paul Lindner <li...@inuus.com> wrote:
>>
>> -----------------------------------------------------------
>> This is an automatically generated e-mail. To reply, visit:
>> https://reviews.apache.org/r/1480/#review1439
>> -----------------------------------------------------------
>>
>> Ship it!
>>
>>
>> committing with henry's suggestion.
>>
>> - Paul
>>
>>
>> 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/j
>ava/org/apache/shindig/auth/BlobCrypterSecurityTokenCodec.java 1036287
>>> http://svn.apache.org/repos/asf/shindig/trunk/java/common/src/main/j
>ava/org/apache/shindig/common/crypto/BasicBlobCrypter.java 1067589
>>> http://svn.apache.org/repos/asf/shindig/trunk/java/common/src/test/ja
>va/org/apache/shindig/auth/BlobCrypterSecurityTokenCodecTest.java
>1036287
>>> http://svn.apache.org/repos/asf/shindig/trunk/java/common/src/test/ja
>va/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
>>>
>>>
>>
>>
Re: Review Request: Enable loading security token key from either an
absolute filesystem reference or from the classpath
Posted by Henry Saputra <he...@gmail.com>.
Cool, thanks Paul.
Closing SHINDIG-811 with this rev.
- Henry
On Fri, Aug 12, 2011 at 6:41 PM, Paul Lindner <li...@inuus.com> wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/1480/#review1439
> -----------------------------------------------------------
>
> Ship it!
>
>
> committing with henry's suggestion.
>
> - Paul
>
>
> 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
>>
>>
>
>
Re: Review Request: Enable loading security token key from either an
absolute filesystem reference or from the classpath
Posted by Paul Lindner <li...@inuus.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/1480/#review1439
-----------------------------------------------------------
Ship it!
committing with henry's suggestion.
- Paul
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
>
>
Re: Review Request: Enable loading security token key from either an absolute filesystem reference or from the classpath
Posted by "Davies,Douglas" <da...@oclc.org>.
Excellent. I had just done something similar. Thanks.
Doug
Sent from my iPhone
On Aug 12, 2011, at 11:17 AM, "Jesse Ciancetta" <jc...@mitre.org> wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/1480/
> -----------------------------------------------------------
>
> 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
>