RFR: 8361711: Add library name configurability to PKCS11Test.java

[Thomas Fitzsimmons]

On Wed, 20 Aug 2025 02:13:17 GMT, Valerie Peng <valeriep at openjdk.org> wrote:

>> test/jdk/sun/security/pkcs11/PKCS11Test.java line 490:
>> 
>>> 488:             configFilePath = configFilePath.replaceFirst(
>>> 489:                     "(\\.[^\\.]*)?$", "-" + customConfigVariant + "$1");
>>> 490:         }
>> 
>> Hmm, I find it somewhat obscure that the config variant property changes the value of the config file name. With this new config variant property, it assumes that the confg file name has a "." which is probably true most if not all times. We should document all these properties so it's clear their precedence as well as the assumptions/implications.
>> All these security can be set independently, right? It's a bit strange that you set the CUSTOM_P11_CONFIG NAME and then setting the config variant property would actually changes the config file to a different name.
>
> Perhaps check the existence of the file and error out with the config file and its path if the check fails, this way, it's crystal clear.

Thank you for reviewing.

> Hmm, I find it somewhat obscure that the config variant property changes the value of the config file name.

Yes, I see your point.

> With this new config variant property, it assumes that the confg file name has a "." which is probably true most if not all times.

The regular expression supports appending to a file without a ".":


$ jshell -q
jshell> "kryoptic".replaceFirst("(\.[^\.]*)?$", "-" + "sensitive" + "$1");
$1 ==> "kryoptic-sensitive"


I should have added this case to the comment you mentioned above, will do in the expanded comment you requested.

> We should document all these properties so it's clear their precedence as well as the assumptions/implications. All these security can be set independently, right? It's a bit strange that you set the CUSTOM_P11_CONFIG NAME and then setting the config variant property would actually changes the config file to a different name.

Yes, conceptually I am treating file pairs like `p11-nss.txt` and `p11-nss-sensitive.txt` (and `p11-kryoptic.txt`, `p11-kryoptic-sensitive.txt`) as variants of the same configuration.  When `CUSTOM_P11_CONFIG_VARIANT` is set, `CUSTOM_P11_CONFIG_NAME`'s meaning becomes something like "base configuration file name".

Given the current test suite, and how I am specifying the use of Kryoptic, I wouldn't expect both `CUSTOM_P11_CONFIG_VARIANT` and `CUSTOM_P11_CONFIG_NAME` (or `CUSTOM_P11_CONFIG`) to be specified by the user at the same time.  `CUSTOM_P11_CONFIG_VARIANT` is meant for hard-coding in tests that invoke the test VM separately in sensitive and normal modes.

> Perhaps check the existence of the file and error out with the config file and its path if the check fails, this way, it's crystal clear.

OK, I can do that.  I will add a `/** ... */` block above `getNssConfig`.  These two changes will hopefully reduce the weirdness of the `CUSTOM_P11_CONFIG_NAME`/`CUSTOM_P11_CONFIG_VARIANT` combination.  I will also document the existing `CUSTOM_P11_CONFIG_NAME` versus `CUSTOM_P11_CONFIG` precedence since that might also be surprising when both are set.

-------------

PR Review Comment: https://git.openjdk.org/jdk/pull/26325#discussion_r2308810783