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

Roger Riggs roger.riggs at oracle.com
Mon Sep 13 13:43:24 UTC 2021

Hi Jaikiran,

"Editing" the value of the comment property, to remove or ignore blanks 
for other special
characters makes the code more complex and adds a bunch of conditions. 
the comment if it doesn't have the allowed format is hard to track down, 
because there's
no way to report it was dropped or why.

I would write the value of the comment property using the writeComments 
so it is encoded the same as the other comment passed as an store argument.
That will handle all special characters and multi-line comments.  It is 
simpler to specify
and has the same predictable output as other comments.

if the property is non-empty

On 9/12/21 10:29 AM, Jaikiran Pai wrote:
> ...
>> 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