Reject malformed UTF-8 sequences in read_ut8_sequence

src/main/java/com/amazon/ion/impl/IonReaderTextRawTokensX.java

@@ -2450,26 +2450,52 @@ private final int read_ut8_sequence(int c) throws IOException
         case 1:
             break;
         case 2:
-            b2 = read_char();
+            b2 = read_ut8_continuation_byte();
             c = IonUTF8.twoByteScalar(c, b2);
+            if (c < 0x80) {
+                error("overlong UTF8 sequence");
+            }
             break;
         case 3:
-            b2 = read_char();
-            b3 = read_char();
+            b2 = read_ut8_continuation_byte();
+            b3 = read_ut8_continuation_byte();
             c = IonUTF8.threeByteScalar(c, b2, b3);
+            if (c < 0x800) {
+                error("overlong UTF8 sequence");
+            }
             break;
         case 4:
-            b2 = read_char();
-            b3 = read_char();
-            b4 = read_char();
+            b2 = read_ut8_continuation_byte();
+            b3 = read_ut8_continuation_byte();
+            b4 = read_ut8_continuation_byte();
             c = IonUTF8.fourByteScalar(c, b2, b3, b4);
+            if (c < 0x10000) {
+                error("overlong UTF8 sequence");
+            }
             break;
         default:
             error("invalid UTF8 starting byte");
         }
         return c;
     }
 
+    /**
+     * Reads the next byte of a multi-byte UTF8 sequence, which must be a
+     * continuation byte. Without this check a truncated sequence silently
+     * consumes whatever follows it, including a token terminator.
+     */
+    private final int read_ut8_continuation_byte() throws IOException
+    {
+        int b = read_char();
+        if (b == UnifiedInputStreamX.EOF) {
+            unexpected_eof();
+        }
+        if (!IonUTF8.isContinueByteUTF8(b)) {
+            error("invalid UTF8 continuation byte");
+        }
+        return b;
+    }
+
     private void skip_over_blob(SavePoint sp) throws IOException
     {
         int c;

src/test/java/com/amazon/ion/IonExceptionTest.java

@@ -1,18 +1,5 @@
-/*
- * Copyright 2007-2019 Amazon.com, Inc. or its affiliates. All Rights Reserved.
- *
- * Licensed under the Apache License, Version 2.0 (the "License").
- * You may not use this file except in compliance with the License.
- * A copy of the License is located at
- *
- *     http://www.apache.org/licenses/LICENSE-2.0
- *
- * or in the "license" file accompanying this file. This file is distributed
- * on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either
- * express or implied. See the License for the specific language governing
- * permissions and limitations under the License.
- */
-
+// Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
+// SPDX-License-Identifier: Apache-2.0
 package com.amazon.ion;
 
 import static org.junit.Assert.assertEquals;
@@ -114,7 +101,7 @@ public void testCauseOfWrongEncoding() {
             IonSystemBuilder.standard().build().newLoader().load(bytes_input);
         } catch (IonException e) {
             // The exception should be caught here
-            assertEquals(e.getMessage(), "Invalid encoding: encountered non-Unicode character.");
+            assertEquals(e.getMessage(), "invalid UTF8 continuation byte at line 1 offset 5");
             return;
         } catch (Exception ignore) {}
 

src/test/java/com/amazon/ion/impl/IonReaderTextUtf8Test.java

@@ -0,0 +1,97 @@
+// Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
+// SPDX-License-Identifier: Apache-2.0
+package com.amazon.ion.impl;
+
+import com.amazon.ion.IonException;
+import com.amazon.ion.IonReader;
+import com.amazon.ion.IonType;
+import com.amazon.ion.system.IonReaderBuilder;
+import org.junit.Test;
+
+import java.io.ByteArrayOutputStream;
+import java.io.IOException;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertNull;
+import static org.junit.Assert.assertThrows;
+
+public class IonReaderTextUtf8Test {
+
+    /**
+     * Builds Ion text from a mix of ASCII fragments and raw byte values, so that
+     * byte sequences that are not valid UTF-8 can be fed to the text reader.
+     */
+    private static byte[] ionText(Object... parts) throws IOException {
+        ByteArrayOutputStream out = new ByteArrayOutputStream();
+        for (Object part : parts) {
+            if (part instanceof String) {
+                out.write(((String) part).getBytes("US-ASCII"));
+            } else {
+                out.write((Integer) part);
+            }
+        }
+        return out.toByteArray();
+    }
+
+    private static void readFully(IonReader reader) {
+        IonType type;
+        while ((type = reader.next()) != null) {
+            switch (type) {
+                case LIST:
+                case SEXP:
+                case STRUCT:
+                    reader.stepIn();
+                    readFully(reader);
+                    reader.stepOut();
+                    break;
+                case STRING:
+                case SYMBOL:
+                    reader.stringValue();
+                    break;
+                default:
+                    break;
+            }
+        }
+    }
+
+    private static void expectRejected(byte[] data) {
+        assertThrows(IonException.class, () -> readFully(IonReaderBuilder.standard().build(data)));
+    }
+
+    private static String readOneString(byte[] data) {
+        IonReader reader = IonReaderBuilder.standard().build(data);
+        reader.next();
+        String value = reader.stringValue();
+        assertNull(reader.next());
+        return value;
+    }
+
+    @Test
+    public void truncatedSequenceDoesNotConsumeTheTokenTerminator() throws Exception {
+        // Without continuation byte validation the 0xC3 lead byte absorbs the closing
+        // quote, so this three element list is read as a two element list.
+        expectRejected(ionText("[\"", 0xC3, "\"", "x", "\",\"c\"]"));
+        expectRejected(ionText("['", 0xC3, "','b']"));
+    }
+
+    @Test
+    public void truncatedSequenceAtEndOfInputIsRejected() throws Exception {
+        expectRejected(ionText("\"", 0xE0, 0xA0));
+    }
+
+    @Test
+    public void overlongSequencesAreRejected() throws Exception {
+        expectRejected(ionText("[\"", 0xC0, 0x80, "\"]"));             // NUL as two bytes
+        expectRejected(ionText("[\"", 0xC0, 0xA2, "\"]"));             // '"' as two bytes
+        expectRejected(ionText("[\"", 0xE0, 0x80, 0xAF, "\"]"));       // '/' as three bytes
+        expectRejected(ionText("[\"", 0xF0, 0x80, 0x80, 0xAF, "\"]")); // '/' as four bytes
+    }
+
+    @Test
+    public void wellFormedSequencesAreStillAccepted() throws Exception {
+        assertEquals("\u00e9", readOneString(ionText("\"", 0xC3, 0xA9, "\"")));
+        assertEquals("\u20ac", readOneString(ionText("\"", 0xE2, 0x82, 0xAC, "\"")));
+        assertEquals("\ud83d\ude02", readOneString(ionText("\"", 0xF0, 0x9F, 0x98, 0x82, "\"")));
+        assertEquals("abc", readOneString(ionText("\"abc\"")));
+    }
+}