(was Re: Code Review Request: TLS 1.3 Implementation)

Xuelei Fan at
Fri Jun 8 19:05:04 UTC 2018


On 6/6/2018 3:58 AM, Weijun Wang wrote:
> 1. I assume the toString() method does not change the internal status of a ByteBuffer? Is it worth mentioning this in the spec?
> 2. Should this interface and all its implementations be renamed to ***Stringnizer?
Good points!  Updated in the above changeset.

> 3. Lot of typo: ***Concumer. You can probably "cd sun/security/ssl" and run
>     perl -i -pe 's/Concumer/Consumer/g' *.java
The issues had been updated in another changeset.

> The whole SSLStringize + children classes looks complicated to me. Since the implementation is almost always creating a SSLExtensionSpec object and calling its toString, is it possible to register this SSLExtensionSpec class name in each enum value in SSLExtension and somehow simplify the design? Something like --
>     In constructor of SSLExtension, change the last parameter from
>     "SSLStringize stringize" to "Class<? extends SSLExtensionSpec> specClazz"
>     store the value into a private field named specClazz, and
>     then you can change "stringize.toString(byteBuffer)" to something like
>     "specClazz.newInstance(byteBuffer).toString()".
There is one or two exceptions that the 
specClazz.newInstance(byteBuffer) does not apply.  If we use this style, 
we would have a limit on the extension constructor.  I would prefer to 
have the flexibility in the SSLExtensionSpec implementation.


> That said, I know you must have thought about this much more. If there is any space for enhancement, we can always refactor it later.
> Thanks
> Max

More information about the security-dev mailing list