Merge "Fix BufferedReader.readLine to not read ahead after '\r'."

2 files changed, 159 insertions, 80 deletions

diff --git a/luni/src/main/java/java/io/BufferedReader.java b/luni/src/main/java/java/io/BufferedReader.java
index 9fba039..f8a36d3 100644
--- a/luni/src/main/java/java/io/BufferedReader.java
+++ b/luni/src/main/java/java/io/BufferedReader.java
@@ -71,6 +71,21 @@ public class BufferedReader extends Reader {
     private int markLimit = -1;
 
     /**
+     * readLine returns a line as soon as it sees '\n' or '\r'. In the latter
+     * case, there might be a following '\n' that should be treated as part of
+     * the same line ending. Both readLine and all read methods are supposed
+     * to skip the '\n' (and clear this field) but only readLine looks for '\r'
+     * and sets it.
+     */
+    private boolean lastWasCR;
+
+    /**
+     * We also need to keep the 'lastWasCR' state for the mark position, in case
+     * we reset to there.
+     */
+    private boolean markedLastWasCR;
+
+    /**
      * Constructs a new {@code BufferedReader}, providing {@code in} with a buffer
      * of 8192 characters.
      *
@@ -195,7 +210,8 @@ public class BufferedReader extends Reader {
         synchronized (lock) {
             checkNotClosed();
             this.markLimit = markLimit;
-            mark = pos;
+            this.mark = pos;
+            this.markedLastWasCR = lastWasCR;
         }
     }
 
@@ -234,12 +250,20 @@ public class BufferedReader extends Reader {
     public int read() throws IOException {
         synchronized (lock) {
             checkNotClosed();
-            /* Are there buffered characters available? */
-            if (pos < end || fillBuf() != -1) {
-                return buf[pos++];
+            int ch = readChar();
+            if (lastWasCR && ch == '\n') {
+                ch = readChar();
             }
-            return -1;
+            lastWasCR = false;
+            return ch;
+        }
+    }
+
+    private int readChar() throws IOException {
+        if (pos < end || fillBuf() != -1) {
+            return buf[pos++];
         }
+        return -1;
     }
 
     /**
@@ -273,12 +297,15 @@ public class BufferedReader extends Reader {
         synchronized (lock) {
             checkNotClosed();
             Arrays.checkOffsetAndCount(buffer.length, offset, length);
+            if (length == 0) {
+                return 0;
+            }
+
+            maybeSwallowLF();
+
             int outstanding = length;
             while (outstanding > 0) {
-
-                /*
-                 * If there are chars in the buffer, grab those first.
-                 */
+                // If there are chars in the buffer, grab those first.
                 int available = end - pos;
                 if (available > 0) {
                     int count = available >= outstanding ? outstanding : available;
@@ -321,7 +348,10 @@ public class BufferedReader extends Reader {
             }
 
             int count = length - outstanding;
-            return (count > 0 || count == length) ? count : -1;
+            if (count > 0) {
+                return count;
+            }
+            return -1;
         }
     }
 
