PATCH [6901992] : Possible InvalidJarIndexException due to bug in sun.misc.JarIndex.merge()

Chris Hegarty chris.hegarty at oracle.com
Tue Jun 19 10:15:30 UTC 2012


To close the loop on this, here is a link to the changeset:

   http://hg.openjdk.java.net/jdk8/tl/jdk/rev/efc2791d7c5d

Thanks for this valuable contribution Diego.

-Chris

On 16/06/12 05:20, Diego Belfer wrote:
> Hi Chris,
>
> Here is a new patch containing new version of the tests. The new
> versions generate a the Jar on the fly as we discussed.
>
> Let me know if there is anything else you think should be improved.
>
> Best,
> Diego
>
> On Thu, Jun 14, 2012 at 7:32 AM, Chris Hegarty <chris.hegarty at oracle.com
> <mailto:chris.hegarty at oracle.com>> wrote:
>
>     Diego,
>
>     It's not too difficult to create jars on the fly using the Jar API.
>     Here is a small example that I think would work nice in this case.
>     Files created ( and paths are relative to the jtreg scratch, or
>     working dir if running outside of jtreg ).
>
>     Do you think you could use similar to create the jars for your test?
>
>         createJar("a.jar", jarAList);
>         createJar("b.jar", jarBList);
>         .......
>
>         static void createJar(String jarName, Map<String,String> contents)
>             throws Exception
>         {
>             try (FileOutputStream aJar = new FileOutputStream(jarName);
>                  JarOutputStream jos = new JarOutputStream(aJar)) {
>                 Set<Entry<String,String>> entries = contents.entrySet();
>                 for (Entry<String,String> entry : entries)
>                     writeJarEntry(jos, entry.getKey(),
>                                   entry.getValue().getBytes("__ASCII"));
>             }
>         }
>
>         static void writeJarEntry(JarOutputStream jos, String name,
>     byte[] data)
>             throws Exception
>         {
>             JarEntry entry = new JarEntry(name);
>             jos.putNextEntry(entry);
>             jos.write(data);
>         }
>
>         static final Map<String,String> jarAList = new HashMap<>();
>         static final Map<String,String> jarBList = new HashMap<>();
>         static {
>             jarAList.put("com/foo/__resource1.txt", "some random data");
>             jarAList.put("com/bar/__resource2.txt", "some more random
>     data!");
>             jarAList.put("com/baz/__resource3.txt", "even more random
>     data!!!");
>             jarBList.put("x/y/resourceA.__dat", "Hello there");
>             jarBList.put("x/y/resourceB.__dat", "Goodbye");
>             jarBList.put("x/y/resourceC.__dat", "Hello\nfrom\nb\ndot\njar");
>         }
>
>     Thanks,
>     -Chris.
>
>
>     On 14/06/2012 03:20, Diego Belfer wrote:
>
>         Hi Chris,
>
>         There is no way to generate a jar without directory entries
>         using the
>         jar tool; there is no option for that. What do you think is the
>         best way
>         to handle this ?
>         I don't want to re-implement the logic for creating a jar using the
>         JarOutputStream class.
>
>         Do you think it is possible to add a new option to the Jar tool Main
>         class to exclude directory entries? The option does not need to be
>         exposed by the command line tool to final users if this an issue,
>         although I think it may be useful for them too.
>
>         Best,
>         Diego
>
>
>         On Wed, Jun 13, 2012 at 7:12 PM, Diego Belfer <dbelfer at gmail.com
>         <mailto:dbelfer at gmail.com>
>         <mailto:dbelfer at gmail.com <mailto:dbelfer at gmail.com>>> wrote:
>
>             Chris,
>
>             I was thinking of something similar. I will create the jars and
>             their contents using Java code. There is no need of creating
>         real
>             class files, using a few resource files and some directories
>         will be
>             enough.
>
>             Best,
>             Diego
>
>
>             On Wed, Jun 13, 2012 at 6:46 PM, chris hegarty
>         <chris.hegarty at oracle.com <mailto:chris.hegarty at oracle.com>
>         <mailto:chris.hegarty at oracle.__com
>         <mailto:chris.hegarty at oracle.com>>> wrote:
>
>                 Diego,
>
>                 I'm thinking that some of the trivial source files, to
>         compile
>                 and built into the jars, could be simply created and
>         written by
>                 the test itself, rather than checking them all in. If
>         this makes
>                 it cleaner. I really don't like all the file in
>                 test/sun/misc/JarIndex/____metaInfFilenames, but at
>         least it is
>
>                 quite understandable.
>
>                 -Chris.
>
>
>                 On 13/06/2012 20:36, Diego Belfer wrote:
>
>                     Hi Chris,
>
>                     That's right. The only non-cleanup change is the one
>         in the
>                     merge.
>
>                     Regarding the test case,  I will re-write them in
>         order to
>                     generate the
>                     jars on fly. I'd scanned the jdk/test folder and
>         found a few
>                     jars,
>                     that's why I included them.  I have seen your test
>         case, I
>                     will use it
>                     as a sample.
>
>                     I had not seen your comment in the bug report. Maybe
>         there
>                     are other
>                     cases which trigger the InvalidJarIndexException,
>         but, as
>                     far as I could
>                     see, the validIndex method checks that at least one
>         entry of
>                     the jar
>                     matches the target path found in the index. If directory
>                     entries are not
>                     present in the jar, stripped paths generated during the
>                     merge and used
>                     by the index will return jars which may not contain
>         entries
>                     for them,
>                     triggering the exception.
>                     When all directory entries are present, if a jar
>         contains an
>                     entry for
>         "xxx/yyy/resource.file", it  will contain entries for "xxx",
>         "xxx/yyy"
>                     and "xxx/yyy/resource.file".
>
>
>                     Best,
>                     Diego
>
>
>                     On Wed, Jun 13, 2012 at 12:05 PM, Chris Hegarty
>         <chris.hegarty at oracle.com <mailto:chris.hegarty at oracle.com>
>         <mailto:chris.hegarty at oracle.__com
>         <mailto:chris.hegarty at oracle.com>>
>         <mailto:chris.hegarty at oracle. <mailto:chris.hegarty at oracle.>____com
>
>         <mailto:chris.hegarty at oracle.__com
>         <mailto:chris.hegarty at oracle.com>>>> wrote:
>
>                         Hi Diego,
>
>                         Thanks for picking up this bug.
>
>                         I think your changes look fine. Mainly cleanup
>         except
>                     for add ->
>                         addExplicit/addMapping in merge, right? BTW the
>         cleanup
>                     makes this
>                         more readable.
>
>                         Unfortunately, the tests you created require
>         checking in
>                     a binary
>                         jar file. This is a real no no for the OpenJDK, we
>                     really need to
>                         create these jars on the fly. I did similar for
>                         test/sun/misc/JarIndex/______metaInfFilenames/,
>         but I
>
>                     really wish I
>
>                         generated the source files for these tests
>         rather than
>                     checking in
>                         so many pointless files.
>
>                         I can look at helping with writing suitable
>         tests for this.
>
>
>          > That's because I was using jars containing "directory
>                     entries"
>          > (I was unaware that jar files may not include them)
>
>                         Strangely I added the comment "Remove
>         directories from
>                     jar files
>                         being indexed." to the workaround section of the
>         bug.
>                     You seem to be
>                         seeing the opposite, right?
>
>                         -Chris.
>
>
>
>                         On 13/06/2012 06:11, Diego Belfer wrote:
>
>                             Hi,
>
>                             I have finally reproduced the
>                     InvalidJarIndexException bug as
>                             reported in
>                             the ticket. I mentioned in a previous email,
>         that
>                     the only way
>                             I'd found
>                             for getting the error was to use an invalid
>         index file
>                             (INDEX.LIST), which
>                             did not have any sense. That's because I was
>         using
>                     jars containing
>         "directory entries" (I was unaware that jar files may not
>                             include them)
>
>                             After reviewing the URLClasspath$JarLoader
>         class and the
>                             validIndex method,
>                             I notice it is possible to get the exception
>         for a
>                     Jar file
>                             which does not
>                             include directory entries. In order to
>         trigger the
>                     issue, the
>                             Jar must be
>                             referenced by an intermediary INDEX.LIST and the
>                     intermediary
>                             Jar index
>                             should have been merged to its parent index.
>                     Although, jar tool
>                             includes
>                             directory entries in the generated jar files,
>                     Eclipse default
>                             option for
>                             exporting jars does not include them (AFAIK), so
>                     this might be
>                             quite common.
>
>                             I have created a new PATCH which includes an
>                     additional test
>                             case which
>                             uses the URLClassLoader to trigger the
>                     InvalidIndexException.
>
>                             The patch is attached, please consider it
>         for review.
>
>                             Best,
>                             Diego Belfer [muralx]
>
>
>
>                             On Mon, Jun 11, 2012 at 4:47 PM, Diego
>                     Belfer<dbelfer at gmail.com <mailto:dbelfer at gmail.com>
>         <mailto:dbelfer at gmail.com <mailto:dbelfer at gmail.com>>
>         <mailto:dbelfer at gmail.com <mailto:dbelfer at gmail.com>
>         <mailto:dbelfer at gmail.com <mailto:dbelfer at gmail.com>>>>  wrote:
>
>
>
>                                 Hi,
>
>                                 Here is a patch that fixes the merge
>         method of
>                     the JarIndex.
>                                 This bug was
>                                 reported as the cause of the bug 6901992.
>                     Although, I was
>                                 not able to
>                                 reproduce the BUG itself
>                     (InvalidJarIndexException), I did
>                                 verified that
>                                 the method had a bug, and resources/classes
>                     where not found
>                                 in a jarIndex
>                                 with merged contents.
>
>                                 If you think it is possible to commit
>         this fix
>                     without actually
>                                 reproducing the original bug report, please
>                     consider this
>                                 patch for review.
>
>                                 Thanks,
>                                 Diego Belfer [muralx]
>
>
>
>
>
>



More information about the core-libs-dev mailing list