Forward compatibility for Commons IO 2.13.0 in hudson.Util#loadFile (#8142)

core/src/main/java/hudson/Util.java

@@ -32,13 +32,14 @@ import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
 import hudson.model.TaskListener;
 import hudson.util.QuotedStringTokenizer;
 import hudson.util.VariableResolver;
+import java.io.BufferedReader;
 import java.io.ByteArrayInputStream;
 import java.io.ByteArrayOutputStream;
 import java.io.Closeable;
 import java.io.File;
-import java.io.FileNotFoundException;
 import java.io.IOException;
 import java.io.InputStream;
+import java.io.InputStreamReader;
 import java.io.OutputStream;
 import java.io.OutputStreamWriter;
 import java.io.PrintStream;
@@ -55,7 +56,9 @@ import java.nio.ByteBuffer;
 import java.nio.CharBuffer;
 import java.nio.charset.CharacterCodingException;
 import java.nio.charset.Charset;
+import java.nio.charset.CharsetDecoder;
 import java.nio.charset.CharsetEncoder;
+import java.nio.charset.CodingErrorAction;
 import java.nio.charset.StandardCharsets;
 import java.nio.file.CopyOption;
 import java.nio.file.FileAlreadyExistsException;
@@ -64,6 +67,7 @@ import java.nio.file.FileSystems;
 import java.nio.file.Files;
 import java.nio.file.InvalidPathException;
 import java.nio.file.LinkOption;
+import java.nio.file.NoSuchFileException;
 import java.nio.file.Path;
 import java.nio.file.Paths;
 import java.nio.file.StandardCopyOption;
@@ -108,7 +112,6 @@ import jenkins.util.MemoryReductionUtil;
 import jenkins.util.SystemProperties;
 import jenkins.util.io.PathRemover;
 import org.apache.commons.codec.digest.DigestUtils;
-import org.apache.commons.io.FileUtils;
 import org.apache.commons.io.IOUtils;
 import org.apache.commons.io.output.NullOutputStream;
 import org.apache.commons.lang.time.FastDateFormat;
@@ -247,15 +250,22 @@ public class Util {
         // contain unmappable and/or malformed byte sequences. We need to make
         // sure that in such cases, no CharacterCodingException is thrown.
         //
-        // One approach that cannot be used is to call Files.newBufferedReader()
-        // because there is a difference in how an InputStreamReader constructed
-        // from a Charset and the reader returned by Files.newBufferedReader()
-        // handle malformed and unmappable byte sequences for the specified
-        // encoding; the latter is more picky and will throw an exception.
+        // One approach that cannot be used is Files.newBufferedReader, which
+        // creates its CharsetDecoder with the default behavior of reporting
+        // malformed input and unmappable character errors. The implementation
+        // of InputStreamReader(InputStream, Charset) has the desired behavior
+        // of replacing malformed input and unmappable character errors, but
+        // this implementation is not specified in the API contract. Therefore,
+        // we explicitly use a decoder with the desired behavior.
         // See: https://issues.jenkins.io/browse/JENKINS-49060?focusedCommentId=325989&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-325989
-        try {
-            return FileUtils.readFileToString(logfile, charset);
-        } catch (FileNotFoundException e) {
+        CharsetDecoder decoder = charset.newDecoder()
+                .onMalformedInput(CodingErrorAction.REPLACE)
+                .onUnmappableCharacter(CodingErrorAction.REPLACE);
+        try (InputStream is = Files.newInputStream(Util.fileToPath(logfile));
+                Reader isr = new InputStreamReader(is, decoder);
+                Reader br = new BufferedReader(isr)) {
+            return IOUtils.toString(br);
+        } catch (NoSuchFileException e) {
             return "";
         } catch (Exception e) {
             throw new IOException("Failed to fully read " + logfile, e);

core/src/test/java/hudson/UtilTest.java

@@ -58,6 +58,7 @@ import java.util.HashMap;
 import java.util.Locale;
 import java.util.Map;
 import java.util.Properties;
+import org.apache.commons.io.FileUtils;
 import org.hamcrest.BaseMatcher;
 import org.hamcrest.Description;
 import org.junit.Assert;
@@ -433,6 +434,39 @@ public class UtilTest {
         assertTrue(Util.isSafeToRedirectTo("/?foo"));
     }
 
+    @Test
+    public void loadFile() throws IOException {
+        // Standard character sets
+        assertEquals(
+                "Iñtërnâtiônàlizætiøn",
+                Util.loadFile(FileUtils.toFile(getClass().getResource("internationalization-utf-8.txt")), StandardCharsets.UTF_8));
+        assertEquals(
+                "Iñtërnâtiônàlizætiøn",
+                Util.loadFile(FileUtils.toFile(getClass().getResource("internationalization-iso-8859-1.txt")), StandardCharsets.ISO_8859_1));
+        assertEquals(
+                "Iñtërnâtiônàlizætiøn",
+                Util.loadFile(FileUtils.toFile(getClass().getResource("internationalization-windows-1252.txt")), Charset.forName("windows-1252")));
+
+        // Malformed input is replaced without throwing an exception
+        assertEquals(
+                "Itrntinliztin",
+                Util.loadFile(FileUtils.toFile(getClass().getResource("internationalization-utf-8.txt")), StandardCharsets.US_ASCII).replaceAll("�", ""));
+        assertEquals(
+                "Itrntinliztin",
+                Util.loadFile(FileUtils.toFile(getClass().getResource("internationalization-iso-8859-1.txt")), StandardCharsets.US_ASCII).replaceAll("�", ""));
+        assertEquals(
+                "Itrntinliztin",
+                Util.loadFile(FileUtils.toFile(getClass().getResource("internationalization-windows-1252.txt")), StandardCharsets.US_ASCII).replaceAll("�", ""));
+
+        // Unmappable character is replaced without throwing an exception
+        assertEquals(
+                "foobar",
+                Util.loadFile(FileUtils.toFile(getClass().getResource("foo-0x81-bar.txt")), Charset.forName("windows-1252")).replaceAll("�", ""));
+
+        // Nonexistent file is returned as an empty string without throwing an exception
+        assertEquals("", Util.loadFile(new File("i-do-not-exist"), StandardCharsets.UTF_8));
+    }
+
     @Test
     public void loadProperties() throws IOException {
 

core/src/test/resources/hudson/foo-0x81-bar.txt

@@ -0,0 +1 @@
+foo�bar

core/src/test/resources/hudson/internationalization-iso-8859-1.txt

@@ -0,0 +1 @@
+I��n穰i�瀝iz誥i�

core/src/test/resources/hudson/internationalization-utf-8.txt

@@ -0,0 +1 @@
+Iñtërnâtiônàlizætiøn

core/src/test/resources/hudson/internationalization-windows-1252.txt

@@ -0,0 +1 @@
+I��n穰i�瀝iz誥i