RFR(s): 8240235: jdk.test.lib.util.JarUtils updates jar files incorrectly
Hi, can I please have a review for this small test fix: http://cr.openjdk.java.net/~simonis/webrevs/2020/8240235/ https://bugs.openjdk.java.net/browse/JDK-8240235 JarUtils.updateJar() and JarUtils.updateJarFile() update jar files by reading JarEntry-s from a source jar file and writing these exact JarEntry-s to the destination jar file followed be the inflated and re-deflated data belonging to this entry. This approach is not correct, because JarEntry contains both, the compressed and uncompressed size of the corresponding entry. But the original Defalter which initially compressed that entry can be either different from the current one or it might have used a different compression level compared to the current Deflater which re-deflates the entry data. When the newly created entry is closed, the new compressed size will be checked against the original compressed size if that was recorded in the JarEntry and an exception will be thrown, when they differ: java.util.zip.ZipException: invalid entry compressed size (expected 237 but got 238 bytes) at java.base/java.util.zip.ZipOutputStream.closeEntry(ZipOutputStream.java:267) at java.base/java.util.zip.ZipOutputStream.putNextEntry(ZipOutputStream.java:192) at java.base/java.util.jar.JarOutputStream.putNextEntry(JarOutputStream.java:108) at jdk.test.lib.util.JarUtils.updateJar(JarUtils.java:294) at jdk.test.lib.util.JarUtils.updateJar(JarUtils.java:252) at HasUnsignedEntryTest.start(HasUnsignedEntryTest.java:77) at HasUnsignedEntryTest.main(HasUnsignedEntryTest.java:44) at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method) at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62) at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43) at java.base/java.lang.reflect.Method.invoke(Method.java:564) at com.sun.javatest.regtest.agent.MainWrapper$MainThread.run(MainWrapper.java:127) at java.base/java.lang.Thread.run(Thread.java:832) Suppressed: java.util.zip.ZipException: invalid entry compressed size (expected 237 but got 238 bytes) at java.base/java.util.zip.ZipOutputStream.closeEntry(ZipOutputStream.java:267) at java.base/java.util.zip.ZipOutputStream.finish(ZipOutputStream.java:360) at java.base/java.util.zip.DeflaterOutputStream.close(DeflaterOutputStream.java:237) at java.base/java.util.zip.ZipOutputStream.close(ZipOutputStream.java:377) at jdk.test.lib.util.JarUtils.updateJar(JarUtils.java:280) The fix is trivial. Instead of copying the JarEntry-s verbatim to the output stream, simply write newly created JarEntry-s to the output stream which only contain the entry name. This is also the way how this is handled by the jar tool, when it updates jar files. Thank you and best regards, Volker
Welcome to the troublesome world of zip implementations! It looks like by creating a new JarEntry you are discarding possible information from the old one, e.g. you might be discarding whether the input entry was compressed. If "jar u" does that, I would consider it a bug! Arguably there is a missing API that would allow you to copy zip entries from an input to an output completely intact, without having to decompress and recompress. (might be hard to design!) Probably there should be no such utility method as updateJarFile at all. Perhaps instead the jar/zip code could be refactored so that tests can reuse the same code used by "jar u". On Fri, Feb 28, 2020 at 10:50 AM Volker Simonis <volker.simonis@gmail.com> wrote:
Hi,
can I please have a review for this small test fix:
http://cr.openjdk.java.net/~simonis/webrevs/2020/8240235/ https://bugs.openjdk.java.net/browse/JDK-8240235
JarUtils.updateJar() and JarUtils.updateJarFile() update jar files by reading JarEntry-s from a source jar file and writing these exact JarEntry-s to the destination jar file followed be the inflated and re-deflated data belonging to this entry.
This approach is not correct, because JarEntry contains both, the compressed and uncompressed size of the corresponding entry. But the original Defalter which initially compressed that entry can be either different from the current one or it might have used a different compression level compared to the current Deflater which re-deflates the entry data.
When the newly created entry is closed, the new compressed size will be checked against the original compressed size if that was recorded in the JarEntry and an exception will be thrown, when they differ:
java.util.zip.ZipException: invalid entry compressed size (expected 237 but got 238 bytes) at java.base/java.util.zip.ZipOutputStream.closeEntry(ZipOutputStream.java:267) at java.base/java.util.zip.ZipOutputStream.putNextEntry(ZipOutputStream.java:192) at java.base/java.util.jar.JarOutputStream.putNextEntry(JarOutputStream.java:108) at jdk.test.lib.util.JarUtils.updateJar(JarUtils.java:294) at jdk.test.lib.util.JarUtils.updateJar(JarUtils.java:252) at HasUnsignedEntryTest.start(HasUnsignedEntryTest.java:77) at HasUnsignedEntryTest.main(HasUnsignedEntryTest.java:44) at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method) at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62) at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43) at java.base/java.lang.reflect.Method.invoke(Method.java:564) at com.sun.javatest.regtest.agent.MainWrapper$MainThread.run(MainWrapper.java:127) at java.base/java.lang.Thread.run(Thread.java:832) Suppressed: java.util.zip.ZipException: invalid entry compressed size (expected 237 but got 238 bytes) at
java.base/java.util.zip.ZipOutputStream.closeEntry(ZipOutputStream.java:267) at java.base/java.util.zip.ZipOutputStream.finish(ZipOutputStream.java:360) at
java.base/java.util.zip.DeflaterOutputStream.close(DeflaterOutputStream.java:237) at java.base/java.util.zip.ZipOutputStream.close(ZipOutputStream.java:377) at jdk.test.lib.util.JarUtils.updateJar(JarUtils.java:280)
The fix is trivial. Instead of copying the JarEntry-s verbatim to the output stream, simply write newly created JarEntry-s to the output stream which only contain the entry name. This is also the way how this is handled by the jar tool, when it updates jar files.
Thank you and best regards, Volker
On Fri, Feb 28, 2020 at 8:02 PM Martin Buchholz <martinrb@google.com> wrote:
Welcome to the troublesome world of zip implementations!
Well, not many remember that the zip format was designed to work efficiently with floppy discs :)
It looks like by creating a new JarEntry you are discarding possible information from the old one, e.g. you might be discarding whether the input entry was compressed. If "jar u" does that, I would consider it a bug!
You're right. My fix was a little to simple, and can be improved: http://cr.openjdk.java.net/~simonis/webrevs/2020/8240235.01 Preserving the time, comment, extra information, compression method and size/CRC if the compression method was "stored" is the best we can do. And this is actually exactly what "jar -" does (see [1]). [1] http://hg.openjdk.java.net/jdk/jdk/file/e04746cec89c/src/jdk.jartool/share/c...
Arguably there is a missing API that would allow you to copy zip entries from an input to an output completely intact, without having to decompress and recompress. (might be hard to design!)
Yes, I did think about this as well. It might not be that hard at all. We could simply add readRawEntry()/wrtiteRawEntry() methods to ZipInputStream/ZipOutputStream. We'd have to tweak ZipOutputStream.closeEntry() such that it can handle such "raw" writes (e.g. it wouldn't be able to check the CRC any more) and various methods like "skip()" in ZipInputStream. I think this might be interesting not only because it would allow updating zip-files without changing untouched entries, but also because it will considerably improve the speed of updating (i.e. we would save the time for repeatedly inflating and deflating). I'm not sure though how frequent the use-case of updating a zip-file really is? But that would definitely be a larger enhancement with SE scope. Do you think it's worth it?
Probably there should be no such utility method as updateJarFile at all. Perhaps instead the jar/zip code could be refactored so that tests can reuse the same code used by "jar u".
I'll leave that as an extra improvement for others :)
On Fri, Feb 28, 2020 at 10:50 AM Volker Simonis <volker.simonis@gmail.com> wrote:
Hi,
can I please have a review for this small test fix:
http://cr.openjdk.java.net/~simonis/webrevs/2020/8240235/ https://bugs.openjdk.java.net/browse/JDK-8240235
JarUtils.updateJar() and JarUtils.updateJarFile() update jar files by reading JarEntry-s from a source jar file and writing these exact JarEntry-s to the destination jar file followed be the inflated and re-deflated data belonging to this entry.
This approach is not correct, because JarEntry contains both, the compressed and uncompressed size of the corresponding entry. But the original Defalter which initially compressed that entry can be either different from the current one or it might have used a different compression level compared to the current Deflater which re-deflates the entry data.
When the newly created entry is closed, the new compressed size will be checked against the original compressed size if that was recorded in the JarEntry and an exception will be thrown, when they differ:
java.util.zip.ZipException: invalid entry compressed size (expected 237 but got 238 bytes) at java.base/java.util.zip.ZipOutputStream.closeEntry(ZipOutputStream.java:267) at java.base/java.util.zip.ZipOutputStream.putNextEntry(ZipOutputStream.java:192) at java.base/java.util.jar.JarOutputStream.putNextEntry(JarOutputStream.java:108) at jdk.test.lib.util.JarUtils.updateJar(JarUtils.java:294) at jdk.test.lib.util.JarUtils.updateJar(JarUtils.java:252) at HasUnsignedEntryTest.start(HasUnsignedEntryTest.java:77) at HasUnsignedEntryTest.main(HasUnsignedEntryTest.java:44) at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method) at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62) at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43) at java.base/java.lang.reflect.Method.invoke(Method.java:564) at com.sun.javatest.regtest.agent.MainWrapper$MainThread.run(MainWrapper.java:127) at java.base/java.lang.Thread.run(Thread.java:832) Suppressed: java.util.zip.ZipException: invalid entry compressed size (expected 237 but got 238 bytes) at java.base/java.util.zip.ZipOutputStream.closeEntry(ZipOutputStream.java:267) at java.base/java.util.zip.ZipOutputStream.finish(ZipOutputStream.java:360) at java.base/java.util.zip.DeflaterOutputStream.close(DeflaterOutputStream.java:237) at java.base/java.util.zip.ZipOutputStream.close(ZipOutputStream.java:377) at jdk.test.lib.util.JarUtils.updateJar(JarUtils.java:280)
The fix is trivial. Instead of copying the JarEntry-s verbatim to the output stream, simply write newly created JarEntry-s to the output stream which only contain the entry name. This is also the way how this is handled by the jar tool, when it updates jar files.
Thank you and best regards, Volker
On Mon, Mar 2, 2020 at 7:15 AM Volker Simonis <volker.simonis@gmail.com> wrote:
On Fri, Feb 28, 2020 at 8:02 PM Martin Buchholz <martinrb@google.com> wrote:
Welcome to the troublesome world of zip implementations!
Well, not many remember that the zip format was designed to work efficiently with floppy discs :)
It looks like by creating a new JarEntry you are discarding possible information from the old one, e.g. you might be discarding whether the input entry was compressed. If "jar u" does that, I would consider it a bug!
You're right. My fix was a little to simple, and can be improved:
http://cr.openjdk.java.net/~simonis/webrevs/2020/8240235.01
Preserving the time, comment, extra information, compression method and size/CRC if the compression method was "stored" is the best we can do. And this is actually exactly what "jar -" does (see [1]).
[1] http://hg.openjdk.java.net/jdk/jdk/file/e04746cec89c/src/jdk.jartool/share/c...
This fix Looks Good To Me.
Arguably there is a missing API that would allow you to copy zip entries from an input to an output completely intact, without having to decompress and recompress. (might be hard to design!)
Yes, I did think about this as well. It might not be that hard at all. We could simply add readRawEntry()/wrtiteRawEntry() methods to ZipInputStream/ZipOutputStream. We'd have to tweak ZipOutputStream.closeEntry() such that it can handle such "raw" writes (e.g. it wouldn't be able to check the CRC any more) and various methods like "skip()" in ZipInputStream. I think this might be interesting not only because it would allow updating zip-files without changing untouched entries, but also because it will considerably improve the speed of updating (i.e. we would save the time for repeatedly inflating and deflating). I'm not sure though how frequent the use-case of updating a zip-file really is?
But that would definitely be a larger enhancement with SE scope. Do you think it's worth it?
The platform "should" have something like this, and I've had similar thoughts myself in the past, and my employer would benefit, since there's a lot of zip munging that happens when building large applications. But as always, we have trouble finding the time.
Probably there should be no such utility method as updateJarFile at all. Perhaps instead the jar/zip code could be refactored so that tests can reuse the same code used by "jar u".
I'll leave that as an extra improvement for others :)
On Fri, Feb 28, 2020 at 10:50 AM Volker Simonis < volker.simonis@gmail.com> wrote:
Hi,
can I please have a review for this small test fix:
http://cr.openjdk.java.net/~simonis/webrevs/2020/8240235/ https://bugs.openjdk.java.net/browse/JDK-8240235
JarUtils.updateJar() and JarUtils.updateJarFile() update jar files by reading JarEntry-s from a source jar file and writing these exact JarEntry-s to the destination jar file followed be the inflated and re-deflated data belonging to this entry.
This approach is not correct, because JarEntry contains both, the compressed and uncompressed size of the corresponding entry. But the original Defalter which initially compressed that entry can be either different from the current one or it might have used a different compression level compared to the current Deflater which re-deflates the entry data.
When the newly created entry is closed, the new compressed size will be checked against the original compressed size if that was recorded in the JarEntry and an exception will be thrown, when they differ:
java.util.zip.ZipException: invalid entry compressed size (expected 237 but got 238 bytes) at
java.base/java.util.zip.ZipOutputStream.closeEntry(ZipOutputStream.java:267)
at
java.base/java.util.zip.ZipOutputStream.putNextEntry(ZipOutputStream.java:192)
at
java.base/java.util.jar.JarOutputStream.putNextEntry(JarOutputStream.java:108)
at jdk.test.lib.util.JarUtils.updateJar(JarUtils.java:294) at jdk.test.lib.util.JarUtils.updateJar(JarUtils.java:252) at HasUnsignedEntryTest.start(HasUnsignedEntryTest.java:77) at HasUnsignedEntryTest.main(HasUnsignedEntryTest.java:44) at
java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native
Method) at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62) at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43) at java.base/java.lang.reflect.Method.invoke(Method.java:564) at com.sun.javatest.regtest.agent.MainWrapper$MainThread.run(MainWrapper.java:127) at java.base/java.lang.Thread.run(Thread.java:832) Suppressed: java.util.zip.ZipException: invalid entry compressed size (expected 237 but got 238 bytes) at
java.base/java.util.zip.ZipOutputStream.closeEntry(ZipOutputStream.java:267)
at java.base/java.util.zip.ZipOutputStream.finish(ZipOutputStream.java:360) at
java.base/java.util.zip.DeflaterOutputStream.close(DeflaterOutputStream.java:237)
at java.base/java.util.zip.ZipOutputStream.close(ZipOutputStream.java:377) at
jdk.test.lib.util.JarUtils.updateJar(JarUtils.java:280)
The fix is trivial. Instead of copying the JarEntry-s verbatim to the output stream, simply write newly created JarEntry-s to the output stream which only contain the entry name. This is also the way how this is handled by the jar tool, when it updates jar files.
Thank you and best regards, Volker
I think the revised patch looks fine
On Mar 2, 2020, at 10:15 AM, Volker Simonis <volker.simonis@gmail.com> wrote:
On Fri, Feb 28, 2020 at 8:02 PM Martin Buchholz <martinrb@google.com> wrote:
Welcome to the troublesome world of zip implementations!
Well, not many remember that the zip format was designed to work efficiently with floppy discs :)
It looks like by creating a new JarEntry you are discarding possible information from the old one, e.g. you might be discarding whether the input entry was compressed. If "jar u" does that, I would consider it a bug!
You're right. My fix was a little to simple, and can be improved:
http://cr.openjdk.java.net/~simonis/webrevs/2020/8240235.01
Preserving the time, comment, extra information, compression method and size/CRC if the compression method was "stored" is the best we can do. And this is actually exactly what "jar -" does (see [1]).
[1] http://hg.openjdk.java.net/jdk/jdk/file/e04746cec89c/src/jdk.jartool/share/c...
Arguably there is a missing API that would allow you to copy zip entries from an input to an output completely intact, without having to decompress and recompress. (might be hard to design!)
Yes, I did think about this as well. It might not be that hard at all. We could simply add readRawEntry()/wrtiteRawEntry() methods to ZipInputStream/ZipOutputStream. We'd have to tweak ZipOutputStream.closeEntry() such that it can handle such "raw" writes (e.g. it wouldn't be able to check the CRC any more) and various methods like "skip()" in ZipInputStream. I think this might be interesting not only because it would allow updating zip-files without changing untouched entries, but also because it will considerably improve the speed of updating (i.e. we would save the time for repeatedly inflating and deflating). I'm not sure though how frequent the use-case of updating a zip-file really is?
But that would definitely be a larger enhancement with SE scope. Do you think it's worth it?
Probably there should be no such utility method as updateJarFile at all. Perhaps instead the jar/zip code could be refactored so that tests can reuse the same code used by "jar u".
I'll leave that as an extra improvement for others :)
On Fri, Feb 28, 2020 at 10:50 AM Volker Simonis <volker.simonis@gmail.com> wrote:
Hi,
can I please have a review for this small test fix:
http://cr.openjdk.java.net/~simonis/webrevs/2020/8240235/ https://bugs.openjdk.java.net/browse/JDK-8240235
JarUtils.updateJar() and JarUtils.updateJarFile() update jar files by reading JarEntry-s from a source jar file and writing these exact JarEntry-s to the destination jar file followed be the inflated and re-deflated data belonging to this entry.
This approach is not correct, because JarEntry contains both, the compressed and uncompressed size of the corresponding entry. But the original Defalter which initially compressed that entry can be either different from the current one or it might have used a different compression level compared to the current Deflater which re-deflates the entry data.
When the newly created entry is closed, the new compressed size will be checked against the original compressed size if that was recorded in the JarEntry and an exception will be thrown, when they differ:
java.util.zip.ZipException: invalid entry compressed size (expected 237 but got 238 bytes) at java.base/java.util.zip.ZipOutputStream.closeEntry(ZipOutputStream.java:267) at java.base/java.util.zip.ZipOutputStream.putNextEntry(ZipOutputStream.java:192) at java.base/java.util.jar.JarOutputStream.putNextEntry(JarOutputStream.java:108) at jdk.test.lib.util.JarUtils.updateJar(JarUtils.java:294) at jdk.test.lib.util.JarUtils.updateJar(JarUtils.java:252) at HasUnsignedEntryTest.start(HasUnsignedEntryTest.java:77) at HasUnsignedEntryTest.main(HasUnsignedEntryTest.java:44) at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method) at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62) at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43) at java.base/java.lang.reflect.Method.invoke(Method.java:564) at com.sun.javatest.regtest.agent.MainWrapper$MainThread.run(MainWrapper.java:127) at java.base/java.lang.Thread.run(Thread.java:832) Suppressed: java.util.zip.ZipException: invalid entry compressed size (expected 237 but got 238 bytes) at java.base/java.util.zip.ZipOutputStream.closeEntry(ZipOutputStream.java:267) at java.base/java.util.zip.ZipOutputStream.finish(ZipOutputStream.java:360) at java.base/java.util.zip.DeflaterOutputStream.close(DeflaterOutputStream.java:237) at java.base/java.util.zip.ZipOutputStream.close(ZipOutputStream.java:377) at jdk.test.lib.util.JarUtils.updateJar(JarUtils.java:280)
The fix is trivial. Instead of copying the JarEntry-s verbatim to the output stream, simply write newly created JarEntry-s to the output stream which only contain the entry name. This is also the way how this is handled by the jar tool, when it updates jar files.
Thank you and best regards, Volker
<http://oracle.com/us/design/oracle-email-sig-198324.gif> <http://oracle.com/us/design/oracle-email-sig-198324.gif> <http://oracle.com/us/design/oracle-email-sig-198324.gif> <http://oracle.com/us/design/oracle-email-sig-198324.gif>Lance Andersen| Principal Member of Technical Staff | +1.781.442.2037 Oracle Java Engineering 1 Network Drive Burlington, MA 01803 Lance.Andersen@oracle.com <mailto:Lance.Andersen@oracle.com>
Thanks for the review Martin, Lance. On Mon, Mar 2, 2020 at 5:30 PM Lance Andersen <lance.andersen@oracle.com> wrote:
I think the revised patch looks fine
On Mar 2, 2020, at 10:15 AM, Volker Simonis <volker.simonis@gmail.com> wrote:
On Fri, Feb 28, 2020 at 8:02 PM Martin Buchholz <martinrb@google.com> wrote:
Welcome to the troublesome world of zip implementations!
Well, not many remember that the zip format was designed to work efficiently with floppy discs :)
It looks like by creating a new JarEntry you are discarding possible information from the old one, e.g. you might be discarding whether the input entry was compressed. If "jar u" does that, I would consider it a bug!
You're right. My fix was a little to simple, and can be improved:
http://cr.openjdk.java.net/~simonis/webrevs/2020/8240235.01
Preserving the time, comment, extra information, compression method and size/CRC if the compression method was "stored" is the best we can do. And this is actually exactly what "jar -" does (see [1]).
[1] http://hg.openjdk.java.net/jdk/jdk/file/e04746cec89c/src/jdk.jartool/share/c...
Arguably there is a missing API that would allow you to copy zip entries from an input to an output completely intact, without having to decompress and recompress. (might be hard to design!)
Yes, I did think about this as well. It might not be that hard at all. We could simply add readRawEntry()/wrtiteRawEntry() methods to ZipInputStream/ZipOutputStream. We'd have to tweak ZipOutputStream.closeEntry() such that it can handle such "raw" writes (e.g. it wouldn't be able to check the CRC any more) and various methods like "skip()" in ZipInputStream. I think this might be interesting not only because it would allow updating zip-files without changing untouched entries, but also because it will considerably improve the speed of updating (i.e. we would save the time for repeatedly inflating and deflating). I'm not sure though how frequent the use-case of updating a zip-file really is?
But that would definitely be a larger enhancement with SE scope. Do you think it's worth it?
Probably there should be no such utility method as updateJarFile at all. Perhaps instead the jar/zip code could be refactored so that tests can reuse the same code used by "jar u".
I'll leave that as an extra improvement for others :)
On Fri, Feb 28, 2020 at 10:50 AM Volker Simonis <volker.simonis@gmail.com> wrote:
Hi,
can I please have a review for this small test fix:
http://cr.openjdk.java.net/~simonis/webrevs/2020/8240235/ https://bugs.openjdk.java.net/browse/JDK-8240235
JarUtils.updateJar() and JarUtils.updateJarFile() update jar files by reading JarEntry-s from a source jar file and writing these exact JarEntry-s to the destination jar file followed be the inflated and re-deflated data belonging to this entry.
This approach is not correct, because JarEntry contains both, the compressed and uncompressed size of the corresponding entry. But the original Defalter which initially compressed that entry can be either different from the current one or it might have used a different compression level compared to the current Deflater which re-deflates the entry data.
When the newly created entry is closed, the new compressed size will be checked against the original compressed size if that was recorded in the JarEntry and an exception will be thrown, when they differ:
java.util.zip.ZipException: invalid entry compressed size (expected 237 but got 238 bytes) at java.base/java.util.zip.ZipOutputStream.closeEntry(ZipOutputStream.java:267) at java.base/java.util.zip.ZipOutputStream.putNextEntry(ZipOutputStream.java:192) at java.base/java.util.jar.JarOutputStream.putNextEntry(JarOutputStream.java:108) at jdk.test.lib.util.JarUtils.updateJar(JarUtils.java:294) at jdk.test.lib.util.JarUtils.updateJar(JarUtils.java:252) at HasUnsignedEntryTest.start(HasUnsignedEntryTest.java:77) at HasUnsignedEntryTest.main(HasUnsignedEntryTest.java:44) at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method) at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62) at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43) at java.base/java.lang.reflect.Method.invoke(Method.java:564) at com.sun.javatest.regtest.agent.MainWrapper$MainThread.run(MainWrapper.java:127) at java.base/java.lang.Thread.run(Thread.java:832) Suppressed: java.util.zip.ZipException: invalid entry compressed size (expected 237 but got 238 bytes) at java.base/java.util.zip.ZipOutputStream.closeEntry(ZipOutputStream.java:267) at java.base/java.util.zip.ZipOutputStream.finish(ZipOutputStream.java:360) at java.base/java.util.zip.DeflaterOutputStream.close(DeflaterOutputStream.java:237) at java.base/java.util.zip.ZipOutputStream.close(ZipOutputStream.java:377) at jdk.test.lib.util.JarUtils.updateJar(JarUtils.java:280)
The fix is trivial. Instead of copying the JarEntry-s verbatim to the output stream, simply write newly created JarEntry-s to the output stream which only contain the entry name. This is also the way how this is handled by the jar tool, when it updates jar files.
Thank you and best regards, Volker
Lance Andersen| Principal Member of Technical Staff | +1.781.442.2037 Oracle Java Engineering 1 Network Drive Burlington, MA 01803 Lance.Andersen@oracle.com
Hi Volker, this looks good to me. Best regards Christoph
-----Original Message----- From: core-libs-dev <core-libs-dev-bounces@openjdk.java.net> On Behalf Of Volker Simonis Sent: Freitag, 28. Februar 2020 19:48 To: Java Core Libs <core-libs-dev@openjdk.java.net> Subject: RFR(s): 8240235: jdk.test.lib.util.JarUtils updates jar files incorrectly
Hi,
can I please have a review for this small test fix:
http://cr.openjdk.java.net/~simonis/webrevs/2020/8240235/ https://bugs.openjdk.java.net/browse/JDK-8240235
JarUtils.updateJar() and JarUtils.updateJarFile() update jar files by reading JarEntry-s from a source jar file and writing these exact JarEntry-s to the destination jar file followed be the inflated and re-deflated data belonging to this entry.
This approach is not correct, because JarEntry contains both, the compressed and uncompressed size of the corresponding entry. But the original Defalter which initially compressed that entry can be either different from the current one or it might have used a different compression level compared to the current Deflater which re-deflates the entry data.
When the newly created entry is closed, the new compressed size will be checked against the original compressed size if that was recorded in the JarEntry and an exception will be thrown, when they differ:
java.util.zip.ZipException: invalid entry compressed size (expected 237 but got 238 bytes) at java.base/java.util.zip.ZipOutputStream.closeEntry(ZipOutputStream.java:26 7) at java.base/java.util.zip.ZipOutputStream.putNextEntry(ZipOutputStream.java :192) at java.base/java.util.jar.JarOutputStream.putNextEntry(JarOutputStream.java :108) at jdk.test.lib.util.JarUtils.updateJar(JarUtils.java:294) at jdk.test.lib.util.JarUtils.updateJar(JarUtils.java:252) at HasUnsignedEntryTest.start(HasUnsignedEntryTest.java:77) at HasUnsignedEntryTest.main(HasUnsignedEntryTest.java:44) at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method) at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMet hodAccessorImpl.java:62) at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(Delega tingMethodAccessorImpl.java:43) at java.base/java.lang.reflect.Method.invoke(Method.java:564) at com.sun.javatest.regtest.agent.MainWrapper$MainThread.run(MainWrapp er.java:127) at java.base/java.lang.Thread.run(Thread.java:832) Suppressed: java.util.zip.ZipException: invalid entry compressed size (expected 237 but got 238 bytes) at java.base/java.util.zip.ZipOutputStream.closeEntry(ZipOutputStream.java:26 7) at java.base/java.util.zip.ZipOutputStream.finish(ZipOutputStream.java:360) at java.base/java.util.zip.DeflaterOutputStream.close(DeflaterOutputStream.ja va:237) at java.base/java.util.zip.ZipOutputStream.close(ZipOutputStream.java:377) at jdk.test.lib.util.JarUtils.updateJar(JarUtils.java:280)
The fix is trivial. Instead of copying the JarEntry-s verbatim to the output stream, simply write newly created JarEntry-s to the output stream which only contain the entry name. This is also the way how this is handled by the jar tool, when it updates jar files.
Thank you and best regards, Volker
Hi Christoph, thanks for looking at my change. I've slightly improved the implementation as suggested by Martin to preserve all the zip attributes which can be preserved. Still fine for you? http://cr.openjdk.java.net/~simonis/webrevs/2020/8240235.01 On Sun, Mar 1, 2020 at 9:21 PM Langer, Christoph <christoph.langer@sap.com> wrote:
Hi Volker,
this looks good to me.
Best regards Christoph
-----Original Message----- From: core-libs-dev <core-libs-dev-bounces@openjdk.java.net> On Behalf Of Volker Simonis Sent: Freitag, 28. Februar 2020 19:48 To: Java Core Libs <core-libs-dev@openjdk.java.net> Subject: RFR(s): 8240235: jdk.test.lib.util.JarUtils updates jar files incorrectly
Hi,
can I please have a review for this small test fix:
http://cr.openjdk.java.net/~simonis/webrevs/2020/8240235/ https://bugs.openjdk.java.net/browse/JDK-8240235
JarUtils.updateJar() and JarUtils.updateJarFile() update jar files by reading JarEntry-s from a source jar file and writing these exact JarEntry-s to the destination jar file followed be the inflated and re-deflated data belonging to this entry.
This approach is not correct, because JarEntry contains both, the compressed and uncompressed size of the corresponding entry. But the original Defalter which initially compressed that entry can be either different from the current one or it might have used a different compression level compared to the current Deflater which re-deflates the entry data.
When the newly created entry is closed, the new compressed size will be checked against the original compressed size if that was recorded in the JarEntry and an exception will be thrown, when they differ:
java.util.zip.ZipException: invalid entry compressed size (expected 237 but got 238 bytes) at java.base/java.util.zip.ZipOutputStream.closeEntry(ZipOutputStream.java:26 7) at java.base/java.util.zip.ZipOutputStream.putNextEntry(ZipOutputStream.java :192) at java.base/java.util.jar.JarOutputStream.putNextEntry(JarOutputStream.java :108) at jdk.test.lib.util.JarUtils.updateJar(JarUtils.java:294) at jdk.test.lib.util.JarUtils.updateJar(JarUtils.java:252) at HasUnsignedEntryTest.start(HasUnsignedEntryTest.java:77) at HasUnsignedEntryTest.main(HasUnsignedEntryTest.java:44) at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method) at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMet hodAccessorImpl.java:62) at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(Delega tingMethodAccessorImpl.java:43) at java.base/java.lang.reflect.Method.invoke(Method.java:564) at com.sun.javatest.regtest.agent.MainWrapper$MainThread.run(MainWrapp er.java:127) at java.base/java.lang.Thread.run(Thread.java:832) Suppressed: java.util.zip.ZipException: invalid entry compressed size (expected 237 but got 238 bytes) at java.base/java.util.zip.ZipOutputStream.closeEntry(ZipOutputStream.java:26 7) at java.base/java.util.zip.ZipOutputStream.finish(ZipOutputStream.java:360) at java.base/java.util.zip.DeflaterOutputStream.close(DeflaterOutputStream.ja va:237) at java.base/java.util.zip.ZipOutputStream.close(ZipOutputStream.java:377) at jdk.test.lib.util.JarUtils.updateJar(JarUtils.java:280)
The fix is trivial. Instead of copying the JarEntry-s verbatim to the output stream, simply write newly created JarEntry-s to the output stream which only contain the entry name. This is also the way how this is handled by the jar tool, when it updates jar files.
Thank you and best regards, Volker
Hi Volker, sure, looks good. Unfortunately I don't get the mails from Martin ☹ Cheers Christoph
-----Original Message----- From: Volker Simonis <volker.simonis@gmail.com> Sent: Montag, 2. März 2020 16:17 To: Langer, Christoph <christoph.langer@sap.com> Cc: Java Core Libs <core-libs-dev@openjdk.java.net> Subject: Re: RFR(s): 8240235: jdk.test.lib.util.JarUtils updates jar files incorrectly
Hi Christoph,
thanks for looking at my change. I've slightly improved the implementation as suggested by Martin to preserve all the zip attributes which can be preserved. Still fine for you?
http://cr.openjdk.java.net/~simonis/webrevs/2020/8240235.01
On Sun, Mar 1, 2020 at 9:21 PM Langer, Christoph <christoph.langer@sap.com> wrote:
Hi Volker,
this looks good to me.
Best regards Christoph
-----Original Message----- From: core-libs-dev <core-libs-dev-bounces@openjdk.java.net> On
Behalf
Of Volker Simonis Sent: Freitag, 28. Februar 2020 19:48 To: Java Core Libs <core-libs-dev@openjdk.java.net> Subject: RFR(s): 8240235: jdk.test.lib.util.JarUtils updates jar files incorrectly
Hi,
can I please have a review for this small test fix:
http://cr.openjdk.java.net/~simonis/webrevs/2020/8240235/ https://bugs.openjdk.java.net/browse/JDK-8240235
JarUtils.updateJar() and JarUtils.updateJarFile() update jar files by reading JarEntry-s from a source jar file and writing these exact JarEntry-s to the destination jar file followed be the inflated and re-deflated data belonging to this entry.
This approach is not correct, because JarEntry contains both, the compressed and uncompressed size of the corresponding entry. But the original Defalter which initially compressed that entry can be either different from the current one or it might have used a different compression level compared to the current Deflater which re-deflates the entry data.
When the newly created entry is closed, the new compressed size will be checked against the original compressed size if that was recorded in the JarEntry and an exception will be thrown, when they differ:
java.util.zip.ZipException: invalid entry compressed size (expected 237 but got 238 bytes) at
java.base/java.util.zip.ZipOutputStream.closeEntry(ZipOutputStream.java:26
7) at
java.base/java.util.zip.ZipOutputStream.putNextEntry(ZipOutputStream.java
:192) at
java.base/java.util.jar.JarOutputStream.putNextEntry(JarOutputStream.java
:108) at jdk.test.lib.util.JarUtils.updateJar(JarUtils.java:294) at jdk.test.lib.util.JarUtils.updateJar(JarUtils.java:252) at HasUnsignedEntryTest.start(HasUnsignedEntryTest.java:77) at HasUnsignedEntryTest.main(HasUnsignedEntryTest.java:44) at
java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native
Method) at
java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMet
hodAccessorImpl.java:62) at
java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(Delega
tingMethodAccessorImpl.java:43) at java.base/java.lang.reflect.Method.invoke(Method.java:564) at
com.sun.javatest.regtest.agent.MainWrapper$MainThread.run(MainWrapp
er.java:127) at java.base/java.lang.Thread.run(Thread.java:832) Suppressed: java.util.zip.ZipException: invalid entry compressed size (expected 237 but got 238 bytes) at
java.base/java.util.zip.ZipOutputStream.closeEntry(ZipOutputStream.java:26
7) at
java.base/java.util.zip.ZipOutputStream.finish(ZipOutputStream.java:360)
at
java.base/java.util.zip.DeflaterOutputStream.close(DeflaterOutputStream.ja
va:237) at java.base/java.util.zip.ZipOutputStream.close(ZipOutputStream.java:377) at jdk.test.lib.util.JarUtils.updateJar(JarUtils.java:280)
The fix is trivial. Instead of copying the JarEntry-s verbatim to the output stream, simply write newly created JarEntry-s to the output stream which only contain the entry name. This is also the way how this is handled by the jar tool, when it updates jar files.
Thank you and best regards, Volker
On 03/03/20 12:45 pm, Langer, Christoph wrote:
Unfortunately I don't get the mails from Martin ☹
Same for me. They always end up in spam folder which I usually check once a month. I see that there's already https://bugs.openjdk.java.net/browse/JDK-8213225 which seems to be the same issue. -Jaikiran
On 03/03/20 12:45 pm, Langer, Christoph wrote:
Unfortunately I don't get the mails from Martin ☹
Same for me. They always end up in spam folder which I usually check once a month. I see that there's already https://bugs.openjdk.java.net/browse/JDK-8213225 which seems to be the same issue.
You are lucky, that you at least get it delivered to the spam folder... 😉 /Christoph
For the truly desperate in search of a spam folder, recent emails from google.com can be viewed here: https://openjdk.markmail.org/search/?q=from%3Agoogle.com%20date%3A2020- On Mon, Mar 2, 2020 at 11:15 PM Langer, Christoph <christoph.langer@sap.com> wrote:
Hi Volker,
sure, looks good. Unfortunately I don't get the mails from Martin ☹
Cheers Christoph
-----Original Message----- From: Volker Simonis <volker.simonis@gmail.com> Sent: Montag, 2. März 2020 16:17 To: Langer, Christoph <christoph.langer@sap.com> Cc: Java Core Libs <core-libs-dev@openjdk.java.net> Subject: Re: RFR(s): 8240235: jdk.test.lib.util.JarUtils updates jar files incorrectly
Hi Christoph,
thanks for looking at my change. I've slightly improved the implementation as suggested by Martin to preserve all the zip attributes which can be preserved. Still fine for you?
http://cr.openjdk.java.net/~simonis/webrevs/2020/8240235.01
On Sun, Mar 1, 2020 at 9:21 PM Langer, Christoph <christoph.langer@sap.com> wrote:
Hi Volker,
this looks good to me.
Best regards Christoph
-----Original Message----- From: core-libs-dev <core-libs-dev-bounces@openjdk.java.net> On
Behalf
Of Volker Simonis Sent: Freitag, 28. Februar 2020 19:48 To: Java Core Libs <core-libs-dev@openjdk.java.net> Subject: RFR(s): 8240235: jdk.test.lib.util.JarUtils updates jar files incorrectly
Hi,
can I please have a review for this small test fix:
http://cr.openjdk.java.net/~simonis/webrevs/2020/8240235/ https://bugs.openjdk.java.net/browse/JDK-8240235
JarUtils.updateJar() and JarUtils.updateJarFile() update jar files by reading JarEntry-s from a source jar file and writing these exact JarEntry-s to the destination jar file followed be the inflated and re-deflated data belonging to this entry.
This approach is not correct, because JarEntry contains both, the compressed and uncompressed size of the corresponding entry. But the original Defalter which initially compressed that entry can be either different from the current one or it might have used a different compression level compared to the current Deflater which re-deflates the entry data.
When the newly created entry is closed, the new compressed size will be checked against the original compressed size if that was recorded in the JarEntry and an exception will be thrown, when they differ:
java.util.zip.ZipException: invalid entry compressed size (expected 237 but got 238 bytes) at
java.base/java.util.zip.ZipOutputStream.closeEntry(ZipOutputStream.java:26
7) at
java.base/java.util.zip.ZipOutputStream.putNextEntry(ZipOutputStream.java
:192) at
java.base/java.util.jar.JarOutputStream.putNextEntry(JarOutputStream.java
:108) at jdk.test.lib.util.JarUtils.updateJar(JarUtils.java:294) at jdk.test.lib.util.JarUtils.updateJar(JarUtils.java:252) at HasUnsignedEntryTest.start(HasUnsignedEntryTest.java:77) at HasUnsignedEntryTest.main(HasUnsignedEntryTest.java:44) at
java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native
Method) at
java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMet
hodAccessorImpl.java:62) at
java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(Delega
tingMethodAccessorImpl.java:43) at java.base/java.lang.reflect.Method.invoke(Method.java:564) at
com.sun.javatest.regtest.agent.MainWrapper$MainThread.run(MainWrapp
er.java:127) at java.base/java.lang.Thread.run(Thread.java:832) Suppressed: java.util.zip.ZipException: invalid entry compressed size (expected 237 but got 238 bytes) at
java.base/java.util.zip.ZipOutputStream.closeEntry(ZipOutputStream.java:26
7) at
java.base/java.util.zip.ZipOutputStream.finish(ZipOutputStream.java:360)
at
java.base/java.util.zip.DeflaterOutputStream.close(DeflaterOutputStream.ja
va:237) at
java.base/java.util.zip.ZipOutputStream.close(ZipOutputStream.java:377)
at
jdk.test.lib.util.JarUtils.updateJar(JarUtils.java:280)
The fix is trivial. Instead of copying the JarEntry-s verbatim to the output stream, simply write newly created JarEntry-s to the output stream which only contain the entry name. This is also the way how this is handled by the jar tool, when it updates jar files.
Thank you and best regards, Volker
participants (5)
-
Jaikiran Pai
-
Lance Andersen
-
Langer, Christoph
-
Martin Buchholz
-
Volker Simonis