Re: RFR: 8281658: Add a security category to the java -XshowSettings option [v5]

2023-06-19 Thread Sean Coffey
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]

2023-06-19 Thread Roger Riggs
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]

2023-06-19 Thread Sean Coffey
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]

2023-06-19 Thread Sean Coffey
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]

2023-06-16 Thread Weijun Wang
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]

2023-06-16 Thread Roger Riggs
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]

2023-06-16 Thread Sean Coffey
> 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