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
