SSLStringize.java (was Re: Code Review Request: TLS 1.3 Implementation)

[Xuelei Fan]

Update: http://hg.openjdk.java.net/jdk/sandbox/rev/25178bb3e8f5

On 6/6/2018 3:58 AM, Weijun Wang wrote:
> SSLStringize.java:
> 
> 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.

Thanks,
Xuelei

> 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
>