On Tue, 1 Feb 2022 06:47:00 GMT, Xue-Lei Andrew Fan <xue...@openjdk.org> wrote:
>> This update is to support signature schemes customization for individual >> (D)TLS connection. Please review the CSR as well: >> CSR: https://bugs.openjdk.java.net/browse/JDK-8280495 >> RFE: https://bugs.openjdk.java.net/browse/JDK-8280494 > > Xue-Lei Andrew Fan has updated the pull request incrementally with one > additional commit since the last revision: > > Allow null input and return values A few more comments on the API. src/java.base/share/classes/javax/net/ssl/SSLParameters.java line 706: > 704: * over the SSL/TLS/DTLS protocols. > 705: * <p> > 706: * Note that the standard list of signature scheme names may be > found in s/may be found/are defined/ src/java.base/share/classes/javax/net/ssl/SSLParameters.java line 710: > 708: * > "{@docRoot}/../specs/security/standard-names.html#signature-schemes"> > 709: * Signature Schemes</a> section of the Java Security Standard > Algorithm > 710: * Names Specification. Providers may support signature schemes not > found s/found/defined/ src/java.base/share/classes/javax/net/ssl/SSLParameters.java line 711: > 709: * Signature Schemes</a> section of the Java Security Standard > Algorithm > 710: * Names Specification. Providers may support signature schemes not > found > 711: * in this list or might not use the recommended name for a certain s/might/may/ src/java.base/share/classes/javax/net/ssl/SSLParameters.java line 714: > 712: * signature scheme. > 713: * <p> > 714: * The returned array could be {@code null}, in which case the > underlying s/could be/may be/ Suggest rewording as "If the returned array is {@code null}, then the underlying ..." src/java.base/share/classes/javax/net/ssl/SSLParameters.java line 718: > 716: * SSL/TLS/DTLS connections. > 717: * <p> > 718: * The returned array could be empty (zero-length), in which case the Suggest rewording as "If the returned array is empty (zero-length), then the ..." src/java.base/share/classes/javax/net/ssl/SSLParameters.java line 720: > 718: * The returned array could be empty (zero-length), in which case the > 719: * signature scheme negotiation mechanism is turned off for > SSL/TLS/DTLS > 720: * protocols, and the connections may not be able estabilished if the s/estabilished/to be established/ src/java.base/share/classes/javax/net/ssl/SSLParameters.java line 723: > 721: * negotiation mechanism is required by a certain SSL/TLS/DTLS > protocol. > 722: * <p> > 723: * The returned array could be neither {@code null} nor empty > (zero-length), Suggest rewording as "If the returned array is not {@code null} or empty (zero-length), then the signature schemes .." src/java.base/share/classes/javax/net/ssl/SSLParameters.java line 730: > 728: * For non-null returns, this method will return a new array each > time it > 729: * is invoked. Providers should ignore unknown signature scheme names > 730: * while establishing the SSL/TLS/DTLS connections. I think this condition should be part of the API specification, and not the implementation specification. This needs to be a required part of the API, otherwise applications cannot safely rely on other implementations of this class to return copies. I suggest moving it to the `@returns` clause. src/java.base/share/classes/javax/net/ssl/SSLParameters.java line 736: > 734: * schemes for each SSL/TLS/DTLS connection. Applications may also > use > 735: * System Property, {@systemProperty jdk.tls.client.SignatureSchemes} > 736: * and/or {@systemProperty jdk.tls.server.SignatureSchemes}, to > customize Suggest rewording as "the {@systemProperty jdk.tls.client.SignatureSchemes} and/or {@systemProperty jdk.tls.server.SignatureSchemes} system properties to customize ..." src/java.base/share/classes/javax/net/ssl/SSLParameters.java line 742: > 740: * objects, or {@code null} for pre-populated objects. > 741: * > 742: * @return {@code null} or an array of signature scheme {@code > String}s. Suggest rephrasing as "an array of signature scheme {@code Strings} or {@null} if none have been set." src/java.base/share/classes/javax/net/ssl/SSLParameters.java line 757: > 755: * can be used over the SSL/TLS/DTLS protocols. > 756: * <p> > 757: * Note that the standard list of signature scheme names may be > found in s/may be found/are defined/ src/java.base/share/classes/javax/net/ssl/SSLParameters.java line 761: > 759: * > "{@docRoot}/../specs/security/standard-names.html#signature-schemes"> > 760: * Signature Schemes</a> section of the Java Security Standard > Algorithm > 761: * Names Specification. Providers may support signature schemes not > found s/found/defined/ src/java.base/share/classes/javax/net/ssl/SSLParameters.java line 762: > 760: * Signature Schemes</a> section of the Java Security Standard > Algorithm > 761: * Names Specification. Providers may support signature schemes not > found > 762: * in this list or might not use the recommended name for a certain s/might/may/ src/java.base/share/classes/javax/net/ssl/SSLParameters.java line 766: > 764: * > 765: * @implSpec > 766: * This method will make a copy of the {@code signatureSchemes} > array. Should be part of API specification (see my similar comment on `getSignatureSchemes`. src/java.base/share/classes/javax/net/ssl/SSLParameters.java line 769: > 767: * > 768: * @param signatureSchemes {@code null} or an ordered array of > signature > 769: * scheme names, with the first entry being the most > preferred. Suggest rewording as: "an ordered array of signature scheme names with the first entry being the most preferred, or {@null}." src/java.base/share/classes/javax/net/ssl/SSLParameters.java line 770: > 768: * @param signatureSchemes {@code null} or an ordered array of > signature > 769: * scheme names, with the first entry being the most > preferred. > 770: * @throws IllegalArgumentException if any element in the non-empty I don't think the word "non-empty" is necessary as an empty array has no elements. src/java.base/share/classes/javax/net/ssl/SSLParameters.java line 786: > 784: if (scheme == null || scheme.isBlank()) { > 785: throw new IllegalArgumentException( > 786: "An element of signatureSchemes was null or > blank"); s/was/is/ ------------- PR: https://git.openjdk.java.net/jdk/pull/7252
