Fix MalformedInputException in ProcessReader

nbbrd · Jun 21, 2024 · ece6723 · ece6723
1 parent 9a7829e
commit ece6723
Show file tree

Hide file tree

Showing 11 changed files with 91 additions and 55 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -7,6 +7,10 @@ to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
 
 ## [Unreleased]
 
+### Fixed
+
+- Fix MalformedInputException in ProcessReader [#329](https://github.com/nbbrd/java-io-util/issues/329)
+
 ## [0.0.28] - 2024-03-21
 
 ### Fixed

diff --git a/java-io-base/src/main/java/nbbrd/io/sys/EndOfProcessException.java b/java-io-base/src/main/java/nbbrd/io/sys/EndOfProcessException.java
@@ -26,7 +26,7 @@ private EndOfProcessException(int exitValue, String errorMessage) {
     }
 
     private static String readErrorStream(Process process) throws IOException {
-        try (BufferedReader reader = TextResource.newBufferedReader(process.getErrorStream(), Charset.defaultCharset().newDecoder())) {
+        try (BufferedReader reader = TextResource.newBufferedReader(process.getErrorStream(), Charset.defaultCharset())) {
             return InternalTextResource.copyByLineToString(reader, System.lineSeparator());
         }
     }

diff --git a/java-io-base/src/main/java/nbbrd/io/sys/ProcessReader.java b/java-io-base/src/main/java/nbbrd/io/sys/ProcessReader.java
@@ -37,7 +37,7 @@ public class ProcessReader {
     }
 
     public static @NonNull BufferedReader newReader(@NonNull Process process) {
-        return TextResource.newBufferedReader(new ProcessInputStream(process), Charset.defaultCharset().newDecoder());
+        return TextResource.newBufferedReader(new ProcessInputStream(process), Charset.defaultCharset());
     }
 
     public static @NonNull String readToString(@NonNull String... args) throws IOException {

diff --git a/java-io-base/src/main/java/nbbrd/io/text/TextResource.java b/java-io-base/src/main/java/nbbrd/io/text/TextResource.java
@@ -7,6 +7,7 @@
 import java.nio.charset.Charset;
 import java.nio.charset.CharsetDecoder;
 import java.nio.charset.CharsetEncoder;
+import java.nio.charset.CodingErrorAction;
 import java.util.Optional;
 
 import static nbbrd.io.Resource.newInputStream;
@@ -15,20 +16,34 @@
 public class TextResource {
 
     /**
-     * @deprecated use {@link #newBufferedReader(Class, String, CharsetDecoder)} instead.
+     * @deprecated use {@link #newBufferedReader(Class, String, Charset)} instead.
      */
     @Deprecated
     public @NonNull Optional<BufferedReader> getResourceAsBufferedReader(@NonNull Class<?> anchor, @NonNull String name, @NonNull Charset charset) {
         return Resource.getResourceAsStream(anchor, name)
-                .map(stream -> newBufferedReader(stream, charset.newDecoder()));
+                .map(stream -> newBufferedReader(stream, charset));
+    }
+
+    public @NonNull BufferedReader newBufferedReader(@NonNull Class<?> anchor, @NonNull String name, @NonNull Charset charset) throws IOException {
+        return newBufferedReader(newInputStream(anchor, name), charset);
     }
 
     public @NonNull BufferedReader newBufferedReader(@NonNull Class<?> anchor, @NonNull String name, @NonNull CharsetDecoder decoder) throws IOException {
-        return newBufferedReader(newInputStream(anchor, name), decoder);
+        return newBufferedReader(newInputStream(anchor, name), configureDecoderForInputStreamReader(decoder));
+    }
+
+    public @NonNull BufferedReader newBufferedReader(@NonNull InputStream stream, @NonNull Charset charset) {
+        return new BufferedReader(new InputStreamReader(stream, charset));
     }
 
     public @NonNull BufferedReader newBufferedReader(@NonNull InputStream stream, @NonNull CharsetDecoder decoder) {
-        return new BufferedReader(new InputStreamReader(stream, decoder));
+        return new BufferedReader(new InputStreamReader(stream, configureDecoderForInputStreamReader(decoder)));
+    }
+
+    private @NonNull CharsetDecoder configureDecoderForInputStreamReader(@NonNull CharsetDecoder decoder) {
+        return decoder
+                .onMalformedInput(CodingErrorAction.REPLACE)
+                .onUnmappableCharacter(CodingErrorAction.REPLACE);
     }
 
     public @NonNull BufferedWriter newBufferedWriter(@NonNull OutputStream stream, @NonNull CharsetEncoder encoder) {

diff --git a/java-io-base/src/test/java/_test/io/ResourceId.java b/java-io-base/src/test/java/_test/io/ResourceId.java
@@ -44,7 +44,7 @@ public byte[] toBytes() throws IOException {
     }
 
     public BufferedReader open(Charset encoding) throws IOException {
-        return newBufferedReader(anchor, name, encoding.newDecoder());
+        return newBufferedReader(anchor, name, encoding);
     }
 
     public String copyToString(Charset encoding) throws IOException {

diff --git a/java-io-base/src/test/java/nbbrd/io/FileParserTest.java b/java-io-base/src/test/java/nbbrd/io/FileParserTest.java
@@ -81,7 +81,7 @@ public void testOnParsingLock(@TempDir Path temp) throws IOException {
     }
 
     private final IOFunction<InputStream, String> deserializeAndClose = resource -> {
-        try (Reader reader = TextResource.newBufferedReader(resource, UTF_8.newDecoder())) {
+        try (Reader reader = TextResource.newBufferedReader(resource, UTF_8)) {
             return InternalTextResource.copyToString(reader);
         }
     };

diff --git a/java-io-base/src/test/java/nbbrd/io/text/TextResourceTest.java b/java-io-base/src/test/java/nbbrd/io/text/TextResourceTest.java
@@ -19,6 +19,7 @@
 import org.junit.jupiter.api.Test;
 
 import java.io.*;
+import java.nio.charset.Charset;
 import java.nio.charset.CharsetDecoder;
 
 import static java.nio.charset.StandardCharsets.UTF_8;
@@ -46,31 +47,31 @@ public void testGetResourceAsBufferedReader() throws IOException {
     @Test
     @SuppressWarnings({"null", "resource", "DataFlowIssue"})
     public void testNewBufferedReaderOfResource() throws IOException {
-        CharsetDecoder utf8 = UTF_8.newDecoder();
-
-        assertThatNullPointerException().isThrownBy(() -> newBufferedReader(null, "", utf8));
-        assertThatNullPointerException().isThrownBy(() -> newBufferedReader(TextResourceTest.class, null, utf8));
-        assertThatNullPointerException().isThrownBy(() -> newBufferedReader(TextResourceTest.class, "", null));
+        assertThatNullPointerException().isThrownBy(() -> newBufferedReader(null, "", UTF_8));
+        assertThatNullPointerException().isThrownBy(() -> newBufferedReader(TextResourceTest.class, null, UTF_8));
+        assertThatNullPointerException().isThrownBy(() -> newBufferedReader(TextResourceTest.class, "", (Charset) null));
+        assertThatNullPointerException().isThrownBy(() -> newBufferedReader(TextResourceTest.class, "", (CharsetDecoder) null));
 
         assertThatIOException()
-                .isThrownBy(() -> newBufferedReader(TextResourceTest.class, "missing", utf8))
+                .isThrownBy(() -> newBufferedReader(TextResourceTest.class, "missing", UTF_8))
                 .withMessageContaining("missing")
                 .withMessageContaining(TextResourceTest.class.getName());
 
-        try (BufferedReader reader = newBufferedReader(TextResourceTest.class, "/nbbrd/io/text/hello.txt", utf8)) {
+        try (BufferedReader reader = newBufferedReader(TextResourceTest.class, "/nbbrd/io/text/hello.txt", UTF_8)) {
             assertThat(reader.lines()).contains("world");
         }
     }
 
     @SuppressWarnings({"resource", "DataFlowIssue"})
     @Test
     public void testNewBufferedReader() throws IOException {
-        byte[] bytes = "world".getBytes(UTF_8);
+        byte[] bytes = "world" .getBytes(UTF_8);
 
-        assertThatNullPointerException().isThrownBy(() -> newBufferedReader(null, UTF_8.newDecoder()));
-        assertThatNullPointerException().isThrownBy(() -> newBufferedReader(new ByteArrayInputStream(bytes), null));
+        assertThatNullPointerException().isThrownBy(() -> newBufferedReader(null, UTF_8));
+        assertThatNullPointerException().isThrownBy(() -> newBufferedReader(new ByteArrayInputStream(bytes), (Charset) null));
+        assertThatNullPointerException().isThrownBy(() -> newBufferedReader(new ByteArrayInputStream(bytes), (CharsetDecoder) null));
 
-        try (BufferedReader reader = newBufferedReader(new ByteArrayInputStream(bytes), UTF_8.newDecoder())) {
+        try (BufferedReader reader = newBufferedReader(new ByteArrayInputStream(bytes), UTF_8)) {
             assertThat(reader.lines()).contains("world");
         }
     }

diff --git a/java-io-base/src/test/java/nbbrd/io/zip/ZipTest.java b/java-io-base/src/test/java/nbbrd/io/zip/ZipTest.java
@@ -58,10 +58,10 @@ public void testZipLoaderOf() throws IOException {
             assertThatNullPointerException().isThrownBy(() -> loader.load(null));
             assertThatIOException().isThrownBy(() -> loader.load("xyz"));
             try (InputStream stream = loader.load("hello.txt")) {
-                assertThat(TextResource.newBufferedReader(stream, UTF_8.newDecoder()).lines()).containsExactly("hello");
+                assertThat(TextResource.newBufferedReader(stream, UTF_8).lines()).containsExactly("hello");
             }
             try (InputStream stream = loader.load("folder1/world.txt")) {
-                assertThat(TextResource.newBufferedReader(stream, UTF_8.newDecoder()).lines()).containsExactly("world");
+                assertThat(TextResource.newBufferedReader(stream, UTF_8).lines()).containsExactly("world");
             }
         }
 
@@ -83,10 +83,10 @@ public void testZipLoaderCopyOf() throws IOException {
                 assertThatNullPointerException().isThrownBy(() -> loader.load(null));
                 assertThatIOException().isThrownBy(() -> loader.load("xyz"));
                 try (InputStream stream = loader.load("hello.txt")) {
-                    assertThat(TextResource.newBufferedReader(stream, UTF_8.newDecoder()).lines()).containsExactly("hello");
+                    assertThat(TextResource.newBufferedReader(stream, UTF_8).lines()).containsExactly("hello");
                 }
                 try (InputStream stream = loader.load("folder1/world.txt")) {
-                    assertThat(TextResource.newBufferedReader(stream, UTF_8.newDecoder()).lines()).containsExactly("world");
+                    assertThat(TextResource.newBufferedReader(stream, UTF_8).lines()).containsExactly("world");
                 }
             }
         }
@@ -97,7 +97,7 @@ public void testZipLoaderCopyOf() throws IOException {
                 assertThatIOException().isThrownBy(() -> loader.load("xyz"));
                 assertThatIOException().isThrownBy(() -> loader.load("hello.txt"));
                 try (InputStream stream = loader.load("folder1/world.txt")) {
-                    assertThat(TextResource.newBufferedReader(stream, UTF_8.newDecoder()).lines()).containsExactly("world");
+                    assertThat(TextResource.newBufferedReader(stream, UTF_8).lines()).containsExactly("world");
                 }
             }
         }

diff --git a/java-io-curl/src/test/java/nbbrd/io/curl/CurlTest.java b/java-io-curl/src/test/java/nbbrd/io/curl/CurlTest.java
@@ -109,7 +109,7 @@ public void testCommandBuilder(@TempDir File temp) throws MalformedURLException
 
     @Test
     public void testHead() throws IOException {
-        try (BufferedReader reader = newBufferedReader(CurlTest.class, "curlhead.txt", UTF_8.newDecoder())) {
+        try (BufferedReader reader = newBufferedReader(CurlTest.class, "curlhead.txt", UTF_8)) {
             assertThat(Curl.Head.parseResponse(reader))
                     .singleElement()
                     .isEqualTo(new Curl.Head(
@@ -132,7 +132,7 @@ public void testHead() throws IOException {
                     ));
         }
 
-        try (BufferedReader reader = newBufferedReader(CurlTest.class, "curlhead2.txt", UTF_8.newDecoder())) {
+        try (BufferedReader reader = newBufferedReader(CurlTest.class, "curlhead2.txt", UTF_8)) {
             assertThat(Curl.Head.parseResponse(reader))
                     .hasSize(2)
                     .satisfies(head -> assertThat(head.getStatus().getCode()).isEqualTo(301), atIndex(0))

diff --git a/java-io-win/src/test/java/nbbrd/io/win/CScriptWrapperTest.java b/java-io-win/src/test/java/nbbrd/io/win/CScriptWrapperTest.java
@@ -16,71 +16,87 @@
  */
 package nbbrd.io.win;
 
-import nbbrd.io.sys.OS;
 import nbbrd.io.sys.ProcessReader;
-import org.assertj.core.api.Assumptions;
 import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.condition.EnabledOnOs;
+import org.junit.jupiter.api.condition.OS;
 import org.junit.jupiter.api.io.TempDir;
 
 import java.io.File;
 import java.io.IOException;
-import java.nio.charset.StandardCharsets;
 import java.nio.file.Files;
 import java.nio.file.Path;
-import java.util.Arrays;
 
+import static java.nio.charset.StandardCharsets.UTF_8;
+import static java.nio.file.Files.createTempFile;
+import static java.util.Arrays.asList;
 import static nbbrd.io.win.CScriptWrapper.NO_TIMEOUT;
-import static org.assertj.core.api.Assertions.assertThat;
-import static org.assertj.core.api.Assertions.assertThatNullPointerException;
+import static nbbrd.io.win.CScriptWrapper.exec;
+import static org.assertj.core.api.Assertions.*;
 
 /**
  * @author Philippe Charles
  */
 public class CScriptWrapperTest {
 
+    @SuppressWarnings("DataFlowIssue")
     @Test
-    public void testExec(@TempDir Path temp) throws IOException, InterruptedException {
+    public void testParameters(@TempDir Path temp) {
         assertThatNullPointerException()
-                .isThrownBy(() -> CScriptWrapper.exec(null, NO_TIMEOUT, ""));
+                .isThrownBy(() -> exec(null, NO_TIMEOUT, ""));
 
         assertThatNullPointerException()
-                .isThrownBy(() -> CScriptWrapper.exec(Files.createTempFile("a", "b").toFile(), NO_TIMEOUT, (String[]) null));
-
-        Assumptions.assumeThat(OS.NAME).isEqualTo(OS.Name.WINDOWS);
+                .isThrownBy(() -> exec(createTempFile(temp, "a", "b").toFile(), NO_TIMEOUT, (String[]) null));
+    }
 
-        assertThat(CScriptWrapper.exec(vbs(temp, ""), NO_TIMEOUT).waitFor())
+    @Test
+    @EnabledOnOs(OS.WINDOWS)
+    public void testExitCode(@TempDir Path temp) throws IOException, InterruptedException {
+        assertThat(exec(vbs(temp, ""), NO_TIMEOUT).waitFor())
                 .isEqualTo(0);
 
-        assertThat(CScriptWrapper.exec(vbs(temp, "WScript.Quit -123"), NO_TIMEOUT).waitFor())
+        assertThat(exec(vbs(temp, "WScript.Quit -123"), NO_TIMEOUT).waitFor())
                 .isEqualTo(-123);
+    }
+
+    @Test
+    @EnabledOnOs(OS.WINDOWS)
+    public void testTimeOut(@TempDir Path temp) throws IOException, InterruptedException {
+        File infiniteLoop = vbs(temp,
+                "While (true)",
+                "Wend"
+        );
+
+        assertThat(exec(infiniteLoop, (short) 2).waitFor())
+                .isEqualTo(0);
 
+        assertThat(ProcessReader.readToString(exec(infiniteLoop, (short) 2)))
+                .contains(infiniteLoop.toString());
+    }
+
+    @Test
+    @EnabledOnOs(OS.WINDOWS)
+    public void testOutput(@TempDir Path temp) throws IOException, InterruptedException {
         File scriptWithArgs = vbs(temp,
                 "For Each strArg in Wscript.Arguments",
                 "  WScript.Echo strArg",
                 "Next"
         );
 
-        assertThat(CScriptWrapper.exec(scriptWithArgs, NO_TIMEOUT, "a", "b", "c").waitFor())
+        assertThat(exec(scriptWithArgs, NO_TIMEOUT, "a", "b", "c").waitFor())
                 .isEqualTo(0);
 
-        assertThat(ProcessReader.readToString(CScriptWrapper.exec(scriptWithArgs, NO_TIMEOUT, "a", "b", "c")))
+        assertThat(ProcessReader.readToString(exec(scriptWithArgs, NO_TIMEOUT, "a", "b", "c")))
                 .isEqualTo("a" + System.lineSeparator() + "b" + System.lineSeparator() + "c");
 
-        File infiniteLoop = vbs(temp,
-                "While (true)",
-                "Wend"
-        );
-
-        assertThat(CScriptWrapper.exec(infiniteLoop, (short) 2).waitFor())
-                .isEqualTo(0);
-
-        assertThat(ProcessReader.readToString(CScriptWrapper.exec(infiniteLoop, (short) 2)))
-                .contains(infiniteLoop.toString());
+        String emoji = "\uD83D\uDCA1"; // 💡
+        assertThatCode(() -> ProcessReader.readToString(exec(vbs(temp, "WScript.Echo \"" + emoji + "\""), NO_TIMEOUT)))
+                .doesNotThrowAnyException();
     }
 
-    private File vbs(Path temp, String... content) throws IOException {
-        File script = Files.createTempFile("script", ".vbs").toFile();
-        Files.write(script.toPath(), Arrays.asList(content), StandardCharsets.UTF_8);
+    private static File vbs(Path temp, String... content) throws IOException {
+        File script = createTempFile(temp, "script", ".vbs").toFile();
+        Files.write(script.toPath(), asList(content), UTF_8);
         return script;
     }
 }
diff --git a/java-io-win/src/test/java/nbbrd/io/win/RegWrapperTest.java b/java-io-win/src/test/java/nbbrd/io/win/RegWrapperTest.java
@@ -107,7 +107,7 @@ public void testQuery() throws IOException {
     }
 
     static Map<String, List<RegValue>> parse(String resourceName) throws IOException {
-        try (BufferedReader reader = newBufferedReader(RegWrapperTest.class, resourceName, Charset.defaultCharset().newDecoder())) {
+        try (BufferedReader reader = newBufferedReader(RegWrapperTest.class, resourceName, Charset.defaultCharset())) {
             return RegWrapper.parse(reader);
         }
     }