RFR: 8356985: Use "stdin.encoding" in Console's read*() methods [v2]
Justin Lu
jlu at openjdk.org
Wed May 21 06:28:54 UTC 2025
On Tue, 20 May 2025 21:57:45 GMT, Naoto Sato <naoto at openjdk.org> wrote:
>> `java.io.Console` uses the charset specified by the `stdout.encoding` system property for both input and output. While this is generally sufficient, since Console is intended for interactive terminal use, some platforms allow different encodings to be configured for input and output. In such cases, using a single encoding may lead to incorrect behavior when reading from the terminal. To address this, the newly introduced system property, `stdin.encoding`, should be used specifically for input where appropriate.
>
> Naoto Sato has updated the pull request incrementally with one additional commit since the last revision:
>
> Reflects review comments
src/java.base/share/classes/java/io/Console.java line 67:
> 65: * stdout.encoding}, in which case read operations use the {@code Charset}
> 66: * designated by {@code stdin.encoding}.
> 67: * <p>
`Console.charset()` states "The returned charset is used for interpreting the input and output source (e.g., keyboard and/or display) specified by the host environment or user, which defaults to the one based on stdout.encoding." If _stdin.encoding_ is set otherwise, this is no longer true, so I think this method may need a wording update as well.
test/jdk/java/io/Console/StdinEncodingTest.java line 38:
> 36: * @test
> 37: * @bug 8356985
> 38: * @summary Tests if "stdin.encoding" is reflected for reading
Would be helpful to include why the test is limited to only linux and mac,
> "expect" command in Windows/Cygwin does not work as expected. Ignoring tests on Windows.
test/jdk/java/io/Console/StdinEncodingTest.java line 51:
> 49: // check "expect" command availability
> 50: var expect = Paths.get("/usr/bin/expect");
> 51: if (!Files.exists(expect) || !Files.isExecutable(expect)) {
Could use `Assumptions.assumeTrue` here. Condition becomes more readable as: `Files.exists(expect) && Files.isExecutable(expect)`
test/jdk/java/io/Console/StdinEncodingTest.java line 56:
> 54:
> 55: // invoking "expect" command
> 56: var testSrc = System.getProperty("test.src", ".");
The test already imports the JDK test lib, can we just replace this and the below ocurrences with `jdk.test.lib.Utils.TEST_SRC/JDK/CLASSES` directly?
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/25271#discussion_r2099438513
PR Review Comment: https://git.openjdk.org/jdk/pull/25271#discussion_r2099441523
PR Review Comment: https://git.openjdk.org/jdk/pull/25271#discussion_r2099447047
PR Review Comment: https://git.openjdk.org/jdk/pull/25271#discussion_r2099443105
More information about the core-libs-dev
mailing list