RFR - 8132734: java.util.jar.* changes to support multi-release jar files

Steve Drach steve.drach at oracle.com
Sat Nov 7 00:13:41 UTC 2015


Hi Sherman,

Thanks for looking at this.

> (1) The newly added "configured" obviously will help in most use scenario, but (yes, there is
>     always a but :-)), given the related code is not synchronized, I'm pretty sure there is race
>     condition somewhere, for example betwen Ln#388 -- Ln#396, if there is a "faster" second
>     getEntry  catches up there, a base/root entry will return, but a versioned will return next
>     time if the isMultiRelease is set to true and there is a matched versioned entry for it.

It seems to me we could synchronize/lock the code after line 392 with little loss in performance, although the recursive nature of getManifest might cause a problem.  I need to think about it a bit.  I guess JarFile was thread safe since getEntry was idempotent
> 
> (2) Now the BASE_VERSION has been raised to 8, the logic in setVersioned(n) will actually bl
>     any version num < 8, as now it is forced to be the BASE_VERSION, any lookup for versioned
>     will stop at BASE_VERSION. Is it a specified behavior? adjust the BASE_VERSION to the version
>     being set 'n' might be a more reasonable approach?
> 
> (3) Again one more security concern you might want to confirm with vuln team (probably during their
>     review). With current implementation, in use scenario
>     a) it's multi-release jar
>     b) is multi-release jar enabled
>     c) all base/root entries are signed
>     d) the set of versioned entries are not signed.
> 
>     the implementation returns versioned entries successfully, they are unsigned. Is this a concern,
>     as arguably, the user is asking entry with the root name, and the corresponding entry for that
>     root name is signed. But we return a linked versioned-entry, and it's not signed. This might not
>     be  an issue at all. Please confirm this when doing security review.
> 
> -Sherman
> 
> On 11/6/15 9:23 AM, Steve Drach wrote:
>> Hi Sherman.
>> 
>> Would you please give this another pass?  I want make sure all your concerns are met.
>> 
>> Thanks
>> Steve
>> 
>>> On Nov 6, 2015, at 12:44 AM, Paul Sandoz <paul.sandoz at oracle.com> wrote:
>>> 
>>> Hi Steve,
>>> 
>>> This looks good to me (i only browsed the test code). It’s been around the block a few times :-) IMO, baring any major issues, it’s time to push and we can clean up any ancillary issues with later pushes.
>>> 
>>> Paul.
>>> 
>>>> On 5 Nov 2015, at 18:10, Steve Drach <steve.drach at oracle.com> wrote:
>>>> 
>>>> Hi,
>>>> 
>>>> Here’s a new webrev that addresses the issues Paul brought up.  The versioned entry cache has been removed, the search space has been reduced, the documentation for setVersioned(int) and setRuntimeVersioned() has been updated to clarify when IllegalStateException is thrown, and the tests have been changed to accommodate a jar file with versions 9 and 10, rather than 8 and 9.
>>>> 
>>>> Issue: https://bugs.openjdk.java.net/browse/JDK-8132734 <https://bugs.openjdk.java.net/browse/JDK-8132734>
>>>> JEP 238: https://bugs.openjdk.java.net/browse/JDK-8047305 <https://bugs.openjdk.java.net/browse/JDK-8047305>
>>>> Webrev: http://cr.openjdk.java.net/~psandoz/multiversion-jar/jar-webrev/ <http://cr.openjdk.java.net/~psandoz/multiversion-jar/jar-webrev/>
>>>> 
>>>> Steve
> 




More information about the core-libs-dev mailing list