RFR: 8231640: (prop) Canonical property storage

[Alan Bateman]

On 04/09/2021 16:50, Jaikiran Pai wrote:
> The commit in this PR implements the proposal for enhancement that was discussed in the core-libs-dev mailing list recently[1], for https://bugs.openjdk.java.net/browse/JDK-8231640
>
> At a high level - the `store()` APIs in `Properties` have been modified to now look for the `SOURCE_DATE_EPOCH` environment variable[2]. If that env variable is set, then instead of writing out the current date time as a date comment, the `store()` APIs instead will use the value set for this env variable to parse it to a `Date` and write out the string form of such a date. The implementation here uses the `d MMM yyyy HH:mm:ss 'GMT'` date format and `Locale.ROOT` to format and write out such a date. This should provide reproducibility whenever the `SOURCE_DATE_EPOCH` is set. Furthermore, intentionally, no changes in the date format of the "current date" have been done.
>
> These  modified `store()` APIs work in the presence of the `SecurityManager` too. The caller is expected to have a read permission on the `SOURCE_DATE_EPOCH` environment variable. If the caller doesn't have that permission, then the implementation of these `store()` APIs will write out the "current date" and will ignore any value that has been set for the `SOURCE_DATE_EPOCH` env variable. This should allow for backward compatibility of existing applications, where, when they run under a `SecurityManager` and perhaps with an existing restrictive policy file, the presence of `SOURCE_DATE_EPOCH` shouldn't impact their calls to the `store()` APIs.
>
> The modified `store()` APIs will also ignore any value for `SOURCE_DATE_EPOCH` that  cannot be parsed to an `long` value. In such cases, the `store()` APIs will write out the "current date" and ignore the value set for this environment variable. No exceptions will be thrown for such invalid values. This is an additional backward compatibility precaution to prevent any rogue value for `SOURCE_DATE_EPOCH` from breaking applications.
In the discussion on this option then I think we were talking about 
changing the specification of the store methods to allow for an 
implementation specific means to override the date and put the 
discussion on SOURCE_DATE_EPOCH in an @implNote.

The SM case probably needs discussion as I was expecting to see 
something like this:

PrivilegedAction<String> pa = () -> System.getenv("SOURCE_DATE_EPOCH");
String value = AccessController.doPrivileged(pa);

as a caller of the store method (in a library for example) is unlikely 
to have this permission. I assume your concern is that untrusted code 
would have a way to read this specific env variable without a permission 
(by way of creating and storing an empty Properties)?  In this case I'm 
mostly wondering if it's really worth having a behavior difference in 
this mode.

>
> An additional change in the implementation of these `store()` APIs and unrelated to the date comment, is that these APIs will now write out the property keys in a deterministic order. The keys will be written out in the natural ordering as specified by `java.lang.String#compareTo()` API.
>
The store methods are already specified to throw CCE if the properties 
has non-String keys so this should be okay.

-Alan