diff --git a/core/src/main/java/hudson/Util.java b/core/src/main/java/hudson/Util.java index 07254e2b77..02bb6180de 100644 --- a/core/src/main/java/hudson/Util.java +++ b/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); diff --git a/core/src/test/java/hudson/UtilTest.java b/core/src/test/java/hudson/UtilTest.java index 2f69cf2013..d4ecd94f1a 100644 --- a/core/src/test/java/hudson/UtilTest.java +++ b/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 { diff --git a/core/src/test/resources/hudson/foo-0x81-bar.txt b/core/src/test/resources/hudson/foo-0x81-bar.txt new file mode 100644 index 0000000000..2d8482b481 --- /dev/null +++ b/core/src/test/resources/hudson/foo-0x81-bar.txt @@ -0,0 +1 @@ +foo�bar \ No newline at end of file diff --git a/core/src/test/resources/hudson/internationalization-iso-8859-1.txt b/core/src/test/resources/hudson/internationalization-iso-8859-1.txt new file mode 100644 index 0000000000..279ae949f5 --- /dev/null +++ b/core/src/test/resources/hudson/internationalization-iso-8859-1.txt @@ -0,0 +1 @@ +Iñtërnâtiônàlizætiøn \ No newline at end of file diff --git a/core/src/test/resources/hudson/internationalization-utf-8.txt b/core/src/test/resources/hudson/internationalization-utf-8.txt new file mode 100644 index 0000000000..7ddfa871f1 --- /dev/null +++ b/core/src/test/resources/hudson/internationalization-utf-8.txt @@ -0,0 +1 @@ +Iñtërnâtiônàlizætiøn \ No newline at end of file diff --git a/core/src/test/resources/hudson/internationalization-windows-1252.txt b/core/src/test/resources/hudson/internationalization-windows-1252.txt new file mode 100644 index 0000000000..279ae949f5 --- /dev/null +++ b/core/src/test/resources/hudson/internationalization-windows-1252.txt @@ -0,0 +1 @@ +Iñtërnâtiônàlizætiøn \ No newline at end of file