[8u] RFR 8254631: Better support ALPN byte wire values in SunJSSE

Severin Gehwolf sgehwolf at redhat.com
Mon Apr 26 12:39:01 UTC 2021


Hi Martin,

Just a nit on the changed test. Looks fine otherwise.

On Thu, 2021-04-22 at 16:14 -0300, Martin Balao wrote:
> Hi,
> 
> I'd like to request a review for the 8u backport of JDK-8254631 [1].
> 
> Webrev.00:
> 
>  *
>  http://cr.openjdk.java.net/~mbalao/webrevs/8254631/8254631.webrev.jdk8u.jdk.00/
> 
> The 11u patch does not apply cleanly because of the following reasons:
> 
>  * Paths need to be converted to the old scheme

OK.

>  * java.security changes need to be propagated through the different
> java.security-<os> files

OK.

>  * AlpnGreaseTest.java uses a java.util.Arrays public API not available
> in 8u (introduced by JDK-8033148 [2]).
>   * Fixed creating copies of the arrays to compare.

  89             System.arraycopy(bytes, i, a1, 0, greaseBytes.length);
  90             System.arraycopy(greaseBytes, 0, a2, 0, greaseBytes.length);
  91             if (Arrays.equals(a1, a2)) {

I don't think we'd need to copy greaseBytes for this array comparison,
do we? Can't we use 'Arrays.equals(a1, greaseBytes)'?

Better yet, don't do a copy at all and just do a simple search using a
ByteBuffer like this:
https://cr.openjdk.java.net/~sgehwolf/webrevs/JDK-8254631.test-fix.patch

 * AlpnGreaseTest.java uses ByteBuffer::flip that returns a ByteBuffer.
In 8u JDK-4774077 [3] is not present so Buffer::flip gets called and a
Buffer is returned. This is a problem because ::get(byte[]) is then
invoked and this is not available in the Buffer class (but in
ByteBuffer).
  * Fixed with a simple casting.

OK.

Thanks,
Severin



More information about the jdk8u-dev mailing list