RFR: 8231640: (prop) Canonical property storage [v10]

Jaikiran Pai jai.forums2013 at gmail.com
Sun Sep 12 14:29:38 UTC 2021


Hello Stuart,

On 10/09/21 11:12 pm, Stuart Marks wrote:
>
> src/java.base/share/classes/java/util/Properties.java line 832:
>
>> 830:      * {@code #} character, the current date and time (formatted using the
>> 831:      * {@code EEE MMM dd HH:mm:ss zzz yyyy} {@link DateTimeFormatter date format}),
>> 832:      * and a line separator as generated by the {@code Writer}.
> Since this behavior is affected by a system property, and that property name is in the standard `java.*` namespace, that should be documented here. In addition, `Writer` has no notion of a line separator; it just comes from `System.lineSeparator`. I'd suggest something like this, replacing the paragraph starting with "Next":
>
>
> If the {@systemProperty java.util.Properties.storeDate} is set and is non-blank (as determined by
> {@link String#isBlank String.isBlank}, a comment line is written as follows. First, a {@code #} character is written, followed by the contents of the property, followed by a line separator.
>
> If the system property is not set or is blank, a comment line is written as follows. First, a {@code #} character is written, followed by the current date and time formatted as if by {@link DateTimeFormatter} with the format
> {@code "EEE MMM dd HH:mm:ss zzz yyyy"}, followed by a line separator.

Done. I've updated the PR to use this text in the javadoc.

> This was discussed elsewhere, but after writing that, I'm now thinking that we **should** honor the property even if it is blank. It would be useful to disable the timestamp simply by setting the property to the empty string; this will simply result in an empty comment line. This would simplify the code (and the spec) by removing conditions related to String::isBlank.

OK. I don't see any obvious issues with interpreting 
empty/whitespace-only value for the system property to translate to an 
empty comment line. To be clear, an empty comment line that gets written 
out in such cases is *always* going to be the "#" character followed by 
a line separator right? Irrespective of what or how many whitespace 
characters are present in the property value? Or do you want those 
characters to be carried over into that comment line that gets written 
out? The reason I ask this is because I think we should always write 
just the "#" followed by the line separator character in such cases, 
which effectively means we will still need the String::isBlank check 
which would then look something like:

if (propVal.isBlank()) {
   bw.write("#");
   bw.newLine();
}
> Side question: do we want to do any escaping or vetting of the property value?

One of the reasons why we didn't want to use the date in epoch seconds 
or a formatted date string was to avoid the complexities that come with 
managing that property value. So a free-form property value seemed more 
appropriate and I think a free-form value still seems appropriate, but I 
think we should keep any vetting to minimal. More details below.

> If for example it contains embedded newlines, this could result in a malformed property file. Or, if carefully constructed, it could contain additional property values. I think this is an unintended consequence of changing the property value from a numeric time value to a free-form string. We may want to reconsider this.

The specification on the load(Reader reader) method of the 
java.util.Properties class has this to say about comment lines (and 
lines in general).

(snipped relevant content):

    There are two kinds of line, <i>natural lines</i> and <i>logical
    lines</i>.
    A natural line is defined as a line of characters that is terminated
    either
    by a set of line terminator characters ({@code \n} or {@code \r} or
    {@code \r\n}) or by the end of the stream. A natural line may be
    either a blank line, a comment line, or hold all or some of a
    key-element pair. A logical
    line holds all the data of a key-element pair, which may be spread
    out across several adjacent natural lines by escaping
    the line terminator sequence with a backslash character
    {@code \}.  Note that a comment line cannot be extended
    in this manner; every natural line that is a comment must have
    its own comment indicator, as described below.


With that knowledge about comment lines, I think what we should do is 
disallow system property values that contain any form of line terminator 
characters (\n or \r or \r\n). If the system property value is _not_ 
blank (as specified by String::isBlank) and contains any of these line 
terminator characters, we should simply ignore it and write out the 
current date as the date comment. That, IMO, should prevent any of these 
sneaky/rogue value that can end up either creating additional property 
key/values to be written out or causing any malformed properties file. 
Plus, that would help us keep the vetting to minimal without involving 
too much complexity.

> src/java.base/share/classes/java/util/Properties.java line 855:
>
>> 853:      * the value of this system property represents a formatted
>> 854:      * date time value that can be parsed back into a {@link Date} using an appropriate
>> 855:      * {@link DateTimeFormatter}
> With the property behavior added to normative text above, I don't think we need this note anymore. We might want to leave something here about the convention of putting a timestamp here, and an implicit(?) requirement that it be formatted properly.

The newly updated PR has updated this @implNote to remove some of the 
previous text and yet retain some hints on what would be an "ideal" 
value for the system property value. But I think we should perhaps just 
get rid of this @implNote since we are now proposing to allow empty 
comment lines and free form text and this no longer is a "let's use this 
system property to allow users to specify a fixed date" enhancement.

-Jaikiran



More information about the core-libs-dev mailing list