On Tue, 19 Nov 2024 12:38:48 GMT, Sean Coffey <coff...@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
