RFR: 8349492: Update sun/security/pkcs12/KeytoolOpensslInteropTest.java to use a recent Openssl version [v5]
Fernando Guallini
fguallini at openjdk.org
Wed Mar 5 21:24:56 UTC 2025
On Wed, 5 Mar 2025 19:45:14 GMT, Matthew Donovan <mdonovan at openjdk.org> wrote:
>> Fernando Guallini has updated the pull request incrementally with one additional commit since the last revision:
>>
>> updated wording of openssl version
>
> test/jdk/sun/security/pkcs12/KeytoolOpensslInteropTest.java line 78:
>
>> 76: testWithOpensslCommands(opensslPath);
>> 77: } else {
>> 78: // since the current version of openssl is not available, skip all
>
> If this test is to check interoperability with OpenSSL, shouldn't we throw a skippedexception or an error instead of just running with java?
Well, the test is also checking with java commands if openssl is available (line 75), then it makes sense to keep it when it is not available as it does not rely on Openssl.
> test/lib/jdk/test/lib/security/OpensslArtifactFetcher.java line 79:
>
>> 77: }
>> 78: }
>> 79: return verifyOpensslVersion(path, OPENSSL_BUNDLE_VERSION) ? path : null;
>
> Do we want to keep this version check? On the one hand, it ensures that the system binaries or binaries specified via the system property will be a specific, known version, but on the other hand, the only way to run this test with a different version of the library is to change the code. Instead, can we just log the version that is used when the test is run?
I considered removing this check as it seems redundant, but then I realised it was introduced to ensure the bundle version matches the version specified by the artifact name. It's possible that the artifact's filename could show a different version than the actual binaries with `-version`. On the other hand, most tests don't run this extra verification, that could be a good reason to remove it.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/23613#discussion_r1982115420
PR Review Comment: https://git.openjdk.org/jdk/pull/23613#discussion_r1982136351
More information about the security-dev
mailing list