Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

8264208: Console charset API #3419

Closed
wants to merge 14 commits into from
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2005, 2018, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 2005, 2021, Oracle and/or its affiliates. All rights reserved.
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
*
* This code is free software; you can redistribute it and/or modify it
@@ -31,6 +31,7 @@
import jdk.internal.access.SharedSecrets;
import sun.nio.cs.StreamDecoder;
import sun.nio.cs.StreamEncoder;
import sun.security.action.GetPropertyAction;

/**
* Methods to access the character-based console device, if any, associated
@@ -390,13 +391,31 @@ public void flush() {
pw.flush();
}


/**
* Returns the {@link java.nio.charset.Charset Charset} object used for
* the {@code Console}.

This comment has been minimized.

@AlanBateman

AlanBateman Apr 15, 2021
Contributor

What would you think about re-phrasing the first sentence to use "for the Console" rather than "in the Console".

This comment has been minimized.

@naotoj

naotoj Apr 15, 2021
Author Member

Changed to "for the Console", as well as @return.

* <p>
* The returned charset corresponds to the input and output source
* (e.g., keyboard and/or display) specified by the host environment or user.
* It may not necessarily be the same as the default charset returned from
* {@link java.nio.charset.Charset#defaultCharset() Charset.defaultCharset()}.
*
* @return a {@link java.nio.charset.Charset Charset} object used for the
* {@code Console}
* @since 17

This comment has been minimized.

@JoeWang-Java

JoeWang-Java Apr 9, 2021
Member

A couple of minor comments:
May replace {@code Charset} with @link;
Add "the" to the sentence: The returned charset corresponds to "the" input...
@return: javadoc guide suggests "do not capitalize and do not end with a period" when writing a phrase. But I guess for consistency in this class, it's okay to capitalize.

This comment has been minimized.

@naotoj

naotoj Apr 9, 2021
Author Member

Thanks, Joe. Modified as suggested.

This comment has been minimized.

@JoeWang-Java

JoeWang-Java Apr 9, 2021
Member

Thanks. Looks good to me.

*/
public Charset charset() {
assert CHARSET != null : "charset() should not return null";
return CHARSET;
}

private Object readLock;
private Object writeLock;
private Reader reader;
private Writer out;
private PrintWriter pw;
private Formatter formatter;
private Charset cs;
private char[] rcb;
private boolean restoreEcho;
private boolean shutdownHookInstalled;
@@ -551,8 +570,21 @@ public int read(char cbuf[], int offset, int length)
}
}

