csv: remove old workaround in UTF8Reader (#546)

Author: pjfanning

csv/src/main/java/com/fasterxml/jackson/dataformat/csv/impl/UTF8Reader.java

@@ -25,6 +25,16 @@ public final class UTF8Reader
 
     private byte[] _inputBuffer;
 
+    /**
+     * Flag set to indicate {@code inputBuffer} is read-only, and its
+     * content should not be modified. This is the case when caller
+     * has passed in a buffer of contents already read, instead of Jackson
+     * allocating read buffer.
+     *
+     * @since 2.19
+     */
+    private final boolean _inputBufferReadOnly;
+
     /**
      * Pointer to the next available byte (if any), iff less than
      * <code>mByteBufferEnd</code>
@@ -73,7 +83,10 @@ public UTF8Reader(IOContext ctxt, InputStream in, boolean autoClose,
         _inputBuffer = buf;
         _inputPtr = ptr;
         _inputEnd = ptr+len;
-        _autoClose = autoClose; 
+        _autoClose = autoClose;
+        // Unmodifiable if there is no stream to actually read from
+        // (ideally caller should pass explicitly)
+        _inputBufferReadOnly = (in == null);
     }
 
     public UTF8Reader(IOContext ctxt, byte[] buf, int ptr, int len)
@@ -85,6 +98,8 @@ public UTF8Reader(IOContext ctxt, byte[] buf, int ptr, int len)
         _inputPtr = ptr;
         _inputEnd = ptr+len;
         _autoClose = true;
+        // This is the case when we have a buffer of contents already read
+        _inputBufferReadOnly = true;
     }
 
     public UTF8Reader(IOContext ctxt, InputStream in, boolean autoClose)
@@ -96,15 +111,8 @@ public UTF8Reader(IOContext ctxt, InputStream in, boolean autoClose)
         _inputPtr = 0;
         _inputEnd = 0;
         _autoClose = autoClose; 
-    }
-
-    /**
-     * Method that can be used to see if we can actually modify the
-     * underlying buffer. This is the case if we are managing the buffer,
-     * but not if it was just given to us.
-     */
-    protected final boolean canModifyBuffer() {
-        return (_ioContext != null);
+        // Buffer allocated above, modifiable as needed
+        _inputBufferReadOnly = false;
     }
 
     /*
@@ -400,27 +408,17 @@ private boolean loadMore(int available) throws IOException
     {
         _byteCount += (_inputEnd - available);
 
-        // Bytes that need to be moved to the beginning of buffer?
         if (available > 0) {
+            // Should we move bytes to the beginning of buffer?
             if (_inputPtr > 0) {
-                if (!canModifyBuffer()) {
-                    // 15-Aug-2022, tatu: Occurs (only) if we have half-decoded UTF-8
-                    //     characters; uncovered by:
-                    //
-                    // https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=50036
-                    //
-                    // and need to be reported as IOException
-                    if (_inputSource == null) {
-                        throw new IOException(String.format(
-"End-of-input after first %d byte(s) of a UTF-8 character: needed at least one more",
-available));
+                // Can only do so if buffer mutable
+                if (!_inputBufferReadOnly) {
+                    for (int i = 0; i < available; ++i) {
+                        _inputBuffer[i] = _inputBuffer[_inputPtr+i];
                     }
+                    _inputPtr = 0;
+                    _inputEnd = available;
                 }
-                for (int i = 0; i < available; ++i) {
-                    _inputBuffer[i] = _inputBuffer[_inputPtr+i];
-                }
-                _inputPtr = 0;
-                _inputEnd = available;
             }
         } else {
             // Ok; here we can actually reasonably expect an EOF, so let's do a separate read right away:

csv/src/test/java/com/fasterxml/jackson/dataformat/csv/deser/FuzzCSVReadTest.java

@@ -6,16 +6,19 @@
 import org.junit.jupiter.api.Test;
 
 import com.fasterxml.jackson.dataformat.csv.CsvMapper;
+import com.fasterxml.jackson.dataformat.csv.ModuleTestBase;
 
 import static org.assertj.core.api.Assertions.fail;
+import static org.junit.jupiter.api.Assertions.assertArrayEquals;
 
 /**
  * Collection of OSS-Fuzz found issues for CSV format module.
  */
-public class FuzzCSVReadTest extends StreamingCSVReadTest
+public class FuzzCSVReadTest extends ModuleTestBase
 {
     private final CsvMapper CSV_MAPPER = mapperForCsv();
     private final byte[] INPUT = new byte[] { 0x20, (byte) 0xCD };
+    private final byte[] CLONED = INPUT.clone();
 
     // https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=50036
     @Test
@@ -25,8 +28,9 @@ public void testUTF8Decoding50036() throws Exception
             CSV_MAPPER.readTree(INPUT);
             fail("Should not pass");
         } catch (IOException e) {
-            verifyException(e, "End-of-input after first 1 byte");
-            verifyException(e, "of a UTF-8 character");
+            verifyException(e, "Unexpected EOF in the middle of a multi-byte UTF-8 character");
+            // check input was not modified
+            assertArrayEquals(CLONED, INPUT);
         }
     }
 
