On Fri, 9 Jun 2023 13:54:14 GMT, Sean Coffey <coff...@openjdk.org> wrote:
> New functionality in the -XshowSettings menu to display relevant information
> about JDK security configuration
src/java.base/share/classes/sun/launcher/LauncherHelper.java line 65:
> 63: import java.text.MessageFormat;
> 64: import java.text.Normalizer;
> 65: import java.util.*;
Wild card imports are discouraged. (Your IDE may have done this behind your
back)
src/java.base/share/classes/sun/launcher/LauncherHelper.java line 328:
> 326:
> 327: private static void printSecuritySettings(String arg) {
> 328: if (arg.toLowerCase(Locale.ROOT).equals("properties")) {
How about a string switch with "all" and default: being the catchall. It would
be easier to add others and avoid multiple calls to `toLowerCase`.
It might be a good idea for unrecognized settings to print a 1-liner reminder
of the valid values, including `all`
src/java.base/share/classes/sun/launcher/LauncherHelper.java line 354:
> 352: }
> 353: }
> 354: ostream.print("\n");
or just `ostream.println();`
src/java.base/share/classes/sun/launcher/LauncherHelper.java line 363:
> 361:
> SSLContext.getDefault().getSocketFactory().createSocket();
> 362: } catch (IOException | NoSuchAlgorithmException e) {
> 363: throw new RuntimeException(e);
That's pretty rude of the runtime; perhaps `throw InternalError("Unable to
create secure socket")`.
It would give some clue that doesn't look like a coding bug.
src/java.base/share/classes/sun/launcher/LauncherHelper.java line 376:
> 374: System.out.println(THREEINDENT + s);
> 375: }
> 376: ostream.print("\n");
`ostream.println();`
src/java.base/share/classes/sun/launcher/LauncherHelper.java line 393:
> 391: }
> 392:
> 393: // return a string split across multiple lines which aims to limit
> max width
This utility method might be usable in more places if the indentation was an
argument instead of hard coded.
For example, in `printSecurityProperties`. It might need to be able to split
in more places.
src/java.base/share/classes/sun/launcher/resources/launcher.properties line 171:
> 169: \ -XshowSettings:vm\n\
> 170: \ show all vm related settings and continue\n\
> 171: \ -XshowSettings:security\n\
`all` should probably be documented.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/14394#discussion_r1224440655
PR Review Comment: https://git.openjdk.org/jdk/pull/14394#discussion_r1224444089
PR Review Comment: https://git.openjdk.org/jdk/pull/14394#discussion_r1224451282
PR Review Comment: https://git.openjdk.org/jdk/pull/14394#discussion_r1224456125
PR Review Comment: https://git.openjdk.org/jdk/pull/14394#discussion_r1224457654
PR Review Comment: https://git.openjdk.org/jdk/pull/14394#discussion_r1224460749
PR Review Comment: https://git.openjdk.org/jdk/pull/14394#discussion_r1224467126
