March 2020 - core-libs-dev

RFR(S): 8240333: jmod incorrectly updates .jar and .jmod files during hashing
by Volker Simonis 03 Mar '20

03 Mar '20

Hi, can I please get a review for the following small fix: http://cr.openjdk.java.net/~simonis/webrevs/2020/8240333/ https://bugs.openjdk.java.net/browse/JDK-8240333 The "jmod" tool needs to update .jar files as well as .jmod files when adding hashes to the module-info file. This is handled in the method jdk.tools.jmod.JmodTask.updateModularJar() for modular jar files and in the methods jdk.tools.jmod.JmodTask.updateJmodFile() and jdk.tools.jmod.JmodOutputStream.writeEntry() for .jmod files. Unfortunately, both implementations are incorrect because they are writing the JarEntry/ZipEntry entries which they reading from the ZipInputStream verbatim to the ZipOutputStream. This approach is not correct, because Zip/Entry/JarEntry contains both, the compressed and uncompressed size of the corresponding entry. But the original Deflater 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 ZipEntry/JarEntry entry and an exception will be thrown, when they differ. For modular jar files it looks as follows: $ jmod hash --module-path . --hash-modules .* Error: java.util.zip.ZipException: invalid entry compressed size (expected 58 but got 57 bytes) java.io.UncheckedIOException: java.util.zip.ZipException: invalid entry compressed size (expected 58 but got 57 bytes) at jdk.jlink/jdk.tools.jmod.JmodTask$Hasher.lambda$updateModularJar$3(JmodTask.java:995) at java.base/java.util.zip.ZipFile$EntrySpliterator.tryAdvance(ZipFile.java:571) at java.base/java.util.Spliterator.forEachRemaining(Spliterator.java:326) at java.base/java.util.stream.ReferencePipeline$Head.forEach(ReferencePipeline.java:658) at jdk.jlink/jdk.tools.jmod.JmodTask$Hasher.updateModularJar(JmodTask.java:980) at jdk.jlink/jdk.tools.jmod.JmodTask$Hasher.updateModuleInfo(JmodTask.java:957) at jdk.jlink/jdk.tools.jmod.JmodTask.lambda$hashModules$3(JmodTask.java:300) at java.base/java.util.HashMap.forEach(HashMap.java:1425) at jdk.jlink/jdk.tools.jmod.JmodTask.hashModules(JmodTask.java:291) at jdk.jlink/jdk.tools.jmod.JmodTask.run(JmodTask.java:220) at jdk.jlink/jdk.tools.jmod.Main.main(Main.java:34) Suppressed: java.util.zip.ZipException: invalid entry compressed size (expected 58 but got 57 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.jlink/jdk.tools.jmod.JmodTask$Hasher.updateModularJar(JmodTask.java:976) ... 6 more Caused by: java.util.zip.ZipException: invalid entry compressed size (expected 58 but got 57 bytes) at java.base/java.util.zip.ZipOutputStream.closeEntry(ZipOutputStream.java:267) at jdk.jlink/jdk.tools.jmod.JmodTask$Hasher.lambda$updateModularJar$3(JmodTask.java:992) ... 10 more For jmod files it looks like this: $ jmod hash --module-path . --hash-modules .* Error: java.util.zip.ZipException: invalid entry compressed size (expected 383 but got 377 bytes) java.io.UncheckedIOException: java.util.zip.ZipException: invalid entry compressed size (expected 383 but got 377 bytes) at jdk.jlink/jdk.tools.jmod.JmodTask$Hasher.lambda$updateJmodFile$4(JmodTask.java:1021) at java.base/java.util.stream.ForEachOps$ForEachOp$OfRef.accept(ForEachOps.java:183) at java.base/java.util.stream.ReferencePipeline$3$1.accept(ReferencePipeline.java:195) at java.base/java.util.zip.ZipFile$EntrySpliterator.tryAdvance(ZipFile.java:571) at java.base/java.util.Spliterator.forEachRemaining(Spliterator.java:326) at java.base/java.util.stream.AbstractPipeline.copyInto(AbstractPipeline.java:484) at java.base/java.util.stream.AbstractPipeline.wrapAndCopyInto(AbstractPipeline.java:474) at java.base/java.util.stream.ForEachOps$ForEachOp.evaluateSequential(ForEachOps.java:150) at java.base/java.util.stream.ForEachOps$ForEachOp$OfRef.evaluateSequential(ForEachOps.java:173) at java.base/java.util.stream.AbstractPipeline.evaluate(AbstractPipeline.java:234) at java.base/java.util.stream.ReferencePipeline.forEach(ReferencePipeline.java:497) at jdk.jlink/jdk.tools.jmod.JmodTask$Hasher.updateJmodFile(JmodTask.java:1009) at jdk.jlink/jdk.tools.jmod.JmodTask$Hasher.updateModuleInfo(JmodTask.java:955) at jdk.jlink/jdk.tools.jmod.JmodTask.lambda$hashModules$3(JmodTask.java:300) at java.base/java.util.HashMap.forEach(HashMap.java:1425) at jdk.jlink/jdk.tools.jmod.JmodTask.hashModules(JmodTask.java:291) at jdk.jlink/jdk.tools.jmod.JmodTask.run(JmodTask.java:220) at jdk.jlink/jdk.tools.jmod.Main.main(Main.java:34) Suppressed: java.util.zip.ZipException: invalid entry compressed size (expected 383 but got 377 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.jlink/jdk.tools.jmod.JmodOutputStream.close(JmodOutputStream.java:114) at jdk.jlink/jdk.tools.jmod.JmodTask$Hasher.updateJmodFile(JmodTask.java:1006) ... 6 more Caused by: java.util.zip.ZipException: invalid entry compressed size (expected 383 but got 377 bytes) at java.base/java.util.zip.ZipOutputStream.closeEntry(ZipOutputStream.java:267) at jdk.jlink/jdk.tools.jmod.JmodOutputStream.writeEntry(JmodOutputStream.java:97) at jdk.jlink/jdk.tools.jmod.JmodTask$Hasher.lambda$updateJmodFile$4(JmodTask.java:1018) ... 17 more The errors can be provoked by creating the initial jar or jmod files with a different zlib implementation (e.g. LD_LIBRARY_PATH=/ziptests/build/zlib-cloudflare jar -cf ../mb_cf.jar *) The fix is simple. Instead of copying the ZipEntry/JarEntry entries verbatim to the output stream, simply write newly created ZipEntry entries to the output stream which only contain the required attributes. This is also the way how this is done by the jar tool, when it updates jar files. For the modular jar file case, creating a new entry is not feasible, because at that specific location we are dealing with JerEntry entries which can contain additional information. Unfortunately, it is not possible to set this information on a newly created JarEntry entry. But we can still reset the "compressedSize" of the JarEntry to "-1" which also prevents the additional check after the entry has been written. Thank you and best regards, Volker PS: and by the way, this is the last bug related to the wrong usage of ZipOutputSream.putNextEntry() within the OpenJDK :) I've scanned all the sources and couldn't find any other suspicious places.

