RFR: 8044609: javax.net.debug options not working and documented as expected [v8]

[Bradford Wetmore]

On Tue, 19 Nov 2024 12:38:48 GMT, Sean Coffey <coffeys at openjdk.org> wrote:

>> With the current definition in the JSSE docs, `sslexpandhandshakeverbose` is an acceptable value, as are the tokens in any order.  e.g. `sslverbosehandshakeexpand` or `ssl|moohaha|handshake2354432verbose@#%^%SeanWasHere,keygen`.  This code here is position dependent and won't allow it.  
>> 
>> This approach needs a rethink.  Maybe an `EnumSet` initialized here that contains the active, known `ssl` options, that is then checked during the `isOn(checkPoints)`?  Just a thought.
>
> `sslexpandhandshakeverbose` works in the new implementation. 
> 
> the `sslOn `boolean is a fast path helper. We fall back to matching per debug option if required.
> 
> I'm going to suggest that we shouldn't have to accept `ssl|moohaha|handshake2354432verbose@#%^%SeanWasHere,keygen` as a valid property value. The lax parsing currently used is somewhat to blame for javax.net.debug parsing being so badly broken since its introduction in the new TLSv1.3 stack in 2018.  i.e. passing the widely documented `ssl` is broken today. I'm not as concerned as some code that might be trying to use the `sslexpandhandshakeverbose` example!
> 
> can we document some reasonable separators ? We could go with an EnumSet design approach but I do think it's time we specify a valid separator if so.

IMHO, the JSSE docs are pretty clear (even tho functionally it's been broken for quite some time, sigh). 

> You do not have to have a separator between options, although a separator such as a colon (:) or a comma (,) helps readability. It does not matter what separators you use, and the ordering of the option keywords is also not important. 

No separators needed, no ordering enforced.

Could be something like this:

            EnumSet<Options> activeOptions = EnumSet.noneOf(Options.class);
            for (Options e : Options.values()) {
                if (property.contains(e.toString())) {
                    activeOptions.add(e);
                }
            }
        -----------
       public static boolean isOn(String checkPoints) {
            ...etc...
            String[] options = checkPoints.split(",");
            for (String option : options) {
                option = option.trim().toLowerCase(Locale.ROOT);
                if (!activeOptions.contains(option.valueOf(option)) {
                    return false;
                }
            }
            return true;
        }

-------------

PR Review Comment: https://git.openjdk.org/jdk/pull/18764#discussion_r1849045784