On Thu, 30 Oct 2025 21:56:15 GMT, Artur Barashev <[email protected]> wrote:
>> Matthew Donovan has updated the pull request incrementally with one
>> additional commit since the last revision:
>>
>> changed tests to use SecurityUtils.removeDisabled*Algs methods
>
> test/jdk/javax/net/ssl/HttpsURLConnection/CriticalSubjectAltName.java line
> 221:
>
>> 219: if (args[1].contains("MD5")) {
>> 220: // MD5 is used in this test case, don't disable MD5
>> algorithm.
>> 221:
>> SecurityUtils.removeFromDisabledAlgs("jdk.certpath.disabledAlgorithms",
>
> Line length > 80. Also, I think `If MD5 is used ..` comment would be better
> because we don't always use MD5. Here and in other tests.
I removed the comment entirely. It seemed redundant to say "if MD5 is used"
right below the `if(args[0].equals("MD5")`
> test/jdk/sun/net/www/protocol/https/HttpsURLConnection/IdentitiesBase.java
> line 184:
>
>> 182: SSLContext ctx = SSLContext.getInstance(protocol);
>> 183:
>> 184: if (keyCert != null) {
>
> Why do we check for keyCert being `null`? Do we expect some future tests to
> make use of it? BTW, in such case `CertificateBuilder.printCertificate` would
> fail first.
I think that was leftover from refactoring the code. I removed the null checks
and made the method private for now.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/27342#discussion_r2481353898
PR Review Comment: https://git.openjdk.org/jdk/pull/27342#discussion_r2481352676
