On Wed, 23 Apr 2025 13:39:00 GMT, Weijun Wang <wei...@openjdk.org> wrote:
>> Add more description on password handling into the keytool man page. A link
>> to the man page is now added to the keytool help screen.
>>
>> When keytool output is redirected into a file or file, a warning is shown:
>>
>> $ keytool -genkeypair | type
>>
>> Warning: password will be echoed because output is redirected.
>> Enter keystore password: password
>> Warning: password will be echoed because output is redirected.
>> Re-enter new password:
>>
>>
>> A new manual test is added.
>>
>> Sorry we cannot suppress password echoing in this case at the moment because
>> `System.console()` is not available.
>
> Weijun Wang has updated the pull request incrementally with one additional
> commit since the last revision:
>
> hide warning when password is piped into the command; enhance test
test/jdk/java/awt/regtesthelpers/PassFailJFrame.java line 630:
> 628: long testTimeOut,
> 629: int rows, int
> columns,
> 630: List<JButton>
> moreButtons,
Minor: wouldn't `additinalButtons` or `extraButtons` be a more intuitive name?
test/jdk/java/awt/regtesthelpers/PassFailJFrame.java line 1470:
> 1468: */
> 1469: public Builder addButton(JButton button) {
> 1470: if (moreButtons == null) moreButtons = new ArrayList<>();
Nitpick: I'd personally make this as:
if (moreButtons == null) {
moreButtons = new ArrayList<>();
}
just to be consistent with the other if statements in the file and package. But
it is perfectly fine as it is.
test/jdk/sun/security/tools/keytool/EchoPassword.java line 33:
> 31: */
> 32:
> 33: import javax.swing.*;
Nitpick: wildcard import
test/jdk/sun/security/tools/keytool/EchoPassword.java line 65:
> 63:
> 64: 2. Press "Copy First Command" to copy the first command
> into the system clipboard.
> 65: Paste it into the terminal window and execute the
> command.
I personally think, even with the button, it's better to show the full command
itself to the user, just in case whoever is running the test misses the command
or accidentally clear/overwrite the clipboard. What do you think ?
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/24805#discussion_r2056108063
PR Review Comment: https://git.openjdk.org/jdk/pull/24805#discussion_r2056139794
PR Review Comment: https://git.openjdk.org/jdk/pull/24805#discussion_r2056116916
PR Review Comment: https://git.openjdk.org/jdk/pull/24805#discussion_r2056112039
