RFR - 8132734: java.util.jar.* changes to support multi-release jar files
xueming.shen at oracle.com
Fri Nov 6 21:50:31 UTC 2015
My apology for slow response. I was sick in bed since last Sat...
I've scanned the update, here are my comments
(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.
(2) Now the BASE_VERSION has been raised to 8, the logic in
setVersioned(n) will actually block
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.
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.
>> 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.
>>> On 5 Nov 2015, at 18:10, Steve Drach <steve.drach at oracle.com> wrote:
>>> 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/>
More information about the core-libs-dev