On Tue, 18 Aug 2026 13:31:52 GMT, Alan Bateman <[email protected]> wrote:

>> src/jdk.jlink/share/classes/jdk/tools/jlink/internal/plugins/SecurityPropertiesPlugin.java
>>  line 123:
>> 
>>> 121:                     // assume "=" used as delimiter
>>> 122:                     int index = line.indexOf('=');
>>> 123:                     if (index != -1) {
>> 
>> @seanjmullan: assuming `=` would likely discard any `include` or unusual 
>> property in the original build-time `java.security` file.
>> 
>> Perhaps you have already ruled out the following for some reason, but 
>> ideally we should reuse the properties file parsing logic, automatically 
>> supporting everything documented in 
>> [java.util.Properties::load(java.io.Reader)](https://docs.oracle.com/en/java/javase/26/docs/api/java.base/java/util/Properties.html#load(java.io.Reader)).
>> 
>> We could decuple the parsing logic from the `Hashtable`, so we can have a 
>> version that preserves the order. For example, move 
>> `java.util.Properties.LineReader` and a modified version of 
>> `java.util.Properties::load0` to a common place. The modified version allows 
>> line 459 to be customized with a callback:
>> https://github.com/openjdk/jdk/blob/79aa0d7af1264737d1724cee039f4f02315aee70/src/java.base/share/classes/java/util/Properties.java#L413-L461
>> 
>> The new `java.util.Properties::load0` passes `(key, value) -> put(key, 
>> value)` as the callback, while the _security properties jlink plugin_ uses a 
>> callback with the logic of replacing the value if present in the 
>> user-provided file, loaded in `Map<String, String> props`.
>> 
>> ---
>> 
>> An intermediary step would be at least using 
>> `java.util.Properties.LineReader` for a unified line continuation parser.
>> 
>> ---
>> 
>> If you dislike the idea, shouldn't we document this behavior somewhere? 
>> Developers modifying `java.security` in the future could introduce 
>> properties using a different (documented) separator. We also prefer the 
>> space separator for `include` directives, as shown in the [Security 
>> Properties File 
>> guide](https://docs.oracle.com/en/java/javase/26/security/security-properties-file.html#GUID-FF09EB34-CD27-4D1B-B55B-A4A4E6A0F039)
>>  examples. The same applies to downstream projects modifying `java.security` 
>> before building (not a problem for Red Hat, but perhaps for other vendors).
>
> Properties.load/save is read/write and doesn't support round trip. The 
> semantics here is a transformation that retains the comments and whitespace, 
> allowing inline replace of values. It would requite significant refactor to 
> provide this via a jdk.internal package exported to jdk.jlink. Not opposed to 
> this but I think first explore if there a generally useful API for Properties 
> to expose to allow programs do these types of transformations.
> 
> As regards the processProperties loop then I think it will need to handle the 
> 3 possible separators.

My intention was to focus on supporting the syntax that is currently used in 
the JDK `java.security` file. Alan correctly notes that the semantics of this 
use case is quite different than what the current `Properties` API supports, so 
it is not as simple as just reusing and making the 
`java.util.Properties::load0` method more accessible. I'm not inclined to 
significantly increase the scope of this enhancement.

So I think it makes sense to add support for additional separators and document 
any other differences. If any of these become critical parsing issues later, we 
can always consider adding support for them at that time.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/31884#discussion_r3806019913

Reply via email to