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

[Diego Belfer]

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