RFR: 8246129: ZIP entries created for DOS epoch include local timezone metadata

[Claes Redestad]

Hi,

On 2020-06-09 17:51, Martin Buchholz wrote:
> not really a review, but cautions:
> 
> I was afraid that might be hard to fix because DOSTIME_BEFORE_1980 was
> used as a special value indicating that the real time was < 1980 and
> stored in an extra field.  But I guess we now don't have any code that
> makes assumptions based only on DOSTIME_BEFORE_1980?

AFAICT there is no use of this (package-)private constant outside of the
logic that use it to avoid adding extra time information in the normal
case. I added it as an optimization for the purpose of avoiding to
implicitly add the extra time field data, in response to a change that
would have done so without any recourse.

While I missed the corner case of 1980-01-01 00:00:00 ("Who would ever
write entries timestamped as exactly 1980-01-01 00:00:00 in 2015!?"),
without this optimization the workarounds in the linked issue would
probably not have worked, either.

> 
> Keep in mind that dostime is local date time, and there's always risk
> in the zip code of confusing time-zoned date time with un-zoned.

Yes, I think the added test covers this: stray outside of 1980-01-01
00:00:00 in the _local_ time and you get extended time fields. Going
outside the DOS epoch slightly changes semantics since you'll now
implicitly get unix epoch relative timestamps (which adds overhead).

One could also opt to get this behavior explicitly by using
setLastModifiedTime(..), which gives you a bit higher precision and
less weirdness.

/Claes

> 
> On Tue, Jun 9, 2020 at 8:24 AM Claes Redestad <claes.redestad at oracle.com> wrote:
>>
>> Hi,
>>
>> this patch addresses a corner case where extra time stamp information
>> is added when using start of the DOS time epoch (1980-01-01 00:00:00)
>> as a dummy timestamp for entries
>>
>> Bug:    https://bugs.openjdk.java.net/browse/JDK-8246129
>> Webrev: http://cr.openjdk.java.net/~redestad/8246129/open.00/
>>
>> Testing: tier1-2
>>
>> Thanks!
>>
>> /Claes