4 8

RFR: JDK-8237966,: Creating runtime pkg requires --mac-package-identifier
by Andy Herrick 03 Mar '20

03 Mar '20

Please review the fix to issue [1] at [2]. The initial concern with defaulting to the application or installer name was that is might not be a valid mac package identifier (use only alphanumeric, '.' , and '-' chars) but this can be true when the id is derived from main class (as in application case) or explicitly specified (using --mac-package-identifier). So this change now tests it in any case. [1] https://bugs.openjdk.java.net/browse/JDK-8237966 [2] http://cr.openjdk.java.net/~herrick/8237966/webrev.01/ /Andy

4 5

RFR: JDK-8238692: MacOS runtime Installer issue
by Andy Herrick 03 Mar '20

03 Mar '20

Please review the jpackage fix at [1] for the related issues [2], [3], and [4]. This will basically make pkg type runtime installers work on macosx, while we wait for JDK-8237971 to either fix or disable dmg type runtime installers. /Andy [1] http://cr.openjdk.java.net/~herrick/8238692/webrev.02/ [2] https://bugs.openjdk.java.net/browse/JDK-8238692 [3] https://bugs.openjdk.java.net/browse/JDK-8237966 [4] https://bugs.openjdk.java.net/browse/JDK-8237970

4 4

