jdk.tls.handleCertReqAuthoritesOverflow is a little bit long. jdk.tls.server.overFlowAuthorites?
May not need to define the "none" property value. Using enum for HAO_NONE, HAO_EMPTY and HAO_TRUNC? line 1920: Overflow should be rare, what about print only when overflow? Xuelei On 4/26/2016 9:56 PM, Seán Coffey wrote: > Looks like a fair approach to solving this issue Ivan. A few comments > from me : > > typo : authoririesOverflow --> authoritiesOverflow > typo : handleAuthoritesOverflow --> handleAuthoritiesOverflow > typo : jdk.tls.handleCertReqAuthoritesOverflow --> > jdk.tls.handleCertReqAuthoritiesOverflow > > + throw new RuntimeException("Value of " + prop > + + " must be one of '" + HAO_NONE + "', '" > + + HAO_EMPTY + "', '" + HAO_TRUNC + "'"); > > I think it would be good to print the value of s in above exception > also. something like + ". Received: \"" + s + "\""); > == > > s.println("Cert Authorities:" + (authoririesOverflow ? " (overflow)" : > "")); > > I would also be good to indicate the handleAuthoritiesOverflow string > value in above printing *if* authoritiesOverflow turns out to be true. > We should be able to determine from the next message printed - but no > harm to future proof. > Maybe : > > s.println("Cert Authorities:" + (authoritiesOverflow ? " (overflow" + > "[" + handleAuthoritiesOverflow + "])" : "")); > > Regards, > Sean. > > On 26/04/2016 11:57, Ivan Gerasimov wrote: >> Here's a modified version of the fix. >> >> Instead of a boolean-type property, a string-type property is introduced. >> It is used to specify the strategy to use, if we encounter the >> overflow during filling the list of authorities. >> >> The default strategy is to throw an exception (just like the currently >> implemented behavior.) >> >> It can also be set to the values 'empty' or 'truncate', which will >> make the server to send an empty or truncated list upon overflow. >> >> Would you please help review it? >> >> http://cr.openjdk.java.net/~igerasim/8154947/01/webrev/ >> >> With kind regards, >> Ivan >> >> >> On 22.04.2016 20:09, Ivan Gerasimov wrote: >>> Hello everyone! >>> >>> During TLS handshake, a server may be required to send a >>> CertificateRequest, which contains a list of authorities. >>> If the list happens to be too long, the server is throwing an >>> exception, indicating an overflow. >>> >>> It may be convenient to be able to just drop the list altogether, and >>> let the client to choose a certificate randomly. >>> In certain situation this may be more preferable that just block >>> communication. >>> >>> Would you please help review a patch, which introduces an >>> command-line option that controls this behavior of the server? >>> If the approach is approved, I'll file a CCC request for that option. >>> >>> BUGURL: https://bugs.openjdk.java.net/browse/JDK-8154947 >>> WEBREV: http://cr.openjdk.java.net/~igerasim/8154947/00/webrev/ >>> >>> With the proposed fix all the security-related regression tests, >>> including the modified one, passed on all supported platforms. >>> >>> With kind regards, >>> Ivan >>> >>> >> >