From 87391ccee97dd2d51871838f7b065bf6cf9a07a8 Mon Sep 17 00:00:00 2001 From: David Ostrovsky Date: Wed, 8 Jun 2022 07:22:04 +0200 Subject: [PATCH] Fix DefaultCharset bug pattern flagged by error prone See more details in: [1]. [1] https://errorprone.info/bugpattern/DefaultCharset Change-Id: I3de0be57a2d74490a5b4e66801e9767b38f13bf9 --- .../src/org/eclipse/jgit/pgm/DiffTool.java | 9 ++++-- .../src/org/eclipse/jgit/pgm/MergeTool.java | 14 +++++++-- .../tst/org/eclipse/jgit/util/HookTest.java | 29 ++++++++++--------- .../diffmergetool/CommandExecutor.java | 8 +++-- .../internal/diffmergetool/ToolException.java | 8 +++-- 5 files changed, 44 insertions(+), 24 deletions(-) diff --git a/org.eclipse.jgit.pgm/src/org/eclipse/jgit/pgm/DiffTool.java b/org.eclipse.jgit.pgm/src/org/eclipse/jgit/pgm/DiffTool.java index 3113eeb64..87c71795e 100644 --- a/org.eclipse.jgit.pgm/src/org/eclipse/jgit/pgm/DiffTool.java +++ b/org.eclipse.jgit.pgm/src/org/eclipse/jgit/pgm/DiffTool.java @@ -19,6 +19,7 @@ import java.io.FileOutputStream; import java.io.IOException; import java.io.InputStreamReader; +import java.nio.charset.Charset; import java.text.MessageFormat; import java.util.List; import java.util.Map; @@ -244,11 +245,15 @@ private void compare(List files) throws IOException { // TODO: check how to return the exit-code of the tool // to jgit / java runtime ? // int rc =... + Charset defaultCharset = SystemReader.getInstance() + .getDefaultCharset(); outw.println( - new String(result.getStdout().toByteArray())); + new String(result.getStdout().toByteArray(), + defaultCharset)); outw.flush(); errw.println( - new String(result.getStderr().toByteArray())); + new String(result.getStderr().toByteArray(), + defaultCharset)); errw.flush(); } } catch (ToolException e) { diff --git a/org.eclipse.jgit.pgm/src/org/eclipse/jgit/pgm/MergeTool.java b/org.eclipse.jgit.pgm/src/org/eclipse/jgit/pgm/MergeTool.java index 2a411b81f..a382fab75 100644 --- a/org.eclipse.jgit.pgm/src/org/eclipse/jgit/pgm/MergeTool.java +++ b/org.eclipse.jgit.pgm/src/org/eclipse/jgit/pgm/MergeTool.java @@ -18,6 +18,7 @@ import java.io.FileOutputStream; import java.io.IOException; import java.io.InputStreamReader; +import java.nio.charset.Charset; import java.text.MessageFormat; import java.util.ArrayList; import java.util.Collections; @@ -55,6 +56,7 @@ import org.eclipse.jgit.treewalk.WorkingTreeOptions; import org.eclipse.jgit.treewalk.filter.PathFilterGroup; import org.eclipse.jgit.util.FS.ExecutionResult; +import org.eclipse.jgit.util.SystemReader; import org.kohsuke.args4j.Argument; import org.kohsuke.args4j.Option; import org.kohsuke.args4j.spi.RestOfArgumentsHandler; @@ -108,7 +110,9 @@ void noGui(@SuppressWarnings("unused") boolean on) { protected void init(Repository repository, String gitDir) { super.init(repository, gitDir); mergeTools = new MergeTools(repository); - inputReader = new BufferedReader(new InputStreamReader(ins)); + inputReader = new BufferedReader( + new InputStreamReader(ins, + SystemReader.getInstance().getDefaultCharset())); } enum MergeResult { @@ -285,9 +289,13 @@ private MergeResult mergeModified(String mergedFilePath, boolean showPrompt) gui, this::promptForLaunch, this::informUserNoTool); if (optionalResult.isPresent()) { ExecutionResult result = optionalResult.get(); - outw.println(new String(result.getStdout().toByteArray())); + Charset defaultCharset = SystemReader.getInstance() + .getDefaultCharset(); + outw.println(new String(result.getStdout().toByteArray(), + defaultCharset)); outw.flush(); - errw.println(new String(result.getStderr().toByteArray())); + errw.println(new String(result.getStderr().toByteArray(), + defaultCharset)); errw.flush(); } else { return MergeResult.ABORTED; diff --git a/org.eclipse.jgit.test/tst/org/eclipse/jgit/util/HookTest.java b/org.eclipse.jgit.test/tst/org/eclipse/jgit/util/HookTest.java index 33ed360ef..1231aefee 100644 --- a/org.eclipse.jgit.test/tst/org/eclipse/jgit/util/HookTest.java +++ b/org.eclipse.jgit.test/tst/org/eclipse/jgit/util/HookTest.java @@ -9,6 +9,7 @@ */ package org.eclipse.jgit.util; +import static java.nio.charset.StandardCharsets.UTF_8; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertNull; import static org.junit.Assert.fail; @@ -77,7 +78,7 @@ public void testFailedCommitMsgHookBlocksCommit() throws Exception { "Rejected by \"commit-msg\" hook.\nstderr\n", e.getMessage()); assertEquals("unexpected output from commit-msg hook", "test\n", - out.toString()); + out.toString(UTF_8)); } } @@ -95,7 +96,7 @@ public void testCommitMsgHookReceivesCorrectParameter() throws Exception { git.commit().setMessage("commit") .setHookOutputStream(new PrintStream(out)).call(); assertEquals(".git/COMMIT_EDITMSG\n", - out.toString("UTF-8")); + out.toString(UTF_8)); } @Test @@ -129,9 +130,9 @@ public void testPostCommitRunHook() throws Exception { new PrintStream(out), new PrintStream(err), "stdin"); assertEquals("unexpected hook output", "test arg1 arg2\nstdin\n", - out.toString("UTF-8")); + out.toString(UTF_8)); assertEquals("unexpected output on stderr stream", "stderr\n", - err.toString("UTF-8")); + err.toString(UTF_8)); assertEquals("unexpected exit code", 0, res.getExitCode()); assertEquals("unexpected process status", ProcessResult.Status.OK, res.getStatus()); @@ -160,7 +161,7 @@ public void testAllCommitHooks() throws Exception { } assertEquals("unexpected hook output", "test pre-commit\ntest commit-msg .git/COMMIT_EDITMSG\ntest post-commit\n", - out.toString("UTF-8")); + out.toString(UTF_8)); } @Test @@ -181,9 +182,9 @@ public void testRunHook() throws Exception { assertEquals("unexpected hook output", "test arg1 arg2\nstdin\n" + db.getDirectory().getAbsolutePath() + '\n' + db.getWorkTree().getAbsolutePath() + '\n', - out.toString("UTF-8")); + out.toString(UTF_8)); assertEquals("unexpected output on stderr stream", "stderr\n", - err.toString("UTF-8")); + err.toString(UTF_8)); assertEquals("unexpected exit code", 0, res.getExitCode()); assertEquals("unexpected process status", ProcessResult.Status.OK, res.getStatus()); @@ -214,9 +215,9 @@ public void testRunHookHooksPathRelative() throws Exception { "test arg1 arg2\nstdin\n" + db.getDirectory().getAbsolutePath() + '\n' + db.getWorkTree().getAbsolutePath() + '\n', - out.toString("UTF-8")); + out.toString(UTF_8)); assertEquals("unexpected output on stderr stream", "stderr\n", - err.toString("UTF-8")); + err.toString(UTF_8)); assertEquals("unexpected exit code", 0, res.getExitCode()); assertEquals("unexpected process status", ProcessResult.Status.OK, res.getStatus()); @@ -249,9 +250,9 @@ public void testRunHookHooksPathAbsolute() throws Exception { "test arg1 arg2\nstdin\n" + db.getDirectory().getAbsolutePath() + '\n' + db.getWorkTree().getAbsolutePath() + '\n', - out.toString("UTF-8")); + out.toString(UTF_8)); assertEquals("unexpected output on stderr stream", "stderr\n", - err.toString("UTF-8")); + err.toString(UTF_8)); assertEquals("unexpected exit code", 0, res.getExitCode()); assertEquals("unexpected process status", ProcessResult.Status.OK, res.getStatus()); @@ -281,9 +282,9 @@ public void testHookPathWithBlank() throws Exception { "test arg1 arg2\nstdin\n" + db.getDirectory().getAbsolutePath() + '\n' + db.getWorkTree().getAbsolutePath() + '\n', - out.toString("UTF-8")); + out.toString(UTF_8)); assertEquals("unexpected output on stderr stream", "stderr\n", - err.toString("UTF-8")); + err.toString(UTF_8)); assertEquals("unexpected exit code", 0, res.getExitCode()); assertEquals("unexpected process status", ProcessResult.Status.OK, res.getStatus()); @@ -310,7 +311,7 @@ public void testFailedPreCommitHookBlockCommit() throws Exception { "Rejected by \"pre-commit\" hook.\nstderr\n", e.getMessage()); assertEquals("unexpected output from pre-commit hook", "test\n", - out.toString()); + out.toString(UTF_8)); } } diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/internal/diffmergetool/CommandExecutor.java b/org.eclipse.jgit/src/org/eclipse/jgit/internal/diffmergetool/CommandExecutor.java index 668adeab6..ebef5247e 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/internal/diffmergetool/CommandExecutor.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/internal/diffmergetool/CommandExecutor.java @@ -27,6 +27,7 @@ import org.eclipse.jgit.util.FS_Win32; import org.eclipse.jgit.util.FS_Win32_Cygwin; import org.eclipse.jgit.util.StringUtils; +import org.eclipse.jgit.util.SystemReader; /** * Runs a command with help of FS. @@ -87,7 +88,9 @@ public ExecutionResult run(String command, File workingDir, + "execError: " + execError + "\n" //$NON-NLS-1$ //$NON-NLS-2$ + "stderr: \n" //$NON-NLS-1$ + new String( - result.getStderr().toByteArray()), + result.getStderr().toByteArray(), + SystemReader.getInstance() + .getDefaultCharset()), result, execError); } } @@ -202,7 +205,8 @@ private void createCommandFile(String command) commandFile = File.createTempFile(".__", //$NON-NLS-1$ "__jgit_tool" + fileExtension); //$NON-NLS-1$ try (OutputStream outStream = new FileOutputStream(commandFile)) { - byte[] strToBytes = command.getBytes(); + byte[] strToBytes = command + .getBytes(SystemReader.getInstance().getDefaultCharset()); outStream.write(strToBytes); outStream.close(); } diff --git a/org.eclipse.jgit/src/org/eclipse/jgit/internal/diffmergetool/ToolException.java b/org.eclipse.jgit/src/org/eclipse/jgit/internal/diffmergetool/ToolException.java index 7cc5bb50d..73d358890 100644 --- a/org.eclipse.jgit/src/org/eclipse/jgit/internal/diffmergetool/ToolException.java +++ b/org.eclipse.jgit/src/org/eclipse/jgit/internal/diffmergetool/ToolException.java @@ -11,7 +11,7 @@ package org.eclipse.jgit.internal.diffmergetool; import org.eclipse.jgit.util.FS.ExecutionResult; - +import org.eclipse.jgit.util.SystemReader; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -114,7 +114,8 @@ public String getResultStderr() { return ""; //$NON-NLS-1$ } try { - return new String(result.getStderr().toByteArray()); + return new String(result.getStderr().toByteArray(), + SystemReader.getInstance().getDefaultCharset()); } catch (Exception e) { LOG.warn("Failed to retrieve standard error output", e); //$NON-NLS-1$ } @@ -129,7 +130,8 @@ public String getResultStdout() { return ""; //$NON-NLS-1$ } try { - return new String(result.getStdout().toByteArray()); + return new String(result.getStdout().toByteArray(), + SystemReader.getInstance().getDefaultCharset()); } catch (Exception e) { LOG.warn("Failed to retrieve standard output", e); //$NON-NLS-1$ }