Hi Steve,
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.
-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.san...@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.dr...@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
