If no objections, I will push the changeset. Xuelei
On 9/4/2013 11:34 AM, Xuelei Fan wrote: > new webrev: http://cr.openjdk.java.net/~xuelei/7188657/webrev.01/ > > On 9/4/2013 10:35 AM, Weijun Wang wrote: >> Mostly good, only some word/style issues. >> >> SSLParameters.java: >> >> 83 * server name matchers are set to <code>null</code>, cipher >> suites >> 84 * preference, wantClientAuth and needClientAuth are set to >> 85 * <code>false</code>. >> >> Why not just use "preferLocalCipherSuites" instead of "cipher suites >> preference"? Yes it looks ugly to refer to a variable name, but you've >> already used "wantClientAuth". Or, at least use "useCipherSuitesOrder" >> because that's used in the public method names. >> > Good catch. I will use "useCipherSuitesOrder". > >> Handshaker.java: >> >> 148 // Whether local cipher suites preference in server side should be >> 149 // honored during handshaking? >> 150 boolean preferLocalCipherSuites = false; >> >> Since you apply the flag to both server and client, how about adding >> something like "(it's always honored in client side)". >> > Better to have such words. > > // Whether local cipher suites preference should be honored during > // handshaking? > // > // Note that in this provider, this option only applies to server side. > // Local cipher suites preference is always honored in client side in > // this provider. > > >> 550 boolean isNegotiable(CipherSuite s) { >> >> You might need to update the doc for this method saying "within the >> current active cipher suites". You can even let it call the new >> isNegotiable(*,*) method. >> > Good suggestion. > >> UseCipherSuitesOrder.java: >> >> 2 * Copyright (c) 2001, 2011, Oracle and/or its affiliates. All >> rights reserved. >> >> Should be "2001, 2013". >> > Should be 2013. > >> 157 // client enabled cipher suites >> 158 private static String[] CliEnabledCipherSuites; >> 159 >> 160 // server enabled cipher suites >> 161 private static String[] SrvEnabledCipherSuites; >> >> It looks weird for a variable to starts with a capital letter. >> > It looks weird to me, too. Not sure what I was thinking at that time. > > Thanks, > Xuelei > >> Thanks >> Max >> >> On 8/28/13 5:02 PM, Xuelei Fan wrote: >>> Hi, >>> >>> Please review this update to support cipher suites reorder: >>> >>> webrev: http://cr.openjdk.java.net/~xuelei/7188657/webrev.00/ >>> >>> Two new methods are added to SSLParameters: >>> public final void setUseCipherSuitesOrder(boolean honorOrder); >>> public final boolean getUseCipherSuitesOrder(); >>> >>> If SSLParameters.getUseCipherSuitesOrder() return true, the local cipher >>> suites order returned in SSLParameters.getCipherSuites() should be >>> honored during SSL/TLS handshaking. >>> >>> Considering the potential compatibility issues of third party's >>> implementation, I won't define the behaviors if >>> SSLParameters.getUseCipherSuitesOrder() return false. For Oracle >>> provider, SunJSSE, if getUseCipherSuitesOrder() returns false, the order >>> of SSLParameters.getCipherSuites() is honored in client side, and the >>> order of the requested cipher suites in client handshake message is >>> honored in server side. >>> >>> Thanks, >>> Xuelei >>> >
