code review request: 7171982 Cipher getParameters() throws RuntimeException: Cannot find SunJCE provider

[Brad Wetmore]

(Whoops, was working on two reviews with two related comments, and 
reversed the emails).

Just realized, there are no regression tests here.

Simplest is to probably do as much setup as you can, then 
java.security.Security.removeProvider("SunJCE"), then issue the calls 
that call into these changes.  They should all pass in the new version, 
and fail in the old.

Brad



On 3/28/2013 2:34 PM, Brad Wetmore wrote:
> (Vinnie, what do you think about the SunJCE item below?)
>
> On 3/22/2013 11:57 AM, Anthony Scarpino wrote:
>> Hi all,
>>
>> I need a code review for below webrev.  The changes are to have SunJCE
>> call itself, using it's current instance, for checking such things as
>> parameters, instead of searching through the provider list or creating a
>> one time instance.
>>
>> http://cr.openjdk.java.net/~mullan/webrevs/ascarpin/webrev.00/
>
> PBES1Core.java
> ==============
> 173:  indention problem.  Should be at the same level as (algo...)
>
> PBES2Core.java:173
> PKCS12PBECipherCore.java:147
> SealedObjectForKeyProtector:50/57
> ========================
> Indention problem. Normally 4 spaces unless you're trying to line it up
> with something.
>
> SealedObjectForKeyProtector.java
> ================================
> 54/57:  In general, you should initCause() everywhere you possibly can.
>   This will help people (us) debug the real underlying root cause,
> instead of just the top-level error message.
>
> SunJCE.java
> ===========
> 781:  Your code could race during initialization and potentially have
> many SunJCE instances active at once.
>
> Either make instance a volatile (will reduce some of the race
> opportunity), or instead, add locking around assignment/use.  You may
> still be creating multiple SunJCEs, but only one instance will ever be
> obtained from getInstance:
>
>      synchronized (SunJCE.class) {
>          if (instance == null) {
>              instance = this;
>          }
>      }
>
> and
>
>      static SunJCE getInstance() {
>          if (instance == null) {
>              new SunJCE();
>          }
>          synchronized (SunJCE.class) {
>              return instance;
>          }
>      }
>
> Also, when you get ready to push, be sure to address also the closed
> side: that is, please remember to build/integrate the signed
> sunjce_provider.jar file in the closed repo.
>
> HTH,
>
> Brad
>
>