From e5ac84c7b0b30d1511d0b3c54b3fc1b04f3370cd Mon Sep 17 00:00:00 2001 From: Joe Wang Date: Wed, 27 Jun 2018 09:31:51 -0700 Subject: [PATCH] 8205058: (fs) Files read/writeString should throw CharacterCodingException instead of IOException with an IllegalArgumentException as cause Reviewed-by: sherman, alanb, lancea --- .../share/classes/java/lang/StringCoding.java | 57 ++++++++++++++++--- .../share/classes/java/lang/System.java | 5 +- .../share/classes/java/nio/file/Files.java | 14 +---- .../jdk/internal/misc/JavaLangAccess.java | 11 ++-- .../java/nio/file/Files/ReadWriteString.java | 42 +++++++------- 5 files changed, 80 insertions(+), 49 deletions(-) diff --git a/src/java.base/share/classes/java/lang/StringCoding.java b/src/java.base/share/classes/java/lang/StringCoding.java index d121a802638..315e9a81151 100644 --- a/src/java.base/share/classes/java/lang/StringCoding.java +++ b/src/java.base/share/classes/java/lang/StringCoding.java @@ -36,6 +36,8 @@ import java.nio.charset.CharacterCodingException; import java.nio.charset.CoderResult; import java.nio.charset.CodingErrorAction; import java.nio.charset.IllegalCharsetNameException; +import java.nio.charset.MalformedInputException; +import java.nio.charset.UnmappableCharacterException; import java.nio.charset.UnsupportedCharsetException; import java.util.Arrays; import jdk.internal.HotSpotIntrinsicCandidate; @@ -607,7 +609,7 @@ class StringCoding { dp = dp + ret; if (ret != len) { if (!doReplace) { - throwMalformed(sp, 1); + throwUnmappable(sp, 1); } char c = StringUTF16.getChar(val, sp++); if (Character.isHighSurrogate(c) && sp < sl && @@ -679,8 +681,8 @@ class StringCoding { } private static void throwMalformed(int off, int nb) { - throw new IllegalArgumentException("malformed input off : " + off + - ", length : " + nb); + String msg = "malformed input off : " + off + ", length : " + nb; + throw new IllegalArgumentException(msg, new MalformedInputException(nb)); } private static void throwMalformed(byte[] val) { @@ -689,6 +691,17 @@ class StringCoding { throwMalformed(dp, 1); } + private static void throwUnmappable(int off, int nb) { + String msg = "malformed input off : " + off + ", length : " + nb; + throw new IllegalArgumentException(msg, new UnmappableCharacterException(nb)); + } + + private static void throwUnmappable(byte[] val) { + int dp = 0; + while (dp < val.length && val[dp] >=0) { dp++; } + throwUnmappable(dp, 1); + } + private static char repl = '\ufffd'; private static Result decodeUTF8(byte[] src, int sp, int len, boolean doReplace) { @@ -919,7 +932,7 @@ class StringCoding { if (doReplace) { dst[dp++] = '?'; } else { - throwMalformed(sp - 1, 1); // or 2, does not matter here + throwUnmappable(sp - 1, 1); // or 2, does not matter here } } else { dst[dp++] = (byte)(0xf0 | ((uc >> 18))); @@ -972,7 +985,20 @@ class StringCoding { return new String(StringLatin1.inflate(src, 0, src.length), UTF16); } - static String newStringNoRepl(byte[] src, Charset cs) { + static String newStringNoRepl(byte[] src, Charset cs) throws CharacterCodingException { + try { + return newStringNoRepl1(src, cs); + } catch (IllegalArgumentException e) { + //newStringNoRepl1 throws IAE with MalformedInputException or CCE as the cause + Throwable cause = e.getCause(); + if (cause instanceof MalformedInputException) { + throw (MalformedInputException)cause; + } + throw (CharacterCodingException)cause; + } + } + + static String newStringNoRepl1(byte[] src, Charset cs) { if (cs == UTF_8) { if (COMPACT_STRINGS && isASCII(src)) return new String(src, LATIN1); @@ -1023,9 +1049,22 @@ class StringCoding { } /* - * Throws iae, instead of replacing, if unmappable. + * Throws CCE, instead of replacing, if unmappable. */ - static byte[] getBytesNoRepl(String s, Charset cs) { + static byte[] getBytesNoRepl(String s, Charset cs) throws CharacterCodingException { + try { + return getBytesNoRepl1(s, cs); + } catch (IllegalArgumentException e) { + //getBytesNoRepl1 throws IAE with UnmappableCharacterException or CCE as the cause + Throwable cause = e.getCause(); + if (cause instanceof UnmappableCharacterException) { + throw (UnmappableCharacterException)cause; + } + throw (CharacterCodingException)cause; + } + } + + static byte[] getBytesNoRepl1(String s, Charset cs) { byte[] val = s.value(); byte coder = s.coder(); if (cs == UTF_8) { @@ -1045,7 +1084,7 @@ class StringCoding { if (isASCII(val)) { return val; } else { - throwMalformed(val); + throwUnmappable(val); } } } @@ -1083,7 +1122,7 @@ class StringCoding { if (!cr.isUnderflow()) cr.throwException(); } catch (CharacterCodingException x) { - throw new Error(x); + throw new IllegalArgumentException(x); } return safeTrim(ba, bb.position(), isTrusted); } diff --git a/src/java.base/share/classes/java/lang/System.java b/src/java.base/share/classes/java/lang/System.java index c1dcff2e4ed..e4ce3901dfb 100644 --- a/src/java.base/share/classes/java/lang/System.java +++ b/src/java.base/share/classes/java/lang/System.java @@ -41,6 +41,7 @@ import java.lang.reflect.Executable; import java.lang.reflect.Method; import java.lang.reflect.Modifier; import java.net.URI; +import java.nio.charset.CharacterCodingException; import java.security.AccessControlContext; import java.security.ProtectionDomain; import java.security.AccessController; @@ -2184,11 +2185,11 @@ public final class System { return ModuleLayer.layers(loader); } - public String newStringNoRepl(byte[] bytes, Charset cs) { + public String newStringNoRepl(byte[] bytes, Charset cs) throws CharacterCodingException { return StringCoding.newStringNoRepl(bytes, cs); } - public byte[] getBytesNoRepl(String s, Charset cs) { + public byte[] getBytesNoRepl(String s, Charset cs) throws CharacterCodingException { return StringCoding.getBytesNoRepl(s, cs); } diff --git a/src/java.base/share/classes/java/nio/file/Files.java b/src/java.base/share/classes/java/nio/file/Files.java index 7176fe1ff0d..930e3e85994 100644 --- a/src/java.base/share/classes/java/nio/file/Files.java +++ b/src/java.base/share/classes/java/nio/file/Files.java @@ -3281,11 +3281,7 @@ public final class Files { Objects.requireNonNull(cs); byte[] ba = readAllBytes(path); - try { - return JLA.newStringNoRepl(ba, cs); - } catch (IllegalArgumentException e) { - throw new IOException(e); - } + return JLA.newStringNoRepl(ba, cs); } /** @@ -3636,12 +3632,8 @@ public final class Files { Objects.requireNonNull(csq); Objects.requireNonNull(cs); - try { - byte[] bytes = JLA.getBytesNoRepl(String.valueOf(csq), cs); - write(path, bytes, options); - } catch (IllegalArgumentException e) { - throw new IOException(e); - } + byte[] bytes = JLA.getBytesNoRepl(String.valueOf(csq), cs); + write(path, bytes, options); return path; } diff --git a/src/java.base/share/classes/jdk/internal/misc/JavaLangAccess.java b/src/java.base/share/classes/jdk/internal/misc/JavaLangAccess.java index 3fc7e7ea3eb..2fb19ecd513 100644 --- a/src/java.base/share/classes/jdk/internal/misc/JavaLangAccess.java +++ b/src/java.base/share/classes/jdk/internal/misc/JavaLangAccess.java @@ -30,6 +30,7 @@ import java.lang.module.ModuleDescriptor; import java.lang.reflect.Executable; import java.lang.reflect.Method; import java.net.URI; +import java.nio.charset.CharacterCodingException; import java.nio.charset.Charset; import java.security.AccessControlContext; import java.security.ProtectionDomain; @@ -266,9 +267,9 @@ public interface JavaLangAccess { * @param bytes the byte array source * @param cs the Charset * @return the newly created string - * @throws IllegalArgumentException for malformed or unmappable bytes + * @throws CharacterCodingException for malformed or unmappable bytes */ - String newStringNoRepl(byte[] bytes, Charset cs); + String newStringNoRepl(byte[] bytes, Charset cs) throws CharacterCodingException; /** * Encode the given string into a sequence of bytes using the specified Charset. @@ -276,15 +277,15 @@ public interface JavaLangAccess { * This method avoids copying the String's internal representation if the input * is ASCII. * - * This method throws IllegalArgumentException instead of replacing when + * This method throws CharacterCodingException instead of replacing when * malformed input or unmappable characters are encountered. * * @param s the string to encode * @param cs the charset * @return the encoded bytes - * @throws IllegalArgumentException for malformed input or unmappable characters + * @throws CharacterCodingException for malformed input or unmappable characters */ - byte[] getBytesNoRepl(String s, Charset cs); + byte[] getBytesNoRepl(String s, Charset cs) throws CharacterCodingException; /** * Returns a new string by decoding from the given utf8 bytes array. diff --git a/test/jdk/java/nio/file/Files/ReadWriteString.java b/test/jdk/java/nio/file/Files/ReadWriteString.java index 02724c862cc..1dd1c11c80c 100644 --- a/test/jdk/java/nio/file/Files/ReadWriteString.java +++ b/test/jdk/java/nio/file/Files/ReadWriteString.java @@ -24,6 +24,8 @@ import java.io.ByteArrayOutputStream; import java.io.IOException; import java.nio.charset.Charset; +import java.nio.charset.MalformedInputException; +import java.nio.charset.UnmappableCharacterException; import static java.nio.charset.StandardCharsets.US_ASCII; import static java.nio.charset.StandardCharsets.ISO_8859_1; import static java.nio.charset.StandardCharsets.UTF_8; @@ -31,8 +33,8 @@ import java.nio.file.Files; import java.nio.file.OpenOption; import java.nio.file.Path; import java.nio.file.Paths; -import java.nio.file.StandardOpenOption; import static java.nio.file.StandardOpenOption.APPEND; +import static java.nio.file.StandardOpenOption.CREATE; import java.util.Random; import java.util.concurrent.Callable; import static org.testng.Assert.assertTrue; @@ -43,7 +45,7 @@ import org.testng.annotations.DataProvider; import org.testng.annotations.Test; /* @test - * @bug 8201276 + * @bug 8201276 8205058 * @build ReadWriteString PassThroughFileSystem * @run testng ReadWriteString * @summary Unit test for methods for Files readString and write methods. @@ -52,7 +54,6 @@ import org.testng.annotations.Test; @Test(groups = "readwrite") public class ReadWriteString { - private static final OpenOption OPTION_CREATE = StandardOpenOption.CREATE; // data for text files private static final String EN_STRING = "The quick brown fox jumps over the lazy dog"; private static final String JA_STRING = "\u65e5\u672c\u8a9e\u6587\u5b57\u5217"; @@ -74,7 +75,8 @@ public class ReadWriteString { baos.write(str2.getBytes()); return baos.toByteArray(); } catch (IOException ex) { - return null; //shouldn't happen + // in case it happens, fail the test + throw new RuntimeException(ex); } } @@ -125,20 +127,20 @@ public class ReadWriteString { */ @Test public void testNulls() { - Path path = Paths.get("."); + Path path = Paths.get("foo"); String s = "abc"; checkNullPointerException(() -> Files.readString((Path) null)); checkNullPointerException(() -> Files.readString((Path) null, UTF_8)); checkNullPointerException(() -> Files.readString(path, (Charset) null)); - checkNullPointerException(() -> Files.writeString((Path) null, s, OPTION_CREATE)); - checkNullPointerException(() -> Files.writeString(path, (CharSequence) null, OPTION_CREATE)); + checkNullPointerException(() -> Files.writeString((Path) null, s, CREATE)); + checkNullPointerException(() -> Files.writeString(path, (CharSequence) null, CREATE)); checkNullPointerException(() -> Files.writeString(path, s, (OpenOption[]) null)); - checkNullPointerException(() -> Files.writeString((Path) null, s, UTF_8, OPTION_CREATE)); - checkNullPointerException(() -> Files.writeString(path, (CharSequence) null, UTF_8, OPTION_CREATE)); - checkNullPointerException(() -> Files.writeString(path, s, (Charset) null, OPTION_CREATE)); + checkNullPointerException(() -> Files.writeString((Path) null, s, UTF_8, CREATE)); + checkNullPointerException(() -> Files.writeString(path, (CharSequence) null, UTF_8, CREATE)); + checkNullPointerException(() -> Files.writeString(path, s, (Charset) null, CREATE)); checkNullPointerException(() -> Files.writeString(path, s, UTF_8, (OpenOption[]) null)); } @@ -168,13 +170,13 @@ public class ReadWriteString { * @param cs the Charset * @throws IOException if the input is malformed */ - @Test(dataProvider = "malformedWrite", expectedExceptions = IOException.class) + @Test(dataProvider = "malformedWrite", expectedExceptions = UnmappableCharacterException.class) public void testMalformedWrite(Path path, String s, Charset cs) throws IOException { path.toFile().deleteOnExit(); if (cs == null) { - Files.writeString(path, s, OPTION_CREATE); + Files.writeString(path, s, CREATE); } else { - Files.writeString(path, s, cs, OPTION_CREATE); + Files.writeString(path, s, cs, CREATE); } } @@ -188,11 +190,11 @@ public class ReadWriteString { * @param csRead the Charset to use for reading the file * @throws IOException when the Charset used for reading the file is incorrect */ - @Test(dataProvider = "illegalInput", expectedExceptions = IOException.class) + @Test(dataProvider = "illegalInput", expectedExceptions = MalformedInputException.class) public void testMalformedRead(Path path, byte[] data, Charset csWrite, Charset csRead) throws IOException { path.toFile().deleteOnExit(); String temp = new String(data, csWrite); - Files.writeString(path, temp, csWrite, OPTION_CREATE); + Files.writeString(path, temp, csWrite, CREATE); String s; if (csRead == null) { s = Files.readString(path); @@ -212,7 +214,6 @@ public class ReadWriteString { } private void testReadWrite(int size, Charset cs, boolean append) throws IOException { - StringBuilder sb = new StringBuilder(size); String expected; String str = generateString(size); Path result; @@ -235,8 +236,7 @@ public class ReadWriteString { if (append) { - sb.append(str).append(str); - expected = sb.toString(); + expected = str + str; } else { expected = str; } @@ -247,14 +247,12 @@ public class ReadWriteString { } else { read = Files.readString(result, cs); } - //System.out.println("chars read: " + read.length()); - //System.out.println(read); - //System.out.println("---end---"); + assertTrue(read.equals(expected), "String read not the same as written"); } static final char[] CHARS = "abcdefghijklmnopqrstuvwxyz \r\n".toCharArray(); - StringBuilder sb = new StringBuilder(512); + StringBuilder sb = new StringBuilder(1024 << 4); Random random = new Random(); private String generateString(int size) {