RFR JDK-8186464: ZipFile cannot read some InfoZip ZIP64 zip files
Martin Buchholz
martinrb at google.com
Wed Sep 6 00:51:24 UTC 2017
Thanks for doing this.
I'm also hoping to contribute some testing effort, but time ... time ...
This Looks Good To Me, but as always I have comments.
---
I wouldn't create an echo process. Just create a zip subprocess and write
the bytes from java to its stdin.
181 ProcessBuilder echo = new ProcessBuilder("echo", "hello");
---
You should check whether there's a usable zip program, e.g. I once wrote
code like this:
if (! new File("/usr/bin/perl").canExecute()
182 ProcessBuilder zip = new ProcessBuilder("zip",
path.toString().toString(), "-");
oh wait, now I see
196 } catch (IOException x) { 197 // ignore,
probably from process.start() for "Cannot run program"
well ... I would still add some checks for /usr/bin/zip and elide the catch.
---
"""Always specify the charset""" (here "ASCII" is fine).
163 if (!"hello".equals(new
String(zf.getInputStream(new ZipEntry("hello")) 164
.readAllBytes())))
---
Can we say something like
// We must always check for a zip64 EOCD record; it is always permitted to
be present
1125 // try if we have a zip64 end;
---
Maybe
// end64 candidate found
1139 // end64 found,
---
It's 2017. Time to just make centot a long?
1152 end.centot = (int)centot64; // assume
total < 2g
---
Here's a typo to fix:
entires
On Fri, Sep 1, 2017 at 4:17 PM, Xueming Shen <xueming.shen at oracle.com>
wrote:
> On 8/22/17, 5:54 PM, Martin Buchholz wrote:
>
>
>
> On Tue, Aug 22, 2017 at 3:27 PM, Xueming Shen <xueming.shen at oracle.com>
> wrote:
>
>>
>> How about to add an option to our zipfs to force the ZIP64 end record when
>> enabled. Harmless if not specified.
>>
>
> I agree that adding more options for testability is good. Since our users
> are likely to need such things as well, I'd also like to see such features
> become public, analogous to zip's -fz- flag.
>
> OTOH, interoperability testing is very valuable, so despite the troubles
> involved writing tests that involve both jdk and zipinfo code, I think we
> should do it.
>
>
> Martin,
>
> I have added a test case that tests "echo hello | zip infozip.zip -" via
> pb.startPipeline()
> in ReadZip.java, which should add some coverage for the "interoperability
> testing".
>
> Would you please help take a look?
>
> http://cr.openjdk.java.net/~sherman/8186464/webrev/
>
> Thanks,
> Sherman
>
More information about the core-libs-dev
mailing list