// Set up JavaIOAccess in SharedSecrets
private static final Charset CHARSET;
static {
String csname = encoding();
Charset cs = null;
if (csname == null) {
csname = GetPropertyAction.privilegedGetProperty("sun.stdout.encoding");
}
if (csname != null) {
try {
cs = Charset.forName(csname);
} catch (Exception ignored) { }
}
CHARSET = cs == null ? Charset.defaultCharset() : cs;

// Set up JavaIOAccess in SharedSecrets
SharedSecrets.setJavaIOAccess(new JavaIOAccess() {
public Console console() {
if (istty()) {
@@ -564,9 +596,7 @@ public Console console() {
}

public Charset charset() {
// This method is called in sun.security.util.Password,
// cons already exists when this method is called
return cons.cs;
return CHARSET;
}
});

This comment has been minimized.

@RogerRiggs

RogerRiggs Apr 22, 2021
Contributor

Please keep the charset() method and return CHARSET.

I'm looking at a use case that needs to know the platform charset regardless of whether the console exists.
When a process is launched it may be redirected to /dev/tty or a pseudo tty and in that case
a Reader from that stream should be able to use the encoding of the platform.
Its still a work in progress, but it would save some refactoring or duplication later.

This comment has been minimized.

@naotoj

naotoj Apr 22, 2021
Author Member

Would the singleton Console.cons be instantiated in your use case? It is created only when isatty() (or Windows' equivalent) in the native code returns true.

This comment has been minimized.

@RogerRiggs

RogerRiggs Apr 22, 2021
Contributor

Not always, for example, if stderr was redirected to a terminal but not stdin and stdout.
The istty check is only true if both stdin and stdout are ttys.

This comment has been minimized.

@naotoj

naotoj Apr 22, 2021
Author Member

Then charset() in the shared secret would return null. Would that suffice your case?

This comment has been minimized.

@RogerRiggs

RogerRiggs Apr 22, 2021
Contributor

I read lines 575-587 as initializing CHARSET regardless of whether the Console was created.

This comment has been minimized.

@naotoj

naotoj Apr 22, 2021
Author Member

OK, revived the charset() method.

}
@@ -575,24 +605,16 @@ public Charset charset() {
private Console() {
readLock = new Object();
writeLock = new Object();
String csname = encoding();
if (csname != null) {
try {
cs = Charset.forName(csname);
} catch (Exception x) {}
}
if (cs == null)
cs = Charset.defaultCharset();
out = StreamEncoder.forOutputStreamWriter(
new FileOutputStream(FileDescriptor.out),
writeLock,
cs);
CHARSET);
pw = new PrintWriter(out, true) { public void close() {} };
formatter = new Formatter(out);
reader = new LineReader(StreamDecoder.forInputStreamReader(
new FileInputStream(FileDescriptor.in),
readLock,
cs));
CHARSET));
rcb = new char[1024];
}
}
@@ -35,7 +35,7 @@
* reads bytes and decodes them into characters using a specified {@link
* java.nio.charset.Charset charset}. The charset that it uses
* may be specified by name or may be given explicitly, or the platform's
* default charset may be accepted.
* {@link Charset#defaultCharset() default charset} may be accepted.
*
* <p> Each invocation of one of an InputStreamReader's read() methods may
* cause one or more bytes to be read from the underlying byte-input stream.
@@ -48,7 +48,7 @@
*

This comment has been minimized.

@RogerRiggs

RogerRiggs Apr 16, 2021
Contributor

Oddly, none of the reference in this class to the default charset are links to Charset.defaultCharset().
That would be a useful addition, either in the class javadoc or in the 1-arg constructor that uses the default charset.

This comment has been minimized.

@naotoj

naotoj Apr 16, 2021
Author Member

Thanks, Roger. Both are good suggestions. Will incorporate them into the next iteration.

* <pre>
* BufferedReader in
* = new BufferedReader(new InputStreamReader(System.in));
* = new BufferedReader(new InputStreamReader(anInputStream));
* </pre>
*
* @see BufferedReader
@@ -64,9 +64,12 @@
private final StreamDecoder sd;

/**
* Creates an InputStreamReader that uses the default charset.
* Creates an InputStreamReader that uses the
* {@link Charset#defaultCharset() default charset}.
*
* @param in An InputStream
*
* @see Charset#defaultCharset()
*/
public InputStreamReader(InputStream in) {
super(in);
@@ -114,15 +114,24 @@ private System() {
* The "standard" input stream. This stream is already
* open and ready to supply input data. Typically this stream
* corresponds to keyboard input or another input source specified by
* the host environment or user.
* the host environment or user. In case this stream is wrapped
* in a {@link java.io.InputStreamReader}, {@link Console#charset()}
* should be used for the charset, or consider using
* {@link Console#reader()}.
*
* @see Console#charset()
* @see Console#reader()

This comment has been minimized.

@AlanBateman

AlanBateman Apr 15, 2021
Contributor

What would you think about changing the example in InputStreamReader class description as part of this?

This comment has been minimized.

@naotoj

naotoj Apr 15, 2021
Author Member

Replaced System.in with generic anInputStream, as changing new InputStreamReader with Console.reader() would defy the purpose of the example.

*/
public static final InputStream in = null;

/**
* The "standard" output stream. This stream is already
* open and ready to accept output data. Typically this stream
* corresponds to display output or another output destination
* specified by the host environment or user.
* specified by the host environment or user. The encoding used
* in the conversion from characters to bytes is equivalent to
* {@link Console#charset()} if the {@code Console} exists,
* {@link Charset#defaultCharset()} otherwise.
* <p>
* For simple stand-alone Java applications, a typical way to write
* a line of output data is:
@@ -142,6 +151,8 @@ private System() {
* @see java.io.PrintStream#println(long)
* @see java.io.PrintStream#println(java.lang.Object)
* @see java.io.PrintStream#println(java.lang.String)
* @see Console#charset()
* @see Charset#defaultCharset()
*/
public static final PrintStream out = null;

@@ -156,6 +167,12 @@ private System() {
* of a user even if the principal output stream, the value of the
* variable {@code out}, has been redirected to a file or other
* destination that is typically not continuously monitored.
* The encoding used in the conversion from characters to bytes is
* equivalent to {@link Console#charset()} if the {@code Console}
* exists, {@link Charset#defaultCharset()} otherwise.
*
* @see Console#charset()
* @see Charset#defaultCharset()
*/
public static final PrintStream err = null;

This comment has been minimized.

@AlanBateman

AlanBateman Apr 14, 2021
Contributor

I still think we need something in System.in to link to Console::charset. If someone creates an InputStreamReader(System.in) then it will use the default charset, they should be using Console::reader for these cases.

This comment has been minimized.

@naotoj

naotoj Apr 14, 2021
Author Member

Added some explanation to it.


@@ -2014,6 +2031,9 @@ private static void initPhase1() {
FileOutputStream fdOut = new FileOutputStream(FileDescriptor.out);
FileOutputStream fdErr = new FileOutputStream(FileDescriptor.err);
setIn0(new BufferedInputStream(fdIn));
// sun.stdout/err.encoding are set when the VM is associated with the terminal,
// thus they are equivalent to Console.charset(), otherwise the encoding
// defaults to Charset.defaultCharset()
setOut0(newPrintStream(fdOut, props.getProperty("sun.stdout.encoding")));
setErr0(newPrintStream(fdErr, props.getProperty("sun.stderr.encoding")));

@@ -1,5 +1,5 @@
/*
* Copyright (c) 2005, 2018, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 2005, 2021, Oracle and/or its affiliates. All rights reserved.
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
*
* This code is free software; you can redistribute it and/or modify it
@@ -29,6 +29,6 @@
import java.nio.charset.Charset;

public interface JavaIOAccess {
public Console console();
public Charset charset();
Console console();
Charset charset();
}
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2003, 2018, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 2003, 2021, Oracle and/or its affiliates. All rights reserved.
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
*
* This code is free software; you can redistribute it and/or modify it
@@ -29,7 +29,6 @@
import java.nio.*;
import java.nio.charset.*;
import java.util.Arrays;
import jdk.internal.access.SharedSecrets;

/**
* A utility class for reading passwords
@@ -140,7 +139,7 @@
private static byte[] convertToBytes(char[] pass) {
if (enc == null) {
synchronized (Password.class) {
enc = SharedSecrets.getJavaIOAccess()
enc = System.console()
.charset()
.newEncoder()
.onMalformedInput(CodingErrorAction.REPLACE)
@@ -1,5 +1,5 @@
/*
* Copyright (c) 1998, 2020, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 1998, 2021, Oracle and/or its affiliates. All rights reserved.
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
*
* This code is free software; you can redistribute it and/or modify it
@@ -454,6 +454,12 @@ GetJavaProperties(JNIEnv *env)
#else
sprops.sun_jnu_encoding = sprops.encoding;
#endif
if (isatty(STDOUT_FILENO) == 1) {
sprops.sun_stdout_encoding = sprops.encoding;
}
if (isatty(STDERR_FILENO) == 1) {
sprops.sun_stderr_encoding = sprops.encoding;
}

#ifdef _ALLBSD_SOURCE
#if BYTE_ORDER == _LITTLE_ENDIAN
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2005, 2013, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 2005, 2021, Oracle and/or its affiliates. All rights reserved.
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
*
* This code is free software; you can redistribute it and/or modify it
@@ -57,6 +57,8 @@ Java_java_io_Console_encoding(JNIEnv *env, jclass cls)
int cp = GetConsoleCP();
if (cp >= 874 && cp <= 950)
sprintf(buf, "ms%d", cp);
else if (cp == 65001)
sprintf(buf, "UTF-8");
else
sprintf(buf, "cp%d", cp);
return JNU_NewStringPlatform(env, buf);
@@ -1,5 +1,5 @@
/*
* Copyright (c) 1998, 2020, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 1998, 2021, Oracle and/or its affiliates. All rights reserved.
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
*
* This code is free software; you can redistribute it and/or modify it
@@ -80,6 +80,7 @@ getEncodingInternal(LCID lcid)

switch (codepage) {
case 0:
case 65001:
strcpy(ret, "UTF-8");
break;
case 874: /* 9:Thai */
@@ -0,0 +1,77 @@
/*
* Copyright (c) 2021, Oracle and/or its affiliates. All rights reserved.
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
*
* This code is free software; you can redistribute it and/or modify it
* under the terms of the GNU General Public License version 2 only, as
* published by the Free Software Foundation.
*
* This code is distributed in the hope that it will be useful, but WITHOUT
* ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
* FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License
* version 2 for more details (a copy is included in the LICENSE file that
* accompanied this code).
*
* You should have received a copy of the GNU General Public License version
* 2 along with this work; if not, write to the Free Software Foundation,
* Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
*
* Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
* or visit www.oracle.com if you need additional information or have any
* questions.
*/

import java.io.Console;
import java.nio.file.Files;
import java.nio.file.Paths;

import jdk.test.lib.process.OutputAnalyzer;
import jdk.test.lib.process.ProcessTools;

/**
* @test
* @bug 8264208
* @summary Tests Console.charset() method. "expect" command in Windows/Cygwin
* does not work as expected. Ignoring tests on Windows.
* @requires (os.family == "linux") | (os.family == "mac")
* @library /test/lib
* @run main CharsetTest en_US.ISO8859-1 ISO-8859-1
* @run main CharsetTest en_US.US-ASCII US-ASCII
* @run main CharsetTest en_US.UTF-8 UTF-8
* @run main CharsetTest en_US.FOO ignored
*/
public class CharsetTest {
public static void main(String... args) throws Throwable {
if (args.length == 0) {
// no arg means child java process being tested.
Console con = System.console();
System.out.println(con.charset());
return;
} else {
// check "expect" command availability
var expect = Paths.get("/usr/bin/expect");
if (!Files.exists(expect) || !Files.isExecutable(expect)) {
System.out.println("'expect' command not found. Test ignored.");
return;
}

// invoking "expect" command
var testSrc = System.getProperty("test.src", ".");
var testClasses = System.getProperty("test.classes", ".");
var jdkDir = System.getProperty("test.jdk");
OutputAnalyzer output = ProcessTools.executeProcess(
"expect",
"-n",
testSrc + "/script.exp",
jdkDir + "/bin/java",
args[0],
args[1],
testClasses);
output.reportDiagnosticSummary();
var eval = output.getExitValue();
if (eval != 0) {
throw new RuntimeException("Test failed. Exit value from 'expect' command: " + eval);
}
}
}
}
ProTip! Use n and p to navigate between commits in a pull request.