Re: RFR: 8150469: unpack200 fails to compare crc correctly.

2016-04-06 Thread John Rose
Looks good, a clean fix.  The negative test should be called "testBadChecksum" 
or "testBrokenTrailer".

— John

On Feb 29, 2016, at 8:06 AM, Kumar Srinivasan  
wrote:
> 
> Hello John, Alex,
> 
> Please review fix for https://bugs.openjdk.java.net/browse/JDK-8150469
> 
> The previous fix keeps the storage in the unpacker object which is
> reset, for a multi-segmented pack archive, thus the computed crc32 is lost.
> 
> Now the storage is moved into the gzip container, which persists, across
> multiple segments, I  also added the length check as specified by RFC 1952,
> as  implemented by JDK.
> 
> http://hg.openjdk.java.net/jdk9/dev/jdk/file/e4af8119eba4/src/java.base/share/classes/java/util/zip/GZIPInputStream.java#l222
> 
> The review is at:
> http://cr.openjdk.java.net/~ksrini/8150469/webrev.00/
> 
> 
> Thanks
> Kumar
> 
> 



Re: RFR: 8150469: unpack200 fails to compare crc correctly.

2016-02-29 Thread Omair Majid
Hi,

* Kumar Srinivasan  [2016-02-29 11:07]:
> Please review fix for https://bugs.openjdk.java.net/browse/JDK-8150469
> 
> The review is at:
> http://cr.openjdk.java.net/~ksrini/8150469/webrev.00/

I can confirm this patch fixes the problem with a particular pack.gz
that I was observing.

(This isn't a review of the patch.)

Thanks for the quick fix!

Regards,
Omair

-- 
PGP Key: 66484681 (http://pgp.mit.edu/)
Fingerprint = F072 555B 0A17 3957 4E95  0056 F286 F14F 6648 4681


RFR: 8150469: unpack200 fails to compare crc correctly.

2016-02-29 Thread Kumar Srinivasan

Hello John, Alex,

Please review fix for https://bugs.openjdk.java.net/browse/JDK-8150469

The previous fix keeps the storage in the unpacker object which is
reset, for a multi-segmented pack archive, thus the computed crc32 is lost.

Now the storage is moved into the gzip container, which persists, across
multiple segments, I  also added the length check as specified by RFC 1952,
as  implemented by JDK.

http://hg.openjdk.java.net/jdk9/dev/jdk/file/e4af8119eba4/src/java.base/share/classes/java/util/zip/GZIPInputStream.java#l222

The review is at:
http://cr.openjdk.java.net/~ksrini/8150469/webrev.00/


Thanks
Kumar