Repository: netty/netty
Issue: Bug in ByteBufInputStream.readLine()
Labels: defect

The problem is on lines 201-202.  This will fail with a StringIndexOutOfBoundsException if the line contains just "\r\n".  The length() check for > 0 needs to be done each time through the loop.


Issue comments:
This is also broken in Netty 3.6, line 188.

Thanks.. Let me look into it

Am 17.07.2013 um 18:04 schrieb ssilverman notifications@github.com:

> The problem is on line 188. This will fail with a StringIndexOutOfBoundsException if the line contains just "\r\n". The length() check for > 0 needs to be done each time through the loop.
> 
> —
> Reply to this email directly or view it on GitHub.

Not really sure what that loop is for -- if I send the string \r\r\r\n -- why wouldn't the result be \r\r?

If one subscribes to the "\r"|"\n"|"\r\n" linefeed model, I would expect "\r\r\r\n" to have three lines.

I just wrote this quickly (it's untested).  It should be close to what is needed.

```
public String readLine() {
    StringBuilder lineBuf = new StringBuilder();

    loop: while (true) {
      if (!buffer.readable()) {
        return (lineBuf.length() > 0) ? lineBuf.toString() : null;
      }

      int c = buffer.readUnsignedByte();
      switch (c) {
        case '\n':
          break loop;

        case '\r':
          if (buffer.readable() && buffer.getUnsignedByte(buffer.readerIndex()) == '\n') {
            buffer.skipBytes(1);
          }
          break loop;

        default:
          lineBuf.append((char)c);
      }
    }

    return lineBuf.toString();
  }
```

fixed via 674f4bce51047a19000ac1074bb934414511e9aa


Pull request review comments:

Patch that resolved the issue:
diff --git a/buffer/src/main/java/io/netty/buffer/ByteBufInputStream.java b/buffer/src/main/java/io/netty/buffer/ByteBufInputStream.java
index f99e4c6444b..46b11b090c4 100644
--- a/buffer/src/main/java/io/netty/buffer/ByteBufInputStream.java
+++ b/buffer/src/main/java/io/netty/buffer/ByteBufInputStream.java
@@ -186,21 +186,25 @@ public int readInt() throws IOException {
     @Override
     public String readLine() throws IOException {
         lineBuf.setLength(0);
-        for (;;) {
-            int b = read();
-            if (b == -1 && lineBuf.length() == 0) {
-                return null;
-            }
-            if (b < 0 || b == '\n') {
-                break;
+
+        loop: while (true) {
+            if (!buffer.isReadable()) {
+                return (lineBuf.length() > 0) ? lineBuf.toString() : null;
             }
 
-            lineBuf.append((char) b);
-        }
+            int c = buffer.readUnsignedByte();
+            switch (c) {
+                case '\n':
+                    break loop;
+
+                case '\r':
+                    if (buffer.isReadable() && buffer.getUnsignedByte(buffer.readerIndex()) == '\n') {
+                        buffer.skipBytes(1);
+                    }
+                    break loop;
 
-        if (lineBuf.length() > 0) {
-            while (lineBuf.charAt(lineBuf.length() - 1) == '\r') {
-                lineBuf.setLength(lineBuf.length() - 1);
+                default:
+                    lineBuf.append((char) c);
             }
         }
 


Test patch:
diff --git a/buffer/src/test/java/io/netty/buffer/ByteBufStreamTest.java b/buffer/src/test/java/io/netty/buffer/ByteBufStreamTest.java
index 430df8ff5dd..7f2d180061f 100644
--- a/buffer/src/test/java/io/netty/buffer/ByteBufStreamTest.java
+++ b/buffer/src/test/java/io/netty/buffer/ByteBufStreamTest.java
@@ -134,6 +134,7 @@ public void testAll() throws Exception {
 
         assertEquals("Hello, World!", in.readUTF());
         assertEquals("The first line", in.readLine());
+        assertEquals("", in.readLine());
 
         assertEquals(4, in.read(tmp));
         assertEquals(1, tmp[0]);