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

Fix bad encoding when creating default PrintWriter #1321

Merged
merged 1 commit into from
Feb 4, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 19 additions & 8 deletions src/main/java/picocli/CommandLine.java
Original file line number Diff line number Diff line change
Expand Up @@ -1206,7 +1206,7 @@ public CommandLine setColorScheme(Help.ColorScheme colorScheme) {
* help with a {@code --help} or similar option, the usage help message is printed to the standard output stream so that it can be easily searched and paged.</p>
* @since 4.0 */
public PrintWriter getOut() {
if (out == null) { setOut(new PrintWriter(System.out, true)); }
if (out == null) { setOut(newPrintWriter(System.out, getStdoutEncoding())); }
return out;
}

Expand All @@ -1233,7 +1233,7 @@ public CommandLine setOut(PrintWriter out) {
* should use this writer to print error messages (which may include a usage help message) when an unexpected error occurs.</p>
* @since 4.0 */
public PrintWriter getErr() {
if (err == null) { setErr(new PrintWriter(System.err, true)); }
if (err == null) { setErr(newPrintWriter(System.err, getStderrEncoding())); }
return err;
}

Expand Down Expand Up @@ -1807,7 +1807,7 @@ protected R throwOrExit(ExecutionException ex) {
* @since 2.0 */
@Deprecated public static class DefaultExceptionHandler<R> extends AbstractHandler<R, DefaultExceptionHandler<R>> implements IExceptionHandler, IExceptionHandler2<R> {
public List<Object> handleException(ParameterException ex, PrintStream out, Help.Ansi ansi, String... args) {
internalHandleParseException(ex, new PrintWriter(out, true), Help.defaultColorScheme(ansi)); return Collections.emptyList(); }
internalHandleParseException(ex, newPrintWriter(out, getStdoutEncoding()), Help.defaultColorScheme(ansi)); return Collections.emptyList(); }

/** Prints the message of the specified exception, followed by the usage message for the command or subcommand
* whose input was invalid, to the stream returned by {@link #err()}.
Expand All @@ -1817,7 +1817,7 @@ public List<Object> handleException(ParameterException ex, PrintStream out, Help
* @return the empty list
* @since 3.0 */
public R handleParseException(ParameterException ex, String[] args) {
internalHandleParseException(ex, new PrintWriter(err(), true), colorScheme()); return returnResultOrExit(null); }
internalHandleParseException(ex, newPrintWriter(err(), getStderrEncoding()), colorScheme()); return returnResultOrExit(null); }

static void internalHandleParseException(ParameterException ex, PrintWriter writer, Help.ColorScheme colorScheme) {
writer.println(colorScheme.errorText(ex.getMessage()));
Expand Down Expand Up @@ -1878,7 +1878,7 @@ public static boolean printHelpIfRequested(ParseResult parseResult) {
* @since 3.6 */
@Deprecated public static boolean printHelpIfRequested(List<CommandLine> parsedCommands, PrintStream out, PrintStream err, Help.ColorScheme colorScheme) {
// for backwards compatibility
for (CommandLine cmd : parsedCommands) { cmd.setOut(new PrintWriter(out, true)).setErr(new PrintWriter(err, true)).setColorScheme(colorScheme); }
for (CommandLine cmd : parsedCommands) { cmd.setOut(newPrintWriter(out, getStdoutEncoding())).setErr(newPrintWriter(err, getStderrEncoding())).setColorScheme(colorScheme); }
return executeHelpRequest(parsedCommands) != null;
}

Expand Down Expand Up @@ -2134,8 +2134,8 @@ private <T> T enrichForBackwardsCompatibility(T obj) {
// and the application called #useOut, #useErr or #useAnsi on it
if (obj instanceof AbstractHandler<?, ?>) {
AbstractHandler<?, ?> handler = (AbstractHandler<?, ?>) obj;
if (handler.out() != System.out) { setOut(new PrintWriter(handler.out(), true)); }
if (handler.err() != System.err) { setErr(new PrintWriter(handler.err(), true)); }
if (handler.out() != System.out) { setOut(newPrintWriter(handler.out(), getStdoutEncoding())); }
if (handler.err() != System.err) { setErr(newPrintWriter(handler.err(), getStderrEncoding())); }
if (handler.ansi() != Help.Ansi.AUTO) { setColorScheme(handler.colorScheme()); }
}
return obj;
Expand Down Expand Up @@ -14476,6 +14476,17 @@ static void close(Closeable closeable) {
new Tracer().warn("Could not close " + closeable + ": " + ex.toString());
}
}
static Charset getStdoutEncoding() {
String encoding = System.getProperty("sun.stdout.encoding");
return encoding != null ? Charset.forName(encoding) : Charset.defaultCharset();
}
static Charset getStderrEncoding() {
String encoding = System.getProperty("sun.stderr.encoding");
return encoding != null ? Charset.forName(encoding) : Charset.defaultCharset();
}
static PrintWriter newPrintWriter(OutputStream stream, Charset charset) {
return new PrintWriter(new BufferedWriter(new OutputStreamWriter(stream, charset)), true);
}
static class PositionalParametersSorter implements Comparator<ArgSpec> {
private static final Range OPTION_INDEX = new Range(0, 0, false, true, "0");
public int compare(ArgSpec p1, ArgSpec p2) {
Expand Down Expand Up @@ -18015,7 +18026,7 @@ static List<String> stripErrorMessage(List<String> unmatched) {
public boolean isUnknownOption() { return isUnknownOption(unmatched, getCommandLine()); }
/** Returns {@code true} and prints suggested solutions to the specified stream if such solutions exist, otherwise returns {@code false}.
* @since 3.3.0 */
public boolean printSuggestions(PrintStream out) { return printSuggestions(new PrintWriter(out, true)); }
public boolean printSuggestions(PrintStream out) { return printSuggestions(newPrintWriter(out, getStdoutEncoding())); }
/** Returns {@code true} and prints suggested solutions to the specified stream if such solutions exist, otherwise returns {@code false}.
* @since 4.0 */
public boolean printSuggestions(PrintWriter writer) {
Expand Down
15 changes: 2 additions & 13 deletions src/test/java/picocli/ExecuteTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -51,19 +51,8 @@
import static org.hamcrest.CoreMatchers.containsString;
import static org.hamcrest.CoreMatchers.startsWith;
import static org.junit.Assert.*;
import static picocli.CommandLine.Command;
import static picocli.CommandLine.ExecutionException;
import static picocli.CommandLine.Help;
import static picocli.CommandLine.IExecutionStrategy;
import static picocli.CommandLine.*;
import static picocli.CommandLine.Model.UsageMessageSpec.keyValuesMap;
import static picocli.CommandLine.Option;
import static picocli.CommandLine.ParameterException;
import static picocli.CommandLine.Parameters;
import static picocli.CommandLine.ParseResult;
import static picocli.CommandLine.RunAll;
import static picocli.CommandLine.RunFirst;
import static picocli.CommandLine.RunLast;
import static picocli.CommandLine.Spec;

public class ExecuteTest {
@Rule
Expand Down Expand Up @@ -415,7 +404,7 @@ public void testExecuteWithInvalidInput_Ansi_ON() {
@Test
public void testExecuteWithInvalidInput_Ansi_ON_CustomErr() {
new CommandLine(new MyCallable())
.setErr(new PrintWriter(System.out, true))
.setErr(CommandLine.newPrintWriter(System.out, getStdoutEncoding()))
.setColorScheme(Help.defaultColorScheme(Help.Ansi.ON)).execute("invalid input");
assertEquals("", systemErrRule.getLog());
assertEquals(INVALID_INPUT_ANSI + MYCALLABLE_USAGE_ANSI, systemOutRule.getLog());
Expand Down
8 changes: 4 additions & 4 deletions src/test/java/picocli/HelpSubCommandTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -168,8 +168,8 @@ public void run() { }

int exitCode = new CommandLine(new App())
.setExecutionStrategy(new RunLast())
.setOut(new PrintWriter(System.out, true))
.setErr(new PrintWriter(System.err, true))
.setOut(CommandLine.newPrintWriter(System.out, getStdoutEncoding()))
.setErr(CommandLine.newPrintWriter(System.err, getStderrEncoding()))
.setColorScheme(Help.defaultColorScheme(Help.Ansi.OFF))
.execute("customHelp");

Expand Down Expand Up @@ -206,8 +206,8 @@ public void run() { }

int exitCode = new CommandLine(new App())
.setExecutionStrategy(new RunLast())
.setOut(new PrintWriter(System.out, true))
.setErr(new PrintWriter(System.err, true))
.setOut(CommandLine.newPrintWriter(System.out, getStdoutEncoding()))
.setErr(CommandLine.newPrintWriter(System.err, getStderrEncoding()))
.setColorScheme(Help.defaultColorScheme(Help.Ansi.OFF))
.execute("newCustomHelp");

Expand Down
80 changes: 80 additions & 0 deletions src/test/java/picocli/Issue1320.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,80 @@
package picocli;

import org.junit.Rule;
import org.junit.Test;
import org.junit.contrib.java.lang.system.RestoreSystemProperties;
import org.junit.contrib.java.lang.system.SystemErrRule;
import org.junit.contrib.java.lang.system.SystemOutRule;
import org.junit.rules.TestRule;
import picocli.CommandLine.Command;
import picocli.CommandLine.Model.CommandSpec;
import picocli.CommandLine.Parameters;
import picocli.CommandLine.Spec;

import java.nio.charset.Charset;

import static org.junit.Assert.assertEquals;

public class Issue1320 {

@Rule
public final TestRule restoreSystemProperties = new RestoreSystemProperties();

@Rule
public final SystemOutRule systemOutRule = new SystemOutRule().enableLog();

@Rule
public final SystemErrRule systemErrRule = new SystemErrRule().enableLog();

@Command(name = "test")
static class TestCommand implements Runnable {

@Parameters
String text;

@Spec
CommandSpec spec;

@Override
public void run() {
spec.commandLine().getOut().print(text);
spec.commandLine().getOut().flush();
spec.commandLine().getErr().print(text);
spec.commandLine().getErr().flush();
}
}

@Test
public void testIssue1320() {
String unmappable = "[abcµ]";

resetLogs();
System.clearProperty(SUN_STDOUT_ENCODING);
System.clearProperty(SUN_STDERR_ENCODING);
fixLogPrintStream(Charset.defaultCharset().name());
assertEquals(CommandLine.ExitCode.OK, new CommandLine(new TestCommand()).execute(unmappable));
assertEquals(unmappable, systemOutRule.getLog());
assertEquals(unmappable, systemErrRule.getLog());

resetLogs();
System.setProperty(SUN_STDOUT_ENCODING, CP_437);
System.setProperty(SUN_STDERR_ENCODING, CP_437);
fixLogPrintStream(CP_437);
assertEquals(CommandLine.ExitCode.OK, new CommandLine(new TestCommand()).execute(unmappable));
assertEquals(unmappable, systemOutRule.getLog());
assertEquals(unmappable, systemErrRule.getLog());
}

private void resetLogs() {
systemOutRule.clearLog();
systemErrRule.clearLog();
}

private void fixLogPrintStream(String encoding) {
System.setProperty("file.encoding", encoding);
}

private static final String CP_437 = "cp437";
private static final String SUN_STDOUT_ENCODING = "sun.stdout.encoding";
private static final String SUN_STDERR_ENCODING = "sun.stderr.encoding";
}