On Thu, 2 Dec 2021 00:09:15 GMT, John Neffenger <jgn...@openjdk.org> wrote:
>> Andrew Leonard has updated the pull request incrementally with one
>> additional commit since the last revision:
>>
>> Update src/jdk.jlink/share/classes/jdk/tools/jmod/JmodOutputStream.java
>>
>> Co-authored-by: Magnus Ihse Bursie <m...@icus.se>
>
> src/jdk.jartool/share/classes/sun/tools/jar/GNUStyleOptions.java line 199:
>
>> 197: void process(Main jartool, String opt, String arg)
>> throws BadArgs {
>> 198: try {
>> 199: jartool.date = ZonedDateTime.parse(arg,
>> DateTimeFormatter.ISO_DATE_TIME)
>
> The `ISO_ZONED_DATE_TIME` parser works better here.
done
> src/jdk.jartool/share/classes/sun/tools/jar/GNUStyleOptions.java line 201:
>
>> 199: jartool.date = ZonedDateTime.parse(arg,
>> DateTimeFormatter.ISO_DATE_TIME)
>> 200:
>> .withZoneSameInstant(ZoneOffset.UTC).toLocalDateTime();
>> 201: if (jartool.date.getYear() < 1980 ||
>> jartool.date.getYear() > 2099) {
>
> This `if` test can end up with consequences down to the millisecond of the
> instant provided, if present. (Try `1980-01-01T00:00:00.001Z` vs.
> `1980-01-01T00:00:00Z` and check for extended headers.) How about something
> like the following?
>
>
> ... ZonedDateTime TIME_MIN = ZonedDateTime.parse("1980-01-01T00:00:02Z");
> ... ZonedDateTime TIME_MAX = ZonedDateTime.parse("2099-12-31T23:59:59Z");
> ...
> var zoned = ZonedDateTime.parse(date,
> DateTimeFormatter.ISO_ZONED_DATE_TIME);
> if (zoned.isBefore(TIME_MIN) || zoned.isAfter(TIME_MAX)) {
> ....
done
> src/jdk.jartool/share/classes/sun/tools/jar/resources/jar.properties line 88:
>
>> 86: date {0} is not a valid ISO 8601 date and time
>> 87: error.date.out.of.range=\
>> 88: date {0} is not within the valid year range 1980->2099
>
> Maybe just define `date {0} is not within the valid range` and put the exact
> interval in the *man* pages and documentation?
done
> src/jdk.jlink/share/classes/jdk/tools/jmod/JmodTask.java line 1164:
>
>> 1162: public LocalDateTime convert(String value) {
>> 1163: try {
>> 1164: LocalDateTime date = ZonedDateTime.parse(value,
>> DateTimeFormatter.ISO_DATE_TIME)
>
> The `ISO_ZONED_DATE_TIME` parser works better here.
done
> src/jdk.jlink/share/classes/jdk/tools/jmod/JmodTask.java line 1166:
>
>> 1164: LocalDateTime date = ZonedDateTime.parse(value,
>> DateTimeFormatter.ISO_DATE_TIME)
>> 1165:
>> .withZoneSameInstant(ZoneOffset.UTC).toLocalDateTime();
>> 1166: if (date.getYear() < 1980 || date.getYear() > 2099) {
>
> See previous comment for this line in the `jar` tool.
done
> src/jdk.jlink/share/classes/jdk/tools/jmod/resources/jmod.properties line 111:
>
>> 109: err.no.moduleToHash=No hashes recorded: no module matching {0} found to
>> record hashes
>> 110: err.invalid.date=--date {0} is not a valid ISO 8601 date and time: {1}
>> 111: err.date.out.of.range=--date {0} is out of the valid year range
>> 1980->2099
>
> As before, perhaps refer to the exact range in the documentation? It's really
> an instant on the time line that is being compared and not the actual year
> digits that were provided in the command-line option.
done
> test/jdk/tools/jar/JarEntryTime.java line 180:
>
>> 178: "2022-03-15T00:00:00+06:00",
>> 179: "2038-11-26T06:06:06+00:00",
>> 180: "2098-02-18T00:00:00-08:00"};
>
> It would be great to test just inside the exact boundaries of the permitted
> interval here. That is, test `1980-01-01T00:00:02+00:00` and
> `2099-12-31T23:59:59+00:00`.
done
> test/jdk/tools/jar/JarEntryTime.java line 251:
>
>> 249: // Negative Tests --date out of range source date
>> 250: String[] badSourceDates = {"1976-06-24T01:02:03+00:00",
>> 251: "2100-02-18T00:00:00-11:00"};
>
> It would be great to test just outside the exact boundaries of the permitted
> interval here. That is, test `1980-01-01T00:00:01+00:00` and
> `2100-01-01T00:00:00+00:00`.
done
-------------
PR: https://git.openjdk.java.net/jdk/pull/6481
