archive/zip: OpenReader(zipfile) simply says "zip: not a valid zip file" for jar files with extra bytes, which Java and unzip handle fine #51337
Comments
|
Note that you may have trouble repeating this if you just use the opentelemetry 1.10.0 jar file. I just tried downloading that jar file directly and running this test, and it doesn't have that problem. Somehow the jar file that ended up getting into my build process is slightly altered. We can deal with that, but I still think it's worth making archive/zip a little more robust, to let it deal with this somewhat nonstandard zip file. |
|
I'm attaching the jar file (renamed to zip so I can upload it). Again, I don't know how it got into this state, but except for archive/zip OpenReader failing on it, Java is perfectly fine with it, and unzip processes it with a warning. |
|
Given that the extra garbage at the beginning are HTTP headers I suspect you downloaded the file with something like |
|
I don't think anything should be done here, as noted above, the file has extra data (http headers) which isn't present in upstream source (https://github.com/open-telemetry/opentelemetry-java-instrumentation/releases/tag/v1.10.0), pointing to an error in your retrieval of the artifact. |
thanm
added
the
NeedsDecision
|
@dsnet per owners. As others have pointed out, the file in question has extra junk tacked onto the beginning, so it's not at all clear whether we would want to support this behavior. If we do allow extra junk, what are the rules? Is it ok to have extra junk just at the beginning, or can we have extra junk elsewhere as well? How many bytes of extra junk is OK and how much is too much? In your case you have about 3.6kbytes of extra junk-- what would happen if there were 5 MB of extra junk? If we allow extra junk in archive/zip files, what about ELF files or Go object files, should that be supported too? I'm going to close out this issue; feel free to reopen if you disagree. Thanks. |
|
Just so it's clear, both Java and Python can open and process this file without complaint. |
|
I believe this is a dup of #10464 |
|
@ericchiang Thanks, I agree. With https://go.dev/cl/387976 the prefixed zip file can be read. I verified that the Linux unzip program can read it, and the patch that I wrote should act the same way. |
|
Change https://go.dev/cl/387976 mentions this issue: |
A Java jar file is a zip file, but it can have a prefix that is a bash script that unpacks the zip file. Most zip programs ignore such prefixes. This CL changes the archive/zip package to do the same. Fixes #10464 Fixes #51337 Change-Id: I976e9c64684644317bd21077bc5b4a2baf626ee6 Reviewed-on: https://go-review.googlesource.com/c/go/+/387976 Run-TryBot: Ian Lance Taylor <iant@google.com> Reviewed-by: Ian Lance Taylor <iant@google.com> TryBot-Result: Gopher Robot <gobot@golang.org> Run-TryBot: Ian Lance Taylor <iant@golang.org> Reviewed-by: David Chase <drchase@google.com> Reviewed-by: Joseph Tsai <joetsai@digital-static.net> Auto-Submit: Ian Lance Taylor <iant@google.com>
I wrote a small tool that uses "archive/zip" to look at some aspects of zip files. I've tested it with many zip and Java jar files, and it appears to work perfectly fine. I built it with Go v1.17 (not sure what patch version).
today I started working with a jar file from the "opentelemetry java" project, version 1.10.0. The jar file was provided to me by a member of a team in my organization. I don't know whether they got this directly from the opentelemetry Java github site or whether they assembled it themselves somehow. The opentelemetry Java site has that release (https://github.com/open-telemetry/opentelemetry-java/releases/tag/v1.10.0 ), but I don't see any binary artifacts.
When I run my tool against this jar file, it simply says
with no other information. I stepped into the code, and that is the error returned from zip.OpenReader(zipfile).
I tried opening this jar file with "unzip", and it says the following:
Except for this warning, it processes it perfectly fine. Java and the "jar" tool in the Java distribution don't even present a warning. It works fine in Java.
The text was updated successfully, but these errors were encountered: