8205058: (fs) Files read/writeString should throw CharacterCodingException instead of IOException with an IllegalArgumentException as cause

Reviewed-by: sherman, alanb, lancea

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 +
+        String msg = "malformed input off : " + off + ", length : " + nb;
-                                           ", 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);
     }

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);
             }
 

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);
-            return JLA.newStringNoRepl(ba, cs);
-        } catch (IllegalArgumentException e) {
-            throw new IOException(e);
-        }
     }
 
     /**
@@ -3636,12 +3632,8 @@ public final class Files {
         Objects.requireNonNull(csq);
         Objects.requireNonNull(cs);
 
-        try {
+        byte[] bytes = JLA.getBytesNoRepl(String.valueOf(csq), cs);
-            byte[] bytes = JLA.getBytesNoRepl(String.valueOf(csq), cs);
+        write(path, bytes, options);
-            write(path, bytes, options);
-        } catch (IllegalArgumentException e) {
-            throw new IOException(e);
-        }
 
         return path;
     }

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.

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) null, s, CREATE));
-        checkNullPointerException(() -> Files.writeString(path, (CharSequence) null, OPTION_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) null, s, UTF_8, CREATE));
-        checkNullPointerException(() -> Files.writeString(path, (CharSequence) null, UTF_8, OPTION_CREATE));
+        checkNullPointerException(() -> Files.writeString(path, (CharSequence) null, UTF_8, CREATE));
-        checkNullPointerException(() -> Files.writeString(path, s, (Charset) null, OPTION_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 = str + str;
-            expected = sb.toString();
         } 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) {