RFR: 8356985: Use "stdin.encoding" in Console's read*() methods

[Volkan Yazici]

On Fri, 16 May 2025 18:11:39 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.

src/java.base/share/classes/java/io/Console.java line 560:

> 558:     static final Charset STDIN_CHARSET =
> 559:         Charset.forName(System.getProperty("stdin.encoding"), UTF_8.INSTANCE);
> 560:     static final Charset STDOUT_CHARSET =

I guess these can be marked `private`?

Suggestion:

    private static final Charset STDIN_CHARSET =
        Charset.forName(System.getProperty("stdin.encoding"), UTF_8.INSTANCE);
    private static final Charset STDOUT_CHARSET =

src/jdk.internal.le/share/classes/jdk/internal/org/jline/JdkConsoleProviderImpl.java line 160:

> 158: 
> 159:             try {
> 160:                 Terminal terminal = TerminalBuilder.builder().encoding(outCharset)

Shouldn't ideally `JdkConsole::charset` and `Terminal::encoding` be adapted for stdin/stdout variants?

test/jdk/java/io/Console/CharsetTest.java line 1:

> 1: /*

Copyright year update is missing.

test/jdk/java/io/Console/StdinEncodingTest.java line 46:

> 44:  * @run junit StdinEncodingTest
> 45:  */
> 46: public class StdinEncodingTest {

AFAICT, there is no similar test (e.g., one using a mock `CharsetProvider`) for `stdout.encoding`. Will it be addressed by another ticket? Shall we consider adding a similar `StdoutEncodingTest` too? (Not necessarily in this PR.)

test/jdk/java/io/Console/StdinEncodingTest.java line 49:

> 47: 
> 48:     @Test
> 49:     @EnabledOnOs({OS.LINUX, OS.MAC})

Shall we replace `@EnabledOnOs` with the `@requires (os.family == "linux" | os.family == "mac")` JTreg directive instead per [JDK-8211673](https://bugs.openjdk.org/browse/JDK-8211673)?

test/jdk/java/io/Console/csp/provider/MockCharsetProvider.java line 36:

> 34: 
> 35: // A test charset provider that decodes every input byte into its uppercase
> 36: public class MockCharsetProvider extends CharsetProvider {

*Nit:* Maybe a more self-explanatory name, e.g., `UpperCasingCharsetProvider`?

test/jdk/java/io/Console/csp/provider/MockCharsetProvider.java line 83:

> 81:             while (in.remaining() > 0) {
> 82:                 char c = (char)in.get();
> 83:                 if (c != '\n') {

`Character.toUpperCase('\n') == '\n'`, not? If so, do we still need this `if`-branching?

-------------

PR Review Comment: https://git.openjdk.org/jdk/pull/25271#discussion_r2097641573
PR Review Comment: https://git.openjdk.org/jdk/pull/25271#discussion_r2097630717
PR Review Comment: https://git.openjdk.org/jdk/pull/25271#discussion_r2097578092
PR Review Comment: https://git.openjdk.org/jdk/pull/25271#discussion_r2097718253
PR Review Comment: https://git.openjdk.org/jdk/pull/25271#discussion_r2097671831
PR Review Comment: https://git.openjdk.org/jdk/pull/25271#discussion_r2097711653
PR Review Comment: https://git.openjdk.org/jdk/pull/25271#discussion_r2097704883