[8u-dev] RFA (S): JDK-8210985: Update the default SSL session cache size to 20480

[Hohensee, Paul]

Thanks for the review, Andrew.

New webrev: http://cr.openjdk.java.net/~phh/8210985/webrev.8u.01/

I commented on the 8u CSR https://bugs.openjdk.java.net/browse/JDK-8224770,
asking who the 8u CSR approver(s) would be if the 8u maintainer(s) wanted to use the CSR process.

For the time being, there's no 8u CSR process, so imo you can just approve the backport. :)

I took a look at JDK-8154231. It doesn't apply cleanly, and looks like it needs a previous backport to do so. I didn't investigate further, but imo if we want to do so it should wait for 8u232.

On 5/27/19, 7:09 AM, "Andrew John Hughes" <gnu.andrew at redhat.com> wrote:

    On 24/05/2019 22:55, Hohensee, Paul wrote:
    > CSRs are definitely an Oracle thing. See https://wiki.openjdk.java.net/display/csr/Main.
    > 
    > At Oracle, if you want to change an interface or behavior, you file a JBS issue to do the work, then file a CSR to get approval for the interface change. Closing a CSR == approving it. Finalized state means it's ready for review. One is supposed to get consensus before finalizing a CSR.
    > 
    > If a backport involved a CSR, Oracle used to want a backport CSR, so the process was and maybe is: create a backport JBS issue, then a CSR for the backport issue, get the CSR approved, then get the backport approved. I don't know what Oracle's CSR backport process is now, but somehow I was under the impression that I should follow it. :) We could use Oracle's process for 8u and 11u, though it's a bit heavy-weight. It does have the advantage of thoroughly documenting interface/behavior changes in backports. I'm not sure who would do the backport CSR approvals: probably the Maintainers.
    
    I don't have a strong opinion either way, as long as I know what boxes I
    need to tick :-)
    
    > 
    > I was incorrect/hasty about 8u clean patch application. 8u doesn't have SSLLogger, so that code gets dropped from SSLSessionContextImpl.getDefaultCacheLimit().
    > 
    > 8u webrev: http://cr.openjdk.java.net/~phh/8210985/webrev.8u.00/
    
    There's an issue here with the changes to SSLSessionContextImpl.java
    where the doPrivileged is being lost. In the original version [0],
    it is changed to GetIntegerAction.privilegedGetProperty which is
    introduced by JDK-8154231 [1] & JDK-8155775 [2], which I think may be
    worth backporting themselves:
    
        public static Integer privilegedGetProperty(String theProp,
                int defaultVal) {
            Integer value;
            if (System.getSecurityManager() == null) {
                value = Integer.getInteger(theProp);
            } else {
                value = AccessController.doPrivileged(
                        new GetIntegerAction(theProp));
            }
            return (value != null) ? value : defaultVal;
        }
    
    In the 8u backport, the GetIntegerAction is created locally and not run
    inside a doPrivileged call:
    
    +            int defaultCacheLimit =
    +                new GetIntegerAction("javax.net.ssl.sessionCacheSize",
    +
    DEFAULT_MAX_CACHE_SIZE).run().intValue();
    
    which is incorrect. I suggest something along the lines of:
    
    int defaultCacheLimit = java.security.AccessController.doPrivileged(new
    GetIntegerAction("javax.net.ssl.sessionCacheSize",
    DEFAULT_MAX_CACHE_SIZE)).intValue();
    
    [0] https://hg.openjdk.java.net/jdk/jdk/rev/8a85d21d9616
    [1] https://bugs.openjdk.java.net/browse/JDK-8154231
    [2] https://bugs.openjdk.java.net/browse/JDK-8155775
    -- 
    Andrew :)
    
    Senior Free Java Software Engineer
    Red Hat, Inc. (http://www.redhat.com)
    
    PGP Key: ed25519/0xCFDA0F9B35964222 (hkp://keys.gnupg.net)
    Fingerprint = 5132 579D D154 0ED2 3E04  C5A0 CFDA 0F9B 3596 4222
    https://keybase.io/gnu_andrew