PATCH [6901992] : Possible InvalidJarIndexException due to bug in sun.misc.JarIndex.merge()
Diego Belfer
dbelfer at gmail.com
Tue Jun 19 21:20:06 UTC 2012
Thanks for sponsoring the patch..
Best,
Diego
On Tue, Jun 19, 2012 at 7:15 AM, Chris Hegarty <chris.hegarty at oracle.com>wrote:
> To close the loop on this, here is a link to the changeset:
>
> http://hg.openjdk.java.net/**jdk8/tl/jdk/rev/efc2791d7c5d<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 <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<chris.hegarty at oracle.com>
>> >
>> <mailto:chris.hegarty at oracle._**_com
>> <mailto:chris.hegarty at oracle.**com <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<chris.hegarty at oracle.com>
>> >
>> <mailto:chris.hegarty at oracle._**_com
>> <mailto:chris.hegarty at oracle.**com <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 <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