Code review request: 7088502 Security libraries don't build with javac -Werror

[Sean Mullan]

On 9/20/11 5:34 PM, Kurchi Hazra wrote:

>> * MessageDigestAlgorithm.java
>>
>> [74-5] This is preferable:
>>
>> static ThreadLocal<Map<String, MessageDigest>> instances=new
>>     ThreadLocal<HashMap<String, MessageDigest>>() {
>>     protected Map<String, MessageDigest> initialize()...
> The above does not work since the compiler complains that if ThreadLocal 
> is a HashMap, initialize() cannot override unless its return type is 
> also HashMap. Even if I change it to
> static ThreadLocal<Map<String, MessageDigest>> instances=new
>      ThreadLocal<HashMap<String, MessageDigest>>() {
>      protected HashMap<String, MessageDigest> initialize()...
> 
> the compiler throws an error as follows:
> 
> ../../../../../../src/share/classes/com/sun/org/apache/xml/internal/security/algorithms/MessageDigestAlgorithm.java:74: 
> error: incompatible types
>     static ThreadLocal<Map<String, MessageDigest>> instances=new
>                                                              ^
>    required: ThreadLocal<Map<String,MessageDigest>>
>    found: <anonymous ThreadLocal<HashMap<String,MessageDigest>>>

Try this instead?

    static ThreadLocal<Map<String, MessageDigest>> instances =
        new ThreadLocal<Map<String, MessageDigest>>() {

        protected Map<String, MessageDigest> initialValue() {
            return new HashMap<String, MessageDigest>();
        };
    };

>> * Canonicalizer20010315.java
>>
>> [209,314] I'm curious about these changes. Instead of adding a new 
>> method getSortedSetAsCollection and then suppressing the warnings for 
>> that, it seems like it would be sufficient to just suppress the 
>> warnings in this code, ex:
>>
>> @SuppressWarnings("unchecked")
>> ...
>>
>>  ns.getUnrenderedNodes(result);
> 
> 
> I did not put it inside the code, since the method has many lines of 
> code and this would mean suppressing unchecked warnings generated 
> anywhere in the method.

Ok, thanks for the explanation.

>> * SignatureAlgorithm.java
>>
>> [130, 399, 434] change type to Class<? extends SignatureAlgorithmSpi>
> If I make this change (and similar changes in other classes), I need to 
> cast at various places, and then I need to Suppress the unchecked 
> warnings. Is this preferable?
> ../../../../../../src/share/classes/com/sun/org/apache/xml/internal/security/algorithms/SignatureAlgorithm.java:412: 
> warning: [unchecked] unchecked cast
>                           
> SignatureAlgorithm._algorithmHash.put(algorithmURI, (Class <? extends 
> SignatureAlgorithmSpi>)Class.forName(implementingClass));
>                                                                                                                                     ^
>    required: Class<? extends SignatureAlgorithmSpi>
>    found:    Class<CAP#1>
>    where CAP#1 is a fresh type-variable:
>      CAP#1 extends Object from capture of ?
> 
> 
> Is there a workaround?

Not sure. If you can't figure out a workaround, then disregard my comment and
keep the current code.

Thanks,
Sean