[11u] RFR: 8232854: URLClassLoader.close() doesn't close cached JAR file on Windows when load() fails
by Alex Kashchenko 03 Mar '20

03 Mar '20

Hi, Please review the fix to JDK-8232854 for 11u: Jira issue: https://bugs.openjdk.java.net/browse/JDK-8232854 Webrev: http://cr.openjdk.java.net/~akasko/jdk11u/8232854/webrev.00/ Patch is implemented based on a test code included with the Jira issue. It fixes the case, when URLClassLoader is created with URL, that stars with "jar:file:" and ends with non-root path inside JAR like "foo.jar!/somedir/". Codepath in this case includes URLClassPath.Loader class, that requires special handling when resources are loaded from a JAR file. Issue with closing JAR file (on classloader close) on this codepath was previously fixed in JDK-6896088: Jira issue: https://bugs.openjdk.java.net/browse/JDK-6896088 Changeset: http://hg.openjdk.java.net/jdk/jdk/rev/3af394bb6f59 This fix is complementary to 6896088, it is filling the "jarfile" field in URLClassPath.Loader class (to be able to close the JAR file later) in case, when requested resource failed to be loaded from this JAR. Note, that getJarFile() on URLConnection to original resource cannot be used in this case, because this method checks that requested resource is valid. URLConnection to the root URL in this JAR is used instead. There probably exist more elegant solutions to this problem. This patch is fix-only and should not affect any existing logic in URLClassPath. Test included with this fix is similar to java/net/URLClassLoader/B6896088.java test. It uses specific JAR structure (with a classfile inside "somedir" directory that is not a part of class package) and checks JAR file closing after both successful and failed load attempts. Test fails on Windows without this patch, on Linux it always passes. -- -Alex

2 3

RFR(s): 8240235: jdk.test.lib.util.JarUtils updates jar files incorrectly
by Volker Simonis 03 Mar '20

03 Mar '20

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

5 11

Re: RFR: 8237967: No proper error message when --runtime-image points to non-existent path
by Andy Herrick 03 Mar '20

03 Mar '20

I sent this in reply to Alexey, needed to send it to alias: On 2/6/2020 4:54 PM, Andy Herrick wrote: > I had fixed that and failed to push the revised webrev [4] > > [4] http://cr.openjdk.java.net/~herrick/8237967/webrev.03/ > > /Andy > > On 2/6/2020 11:46 AM, Alexey Semenyuk wrote: >> Andy, >> >> defaultSupported() is defined, but not called form the test. What is >> the point of this method? >> >> - Alexey >> >> On 2/6/2020 8:42 AM, Andy Herrick wrote: >>> revised fir at [3] to check for non-existent resource-dir as well as >>> runtime-image dir. >>> >>> [3] http://cr.openjdk.java.net/~herrick/8237967/webrev.02/ >>> >>> /Andy >>> >>> On 2/5/2020 7:46 PM, Andy Herrick wrote: >>>> OK - this makes sense - will update tomorrow. >>>> >>>> /Andy >>>> >>>> On 2/5/2020 4:24 PM, Alexander Matveev wrote: >>>>> Hi Andy, >>>>> >>>>> Fix looks good, but I think we should do same for --resource-dir. >>>>> I do not think that non-existing directory should be same as >>>>> empty. User might misspell directory name and it will not be clear >>>>> for user why we not using information from --resource-dir. >>>>> >>>>> Thanks, >>>>> Alexander >>>>> >>>>> On 2/5/2020 10:23 AM, Andy Herrick wrote: >>>>>> This simple fix [1] to jpackage bug [2] simply adds a proper >>>>>> error message when --runtime-image points to a non-existent >>>>>> directory path. >>>>>> >>>>>> [1] http://cr.openjdk.java.net/~herrick/8237967/webrev.01/index.html >>>>>> >>>>>> [2] https://bugs.openjdk.java.net/browse/JDK-8237967 >>>>>> >>>>>> /Andy >>>>>> >>>>> >>

2 1