[8u] TLSv1.3 RFR: 8251478: Backport TLSv1.3 regression tests to JDK8u

Martin Balao mbalao at redhat.com
Tue Aug 25 22:22:43 UTC 2020


On 8/25/20 5:37 PM, Alexey Bakhtin wrote:
> New webrev: http://cr.openjdk.java.net/~abakhtin/tls1.3/8245466/8251478/webrev.v2
> Git diff:  http://cr.openjdk.java.net/~abakhtin/tls1.3/8245466/8251478/webrev.v2/jdk.git.diff

>> * test/sun/security/ssl/SSLSocketImpl/SSLSocketKeyLimit.java
>>  * 8u's equivalent of InputStream::readNBytes is IOUtils.readNBytes
> OK. Fixed. But actually, it does not matter How many bytes
> read from the stream  in this particular case.

In this file you are not throwing "Throwable" anymore (diff between
Webrev.01 and 02). Was that intended?

>>  * Just out of curiosity, why 'main' method needs to throw Throwable
>> instead of Exception? I've seen this change in other tests too.
> In JDK8 jdk.testlibrary.ProcessTools.executeProcess() throws Throwable instead of Exception. This is a reason of this fix.

Ah, okay.

>>
>> * test/sun/security/ssl/internal/java.base/sun/security/ssl/TestHkdf.java
>> *
>> test/sun/security/ssl/Stapling/java.base/sun/security/ssl/StatusResponseManagerTests.java
>>  * Why did you relocate these tests? I'd keep them in their original
>> location if possible.
> It was part of task to exclude modularisation dependencies. But you are right. It is not necessary. Reverted.

Ok. Thanks.

>>
>> * test/sun/security/ssl/X509TrustManagerImpl/Symantec/Distrust.java
>>  * I've seen the jdk.test.lib.security.SecurityUtils import was removed
>> but looks to me that it's used. I might be overlooking something.
>>
> JDK8 already has SecurityUtils.java class in the test/lib/security directory. This class is in the unnamed package. So, Distrust.java is updated to use original SecurityUtils.java class
> 

Ah, good.

>> * MFLNTest tests
>>  * I'd try something different, to minimize the number of changes.
>> Instead of having scripts that build a JAR and include the new
>> MaxPacketSize class in the sun.security.ssl package to access internal
>> fields; I'd only change SSLEngineTestCase::doHandshake method to set the
>> proper max packet size by means of reflection.
>> SSLEngineTestCase::doHandshake has to be edit anyways to reflect the
>> lack of an API to set the max fragment size. This approach should also
>> avoid a change in MFLNTest class. What do you think?
> 
> Yes, you are right. Thank you. These tests could be simplified by using reflection. Fixed.

Good!

In test/sun/security/ssl/internal/TestRun.sh, you commented the line
that removes the JAR file. Looks to me that you want to revert that change.

I'll now run all the SSL-related tests and expect them to pass. Is that
expectation correct? Let me know if there is any known failure.

Thanks,
Martin.-



More information about the jdk8u-dev mailing list