[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