[10] RFR: 8184940: JDK 9 rejects zip files where the modified day or month is 0

classic Classic list List threaded Threaded
9 messages Options
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

[10] RFR: 8184940: JDK 9 rejects zip files where the modified day or month is 0

Liam Miller-Cushon
Hello,

This change fixes a regression in JDK 9 by allowing the java.util and zipfs
zip implementations to read archives where the modified day or month is 0.

bug: https://bugs.openjdk.java.net/browse/JDK-8184940
webrev: http://cr.openjdk.java.net/~cushon/8184940/webrev.00/

Thanks,
Liam
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [10] RFR: 8184940: JDK 9 rejects zip files where the modified day or month is 0

Martin Buchholz-3
Looks good.  This should go into jdk10 and later into a jdk9 update.  (Will
an Oracle engineer volunteer?)

I wonder where the "8" in the "Friday the 30th" date comes from.  Could
your California time zone be leaking into the test?   The DOS timestamp is
interpreted as local time while Instant.parse uses UTC?  Will the test
still pass if you run jtreg under TZ=EST?

On Wed, Jul 26, 2017 at 3:32 PM, Liam Miller-Cushon <[hidden email]>
wrote:

> Hello,
>
> This change fixes a regression in JDK 9 by allowing the java.util and
> zipfs zip implementations to read archives where the modified day or month
> is 0.
>
> bug: https://bugs.openjdk.java.net/browse/JDK-8184940
> webrev: http://cr.openjdk.java.net/~cushon/8184940/webrev.00/
>
> Thanks,
> Liam
>
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [10] RFR: 8184940: JDK 9 rejects zip files where the modified day or month is 0

Liam Miller-Cushon
On Wed, Jul 26, 2017 at 4:12 PM, Martin Buchholz <[hidden email]>
wrote:

> I wonder where the "8" in the "Friday the 30th" date comes from.  Could
> your California time zone be leaking into the test?   The DOS timestamp is
> interpreted as local time while Instant.parse uses UTC?  Will the test
> still pass if you run jtreg under TZ=EST?
>

Thanks, fixed:
http://cr.openjdk.java.net/~cushon/8184940/webrev.01/
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [10] RFR: 8184940: JDK 9 rejects zip files where the modified day or month is 0