@@ -330,9 +360,16 @@ public class BufferedReader extends Reader {
      * this character is a newline character ("\n"), it is discarded.
      */
     final void chompNewline() throws IOException {
-        if ((pos != end || fillBuf() != -1)
-                && buf[pos] == '\n') {
-            pos++;
+        if ((pos != end || fillBuf() != -1) && buf[pos] == '\n') {
+            ++pos;
+        }
+    }
+
+    // If the last character was CR and the next character is LF, skip it.
+    private void maybeSwallowLF() throws IOException {
+        if (lastWasCR) {
+            chompNewline();
+            lastWasCR = false;
         }
     }
 
@@ -350,77 +387,45 @@ public class BufferedReader extends Reader {
     public String readLine() throws IOException {
         synchronized (lock) {
             checkNotClosed();
-            /* has the underlying stream been exhausted? */
-            if (pos == end && fillBuf() == -1) {
-                return null;
-            }
-            for (int charPos = pos; charPos < end; charPos++) {
-                char ch = buf[charPos];
-                if (ch > '\r') {
-                    continue;
-                }
-                if (ch == '\n') {
-                    String res = new String(buf, pos, charPos - pos);
-                    pos = charPos + 1;
-                    return res;
-                } else if (ch == '\r') {
-                    String res = new String(buf, pos, charPos - pos);
-                    pos = charPos + 1;
-                    if (((pos < end) || (fillBuf() != -1))
-                            && (buf[pos] == '\n')) {
-                        pos++;
-                    }
-                    return res;
+
+            maybeSwallowLF();
+
+            // Do we have a whole line in the buffer?
+            for (int i = pos; i < end; ++i) {
+                char ch = buf[i];
+                if (ch == '\n' || ch == '\r') {
+                    String line = new String(buf, pos, i - pos);
+                    pos = i + 1;
+                    lastWasCR = (ch == '\r');
+                    return line;
                 }
             }
 
-            char eol = '\0';
-            StringBuilder result = new StringBuilder(80);
-            /* Typical Line Length */
-
+            // Accumulate buffers in a StringBuilder until we've read a whole line.
+            StringBuilder result = new StringBuilder(end - pos + 80);
             result.append(buf, pos, end - pos);
             while (true) {
                 pos = end;
-
-                /* Are there buffered characters available? */
-                if (eol == '\n') {
-                    return result.toString();
-                }
-                // attempt to fill buffer
                 if (fillBuf() == -1) {
-                    // characters or null.
-                    return result.length() > 0 || eol != '\0'
-                            ? result.toString()
-                            : null;
+                    // If there's no more input, return what we've read so far, if anything.
+                    return (result.length() > 0) ? result.toString() : null;
                 }
-                for (int charPos = pos; charPos < end; charPos++) {
-                    char c = buf[charPos];
-                    if (eol == '\0') {
-                        if ((c == '\n' || c == '\r')) {
-                            eol = c;
-                        }
-                    } else if (eol == '\r' && c == '\n') {
-                        if (charPos > pos) {
-                            result.append(buf, pos, charPos - pos - 1);
-                        }
-                        pos = charPos + 1;
-                        return result.toString();
-                    } else {
-                        if (charPos > pos) {
-                            result.append(buf, pos, charPos - pos - 1);
-                        }
-                        pos = charPos;
+
+                // Do we have a whole line in the buffer now?
+                for (int i = pos; i < end; ++i) {
+                    char ch = buf[i];
+                    if (ch == '\n' || ch == '\r') {
+                        result.append(buf, pos, i - pos);
+                        pos = i + 1;
+                        lastWasCR = (ch == '\r');
                         return result.toString();
                     }
                 }
-                if (eol == '\0') {
-                    result.append(buf, pos, end - pos);
-                } else {
-                    result.append(buf, pos, end - pos - 1);
-                }
+
+                // Add this whole buffer to the line-in-progress and try again...
+                result.append(buf, pos, end - pos);
             }
         }
-
     }
 
     /**
@@ -459,26 +464,23 @@ public class BufferedReader extends Reader {
             if (mark == -1) {
                 throw new IOException("Invalid mark");
             }
-            pos = mark;
+            this.pos = mark;
+            this.lastWasCR = this.markedLastWasCR;
         }
     }
 
     /**
-     * Skips {@code charCount} chars in this stream. Subsequent calls to
+     * Skips at most {@code charCount} chars in this stream. Subsequent calls to
      * {@code read} will not return these chars unless {@code reset} is
      * used.
      *
      * <p>Skipping characters may invalidate a mark if {@code markLimit}
      * is surpassed.
      *
-     * @param charCount the maximum number of characters to skip.
      * @return the number of characters actually skipped.
      * @throws IllegalArgumentException if {@code charCount < 0}.
      * @throws IOException
      *             if this reader is closed or some other I/O error occurs.
-     * @see #mark(int)
-     * @see #markSupported()
-     * @see #reset()
      */
     @Override
     public long skip(long charCount) throws IOException {
@@ -487,9 +489,6 @@ public class BufferedReader extends Reader {
         }
         synchronized (lock) {
             checkNotClosed();
-            if (charCount < 1) {
-                return 0;
-            }
             if (end - pos >= charCount) {
                 pos += charCount;
                 return charCount;
diff --git a/luni/src/test/java/libcore/java/io/OldBufferedReaderTest.java b/luni/src/test/java/libcore/java/io/OldBufferedReaderTest.java
index 214c1b2..986c672 100644
--- a/luni/src/test/java/libcore/java/io/OldBufferedReaderTest.java
+++ b/luni/src/test/java/libcore/java/io/OldBufferedReaderTest.java
@@ -19,8 +19,13 @@ package libcore.java.io;
 
 import java.io.BufferedReader;
 import java.io.CharArrayReader;
+import java.io.InputStreamReader;
 import java.io.IOException;
+import java.io.OutputStreamWriter;
+import java.io.PipedInputStream;
+import java.io.PipedOutputStream;
 import java.io.PipedReader;
+import java.io.PrintWriter;
 import java.io.Reader;
 import java.io.StringReader;
 import tests.support.Support_ASimpleReader;
@@ -318,4 +323,79 @@ public class OldBufferedReaderTest extends junit.framework.TestCase {
         } catch (Exception e) {
         }
     }
+
+    public void test_readLine_all_line_endings() throws Exception {
+        BufferedReader r = new BufferedReader(new StringReader("1\r2\n3\r\n4"));
+        assertEquals("1", r.readLine());
+        assertEquals("2", r.readLine());
+        assertEquals("3", r.readLine());
+        assertEquals("4", r.readLine());
+        assertNull(r.readLine());
+    }
+
+    public void test_readLine_interaction_with_read() throws Exception {
+        BufferedReader r = new BufferedReader(new StringReader("1\r\n2"));
+        assertEquals('1', r.read());
+        assertEquals('\r', r.read());
+        assertEquals("", r.readLine()); // The '\r' we read() didn't count.
+        assertEquals("2", r.readLine());
+        assertNull(r.readLine());
+    }
+
+    public void test_readLine_interaction_with_array_read_1() throws Exception {
+        BufferedReader r = new BufferedReader(new StringReader("1\r\n2"));
+        assertEquals(2, r.read(new char[2], 0, 2));
+        assertEquals("", r.readLine()); // The '\r' we read() didn't count.
+        assertEquals("2", r.readLine());
+        assertNull(r.readLine());
+    }
+
+    public void test_readLine_interaction_with_array_read_2() throws Exception {
+        BufferedReader r = new BufferedReader(new StringReader("1\r\n2"));
+        assertEquals("1", r.readLine());
+        char[] chars = new char[1];
+        assertEquals(1, r.read(chars, 0, 1)); // This read skips the '\n'.
+        assertEquals('2', chars[0]);
+        assertNull(r.readLine());
+    }
+
+    public void test_readLine_interaction_with_skip() throws Exception {
+        BufferedReader r = new BufferedReader(new StringReader("1\r\n2"));
+        assertEquals(2, r.skip(2));
+        assertEquals("", r.readLine()); // The '\r' we skip()ed didn't count.
+        assertEquals("2", r.readLine());
+        assertNull(r.readLine());
+    }
+
+    public void test_readLine_interaction_with_mark_and_reset() throws Exception {
+        BufferedReader r = new BufferedReader(new StringReader("1\r\n2\n3"));
+        assertEquals("1", r.readLine());
+        r.mark(256);
+        assertEquals('2', r.read()); // This read skips the '\n'.
+        assertEquals("", r.readLine());
+        r.reset(); // Now we're back half-way through the "\r\n".
+        assertEquals("2", r.readLine());
+        assertEquals("3", r.readLine());
+        assertNull(r.readLine());
+    }
+
+    public void test_8778372() throws Exception {
+        final PipedInputStream pis = new PipedInputStream();
+        final PipedOutputStream pos = new PipedOutputStream(pis);
+        final Thread t = new Thread() {
+          @Override public void run() {
+              PrintWriter pw = new PrintWriter(new OutputStreamWriter(pos));
+              pw.print("hello, world\r");
+              pw.flush();
+              try {
+                  Thread.sleep(2*60*1000);
+              } catch (InterruptedException ex) {
+                  fail();
+              }
+            }
+        };
+        t.start();
+        BufferedReader br = new BufferedReader(new InputStreamReader(pis));
+        assertEquals("hello, world", br.readLine());
+    }
 }