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