On Fri, 4 Feb 2022 16:34:14 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: >> >> 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