Re: RFR: 8281658: Add a security category to the java -XshowSettings option [v5]
On Mon, 19 Jun 2023 18:21:49 GMT, Roger Riggs wrote: >> @RogerRiggs - do you mean to print nothing in the "bad command input" >> scenario ? The current -XshowSettings launch behaviour prints all data if a >> bad value is passed to it. I was mimicking this for security subcommands. >> >> Are you suggesting to print the relevant part of the "java -X" help menu >> here or is restoring the message with value subcommands sufficient ? > > Restoring the message with value subcommands is what I would prefer. > I do disagree with the original choice of printing everything if the command > is wrong and would not propagate that behavior to the new command. > Separately, I would consider changing the existing behavior but I suspect > that would fail to achieve consensus based on compatibility concerns. Fair point. Modified the code so that we don't print any security settings if a bad security subcommand value is specified. Info message instead. I'll update the CSR on that point also. Agreed on the compatibility concerns of changing the existing behaviour already present in this area. - PR Review Comment: https://git.openjdk.org/jdk/pull/14394#discussion_r1234416659
Re: RFR: 8281658: Add a security category to the java -XshowSettings option [v5]
On Mon, 19 Jun 2023 16:11:12 GMT, Sean Coffey wrote:
>> src/java.base/share/classes/sun/launcher/SecuritySettings.java line 66:
>>
>>> 64: ostream.println("Unrecognized security subcommand. See
>>> \"java -X\" for help");
>>> 65: ostream.println("Printing all security settings");
>>> 66: printAllSecurityConfig();
>>
>> The error message is going to get lost in the volume of settings.
>> Allowing bad command input reinforces learning the wrong suboption; though
>> it may duplicate the help, I'd print the allowed options.
>
> @RogerRiggs - do you mean to print nothing in the "bad command input"
> scenario ? The current -XshowSettings launch behaviour prints all data if a
> bad value is passed to it. I was mimicking this for security subcommands.
>
> Are you suggesting to print the relevant part of the "java -X" help menu here
> or is restoring the message with value subcommands sufficient ?
Restoring the message with value subcommands is what I would prefer.
I do disagree with the original choice of printing everything if the command is
wrong and would not propagate that behavior to the new command.
Separately, I would consider changing the existing behavior but I suspect that
would fail to achieve consensus based on compatibility concerns.
-
PR Review Comment: https://git.openjdk.org/jdk/pull/14394#discussion_r1234344945
Re: RFR: 8281658: Add a security category to the java -XshowSettings option [v5]
On Sat, 17 Jun 2023 01:51:36 GMT, Weijun Wang wrote:
>> Sean Coffey has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> Pass PrintStream to security helper
>
> src/java.base/share/classes/sun/launcher/SecuritySettings.java line 93:
>
>> 91: for (String key :
>> p.stringPropertyNames().stream().sorted().toList()) {
>> 92: String val = p.getProperty(key);
>> 93: if (val.contains(",") && val.length() > 60) {
>
> Do you want to do the same for `;`? `jceks.key.serialFilter` uses it.
fair point. done.
-
PR Review Comment: https://git.openjdk.org/jdk/pull/14394#discussion_r1234252148
Re: RFR: 8281658: Add a security category to the java -XshowSettings option [v5]
On Fri, 16 Jun 2023 15:15:59 GMT, Roger Riggs wrote:
>> Sean Coffey has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> Pass PrintStream to security helper
>
> src/java.base/share/classes/sun/launcher/SecuritySettings.java line 66:
>
>> 64: ostream.println("Unrecognized security subcommand. See
>> \"java -X\" for help");
>> 65: ostream.println("Printing all security settings");
>> 66: printAllSecurityConfig();
>
> The error message is going to get lost in the volume of settings.
> Allowing bad command input reinforces learning the wrong suboption; though it
> may duplicate the help, I'd print the allowed options.
@RogerRiggs - do you mean to print nothing in the "bad command input" scenario
? The current -XshowSettings launch behaviour prints all data if a bad value is
passed to it. I was mimicking this for security subcommands.
Are you suggesting to print the relevant part of the "java -X" help menu here
or is restoring the message with value subcommands sufficient ?
-
PR Review Comment: https://git.openjdk.org/jdk/pull/14394#discussion_r1234251237
Re: RFR: 8281658: Add a security category to the java -XshowSettings option [v5]
On Fri, 16 Jun 2023 12:14:49 GMT, Sean Coffey wrote:
>> New functionality in the -XshowSettings menu to display relevant information
>> about JDK security configuration
>
> Sean Coffey has updated the pull request incrementally with one additional
> commit since the last revision:
>
> Pass PrintStream to security helper
src/java.base/share/classes/sun/launcher/SecuritySettings.java line 93:
> 91: for (String key :
> p.stringPropertyNames().stream().sorted().toList()) {
> 92: String val = p.getProperty(key);
> 93: if (val.contains(",") && val.length() > 60) {
Do you want to do the same for `;`? `jceks.key.serialFilter` uses it.
-
PR Review Comment: https://git.openjdk.org/jdk/pull/14394#discussion_r1232926833
Re: RFR: 8281658: Add a security category to the java -XshowSettings option [v5]
On Fri, 16 Jun 2023 12:14:49 GMT, Sean Coffey wrote:
>> New functionality in the -XshowSettings menu to display relevant information
>> about JDK security configuration
>
> Sean Coffey has updated the pull request incrementally with one additional
> commit since the last revision:
>
> Pass PrintStream to security helper
src/java.base/share/classes/sun/launcher/SecuritySettings.java line 66:
> 64: ostream.println("Unrecognized security subcommand. See
> \"java -X\" for help");
> 65: ostream.println("Printing all security settings");
> 66: printAllSecurityConfig();
The error message is going to get lost in the volume of settings.
Allowing bad command input reinforces learning the wrong suboption; though it
may duplicate the help, I'd print the allowed options.
-
PR Review Comment: https://git.openjdk.org/jdk/pull/14394#discussion_r1232388914
Re: RFR: 8281658: Add a security category to the java -XshowSettings option [v5]
> New functionality in the -XshowSettings menu to display relevant information > about JDK security configuration Sean Coffey has updated the pull request incrementally with one additional commit since the last revision: Pass PrintStream to security helper - Changes: - all: https://git.openjdk.org/jdk/pull/14394/files - new: https://git.openjdk.org/jdk/pull/14394/files/020a1863..91dc41f3 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk=14394=04 - incr: https://webrevs.openjdk.org/?repo=jdk=14394=03-04 Stats: 14 lines in 2 files changed: 5 ins; 4 del; 5 mod Patch: https://git.openjdk.org/jdk/pull/14394.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/14394/head:pull/14394 PR: https://git.openjdk.org/jdk/pull/14394
