On Mon, 31 Jan 2022 21:55:18 GMT, Sean Mullan <mul...@openjdk.org> wrote:
>> Xue-Lei Andrew Fan has updated the pull request incrementally with one
>> additional commit since the last revision:
>>
>> Rollback to use captialized S
>
> src/java.base/share/classes/javax/net/ssl/SSLParameters.java line 727:
>
>> 725: * Note that the underlying provider may define the default
>> signature
>> 726: * schemes for each SSL/TLS/DTLS connection. Applications may also
>> use
>> 727: * System Property, "jdk.tls.client.SignatureSchemes" and/or
>
> Use the javadoc annotation @systemProperty when referring to the system
> properties. Look at other code for examples.
Thanks! Updated.
> src/java.base/share/classes/javax/net/ssl/SSLParameters.java line 734:
>
>> 732: * pre-populated objects.
>> 733: *
>> 734: * @return a non-null, possibly zero-length array of signature
>> scheme
>
> The other methods that return arrays like `getCipherSuites` and
> `getProtocols` return `null` if none have been set. While one could argue
> whether returning `null` or an empty array is better, there is already a
> precedent in this API of returning `null`, and with this API programmers will
> have to check for two different ways of checking for parameters that are not
> set. I'm not really sure if the inconsistency is worth it.
Yes, it is arguable. The spec is updated, I will update the implementation if
we are on the same page for the specification part.
> src/java.base/share/classes/javax/net/ssl/SSLParameters.java line 765:
>
>> 763: * System Property, "jdk.tls.client.SignatureSchemes" and/or
>> 764: * "jdk.tls.server.SignatureSchemes", to customize the
>> provider-specific
>> 765: * default signature schemes. If the {@code signatureSchemes} array
>> is
>
> In this sentence does `signatureSchemes` mean the "*.SignatureSchemes"
> property or the `signatureSchemes` parameter? I don't think I understand this
> sentence. I think you should explain when the system property overrides the
> parameter in this API, and/or vice-versa.
I removed this section in the setSignatureSchemes() method, and add more
details in the getSignatureSchemes().
> src/java.base/share/classes/javax/net/ssl/SSLParameters.java line 774:
>
>> 772: * is empty (zero-length), the provider-specific default
>> signature
>> 773: * schemes will be used for the SSL/TLS/DTLS connection.
>> 774: * @throws IllegalArgumentException if signatureSchemes is null, or
>> if
>
> The other methods that accept arrays like `setCipherSuites` and
> `setProtocols` accept `null` as a parameter. Like my previous comment, it
> seems more important to keep this behavior consistent in the API and allow
> `null` as an acceptable value, which is also useful for clearing the current
> value of the parameter.
Updated.
-------------
PR: https://git.openjdk.java.net/jdk/pull/7252