Martin Buchholz-3
I took another look.

  90         if ((dtime >> 25) == 0) {

It looks like this will test only the year, not all the date fields.
Shouldn't that be s/25/16/ ?
Does this code handle the "true" epoch of 1980-01-01 ?


On Thu, Jul 27, 2017 at 11:54 AM, Liam Miller-Cushon <[hidden email]>
wrote:

> On Wed, Jul 26, 2017 at 4:12 PM, Martin Buchholz <[hidden email]>
> wrote:
>
>> I wonder where the "8" in the "Friday the 30th" date comes from.  Could
>> your California time zone be leaking into the test?   The DOS timestamp is
>> interpreted as local time while Instant.parse uses UTC?  Will the test
>> still pass if you run jtreg under TZ=EST?
>>
>
> Thanks, fixed:
> http://cr.openjdk.java.net/~cushon/8184940/webrev.01/
>
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [10] RFR: 8184940: JDK 9 rejects zip files where the modified day or month is 0

Liam Miller-Cushon
On Fri, Jul 28, 2017 at 3:30 PM, Martin Buchholz <[hidden email]>
wrote:

> It looks like this will test only the year, not all the date fields.
> Shouldn't that be s/25/16/ ?
> Does this code handle the "true" epoch of 1980-01-01 ?
>

It does now, thanks:
http://cr.openjdk.java.net/~cushon/8184940/webrev.02/

This raises the question of what it should do if a day or month is zero but
year/month/day are not all zero. Is 1980-0-0 special, or should 1980-0-1
and 1980-1-0 also be tolerated for compatibility with JDK 8?
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [10] RFR: 8184940: JDK 9 rejects zip files where the modified day or month is 0

Liam Miller-Cushon
On Mon, Jul 31, 2017 at 11:41 AM, Liam Miller-Cushon <[hidden email]>
wrote:

> On Fri, Jul 28, 2017 at 3:30 PM, Martin Buchholz <[hidden email]>
> wrote:
>
>> It looks like this will test only the year, not all the date fields.
>> Shouldn't that be s/25/16/ ?
>> Does this code handle the "true" epoch of 1980-01-01 ?
>>
>
> It does now, thanks:
> http://cr.openjdk.java.net/~cushon/8184940/webrev.02/
>
> This raises the question of what it should do if a day or month is zero
> but year/month/day are not all zero. Is 1980-0-0 special, or should
> 1980-0-1 and 1980-1-0 also be tolerated for compatibility with JDK 8?
>

As discussed offline, there are no known instances of tools deliberately
writing zip archives where a day or month is '0' other than 1980-0-0. So
the current approach seems better than continuing to interpret e.g.
1980-1-0 as 1979-12-31 or 1980-0-1 as 1979-12-01.
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [10] RFR: 8184940: JDK 9 rejects zip files where the modified day or month is 0

Martin Buchholz-3
Reviewed!

Please push to jdk10/jdk10/jdk.

On Mon, Aug 14, 2017 at 1:07 PM, Liam Miller-Cushon <[hidden email]>
wrote:

> On Mon, Jul 31, 2017 at 11:41 AM, Liam Miller-Cushon <[hidden email]>
> wrote:
>
>> On Fri, Jul 28, 2017 at 3:30 PM, Martin Buchholz <[hidden email]>
>> wrote:
>>
>>> It looks like this will test only the year, not all the date fields.
>>> Shouldn't that be s/25/16/ ?
>>> Does this code handle the "true" epoch of 1980-01-01 ?
>>>
>>
>> It does now, thanks:
>> http://cr.openjdk.java.net/~cushon/8184940/webrev.02/
>>
>> This raises the question of what it should do if a day or month is zero
>> but year/month/day are not all zero. Is 1980-0-0 special, or should
>> 1980-0-1 and 1980-1-0 also be tolerated for compatibility with JDK 8?
>>
>
> As discussed offline, there are no known instances of tools deliberately
> writing zip archives where a day or month is '0' other than 1980-0-0. So
> the current approach seems better than continuing to interpret e.g.
> 1980-1-0 as 1979-12-31 or 1980-0-1 as 1979-12-01.
>
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [10] RFR: 8184940: JDK 9 rejects zip files where the modified day or month is 0

Liam Miller-Cushon
The changeset is attached. (I don't have commit access.)

On Mon, Aug 14, 2017 at 3:57 PM, Martin Buchholz <[hidden email]>
wrote:

> Reviewed!
>
> Please push to jdk10/jdk10/jdk.
>
> On Mon, Aug 14, 2017 at 1:07 PM, Liam Miller-Cushon <[hidden email]>
> wrote:
>
>> On Mon, Jul 31, 2017 at 11:41 AM, Liam Miller-Cushon <[hidden email]>
>> wrote:
>>
>>> On Fri, Jul 28, 2017 at 3:30 PM, Martin Buchholz <[hidden email]>
>>> wrote:
>>>
>>>> It looks like this will test only the year, not all the date fields.
>>>> Shouldn't that be s/25/16/ ?
>>>> Does this code handle the "true" epoch of 1980-01-01 ?
>>>>
>>>
>>> It does now, thanks:
>>> http://cr.openjdk.java.net/~cushon/8184940/webrev.02/
>>>
>>> This raises the question of what it should do if a day or month is zero
>>> but year/month/day are not all zero. Is 1980-0-0 special, or should
>>> 1980-0-1 and 1980-1-0 also be tolerated for compatibility with JDK 8?
>>>
>>
>> As discussed offline, there are no known instances of tools deliberately
>> writing zip archives where a day or month is '0' other than 1980-0-0. So
>> the current approach seems better than continuing to interpret e.g.
>> 1980-1-0 as 1979-12-31 or 1980-0-1 as 1979-12-01.
>>
>
>
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: [10] RFR: 8184940: JDK 9 rejects zip files where the modified day or month is 0

Martin Buchholz-3
OK, I pushed.
Loading...