On Fri, 4 Feb 2022 16:34:14 GMT, Sean Mullan <[email protected]> wrote:
>> Xue-Lei Andrew Fan has updated the pull request incrementally with one
>> additional commit since the last revision:
>>
>> More update for the sec and impl
>
> src/java.base/share/classes/javax/net/ssl/SSLParameters.java line 735:
>
>> 733: * {@link #setSignatureSchemes} method has not been called, this
>> method
>> 734: * should return the default signature schemes for connection
>> populated
>> 735: * objects, or {@code null} for pre-populated objects.
>
> I think this sentence should be part of the specification and not the
> implementation note since ideally you would want all provider implementations
> to behave this way (assuming they supported this method):
>
> "If the {@link #setSignatureSchemes} method has not been called, this method
> should return the default signature schemes for connection populated objects,
> or {@code null} for pre-populated objects."
I agreed.
> src/java.base/share/classes/javax/net/ssl/SSLParameters.java line 752:
>
>> 750:
>> 751: /**
>> 752: * Sets the prioritized array of signature scheme names that
>
> If the system properties are set, does it override this API? I think it is
> important to mention that here in an @implNote. I know you discuss them in
> `getSignatureSchemes` but I think a developer is more likely to want to know
> the behavior of the properties for this method especially if they will
> override this setting.
I though it is a spec of the get method. But I agree with you that it could be
helpful if we have it in the set method as well. Updated.
> src/java.base/share/classes/javax/net/ssl/SSLParameters.java line 761:
>
>> 759: * Names Specification. Providers may support signature schemes
>> not defined
>> 760: * in this list or may not use the recommended name for a certain
>> 761: * signature scheme.
>
> Did you want to say something about what the behavior should be if a provider
> does not support one of these schemes? Is it ignored, or is an exception
> thrown?
Just as your suggested in a later comment, an apiNote was added.
> src/java.base/share/classes/javax/net/ssl/SSLParameters.java line 762:
>
>> 760: * in this list or may not use the recommended name for a certain
>> 761: * signature scheme.
>> 762: *
>
> Regarding my previous comment about providers not supporting these new
> methods, I think something like the following might suffice:
>
>
> @apiNote Note that a provider may not have been updated to support this
> method and in that case may ignore the schemes that are set.
> @implNote The SunJSSE provider supports this method.
Thank you for the suggestion. Updated.
-------------
PR: https://git.openjdk.java.net/jdk/pull/7252
