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

[Chris Hegarty]

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>  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]
>>
>>