@@ -38,6 +42,8 @@ public void testUTF8Decoding50036Stream() throws Exception
             fail("Should not pass");
         } catch (IOException e) {
             verifyException(e, "Unexpected EOF in the middle of a multi-byte UTF-8 character");
+            // check input was not modified
+            assertArrayEquals(CLONED, INPUT);
         }
     }
 }

…mat/csv/tofix/UnicodeCSVRead497Test.java …mat/csv/deser/UnicodeCSVRead497Test.javacsv/src/test/java/com/fasterxml/jackson/dataformat/csv/tofix/UnicodeCSVRead497Test.java renamed to csv/src/test/java/com/fasterxml/jackson/dataformat/csv/deser/UnicodeCSVRead497Test.java csv/src/test/java/com/fasterxml/jackson/dataformat/csv/tofix/UnicodeCSVRead497Test.java renamed to csv/src/test/java/com/fasterxml/jackson/dataformat/csv/deser/UnicodeCSVRead497Test.java

@@ -1,4 +1,4 @@
-package com.fasterxml.jackson.dataformat.csv.tofix;
+package com.fasterxml.jackson.dataformat.csv.deser;
 
 import java.io.ByteArrayInputStream;
 import java.nio.charset.StandardCharsets;
@@ -8,7 +8,6 @@
 import com.fasterxml.jackson.databind.JsonNode;
 import com.fasterxml.jackson.dataformat.csv.CsvMapper;
 import com.fasterxml.jackson.dataformat.csv.ModuleTestBase;
-import com.fasterxml.jackson.dataformat.csv.testutil.failure.JacksonTestFailureExpected;
 
 import static org.junit.jupiter.api.Assertions.*;
 
@@ -18,7 +17,6 @@ public class UnicodeCSVRead497Test extends ModuleTestBase
     private final CsvMapper MAPPER = mapperForCsv();
 
     // [dataformats-text#497]
-    @JacksonTestFailureExpected
     @Test
     public void testUnicodeAtEnd() throws Exception
     {
@@ -35,12 +33,15 @@ public void testUnicodeAtEnd() throws Exception
     public void testUnicodeAtEnd2() throws Exception
     {
         String doc = buildTestString2();
+        final byte[] bytes = doc.getBytes(StandardCharsets.UTF_8);
         JsonNode o = MAPPER.reader() //.with(schema)
-                .readTree(doc.getBytes(StandardCharsets.UTF_8));
+                .readTree(bytes);
         assertNotNull(o);
         assertTrue(o.isArray());
         assertEquals(1, o.size());
         assertEquals(o.get(0).textValue(), doc);
+        // check byte array was not modified
+        assertArrayEquals(doc.getBytes(StandardCharsets.UTF_8), bytes);
     }
 
     @Test