From 131be58f4866e30626d53386df0c236d17044891 Mon Sep 17 00:00:00 2001
From: Norman Maurer <norman_maurer@apple.com>
Date: Mon, 1 Jul 2019 20:55:23 +0200
Subject: [PATCH] =?UTF-8?q?Correctly=20take=20length=20of=20ByteBufInputSt?=
 =?UTF-8?q?ream=20into=20account=20for=20readLine=E2=80=A6=20(#9310)?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

* Correctly take length of ByteBufInputStream into account for readLine() / readByte()

Motivation:

ByteBufInputStream did not correctly take the length into account when validate bounds for readLine() / readByte() which could lead to read more then allowed.

Modifications:

- Correctly take length into account
- Add unit tests
- Fix existing unit test

Result:

Correctly take length of ByteBufInputStream into account.
Related to https://github.com/netty/netty/pull/9306.
---
 .../io/netty/buffer/ByteBufInputStream.java   | 13 ++--
 .../io/netty/buffer/ByteBufStreamTest.java    | 70 +++++++++++++++----
 2 files changed, 67 insertions(+), 16 deletions(-)

diff --git a/buffer/src/main/java/io/netty/buffer/ByteBufInputStream.java b/buffer/src/main/java/io/netty/buffer/ByteBufInputStream.java
index c0b116c148..09425c5fdb 100644
--- a/buffer/src/main/java/io/netty/buffer/ByteBufInputStream.java
+++ b/buffer/src/main/java/io/netty/buffer/ByteBufInputStream.java
@@ -204,7 +204,8 @@ public class ByteBufInputStream extends InputStream implements DataInput {
 
     @Override
     public byte readByte() throws IOException {
-        if (!buffer.isReadable()) {
+        int available = available();
+        if (available == 0) {
             throw new EOFException();
         }
         return buffer.readByte();
@@ -246,22 +247,26 @@ public class ByteBufInputStream extends InputStream implements DataInput {
 
     @Override
     public String readLine() throws IOException {
-        if (!buffer.isReadable()) {
+        int available = available();
+        if (available == 0) {
             return null;
         }
+
         if (lineBuf != null) {
             lineBuf.setLength(0);
         }
 
         loop: do {
             int c = buffer.readUnsignedByte();
+            --available;
             switch (c) {
                 case '\n':
                     break loop;
 
                 case '\r':
-                    if (buffer.isReadable() && (char) buffer.getUnsignedByte(buffer.readerIndex()) == '\n') {
+                    if (available > 0 && (char) buffer.getUnsignedByte(buffer.readerIndex()) == '\n') {
                         buffer.skipBytes(1);
+                        --available;
                     }
                     break loop;
 
@@ -271,7 +276,7 @@ public class ByteBufInputStream extends InputStream implements DataInput {
                     }
                     lineBuf.append((char) c);
             }
-        } while (buffer.isReadable());
+        } while (available > 0);
 
         return lineBuf != null && lineBuf.length() > 0 ? lineBuf.toString() : StringUtil.EMPTY_STRING;
     }
diff --git a/buffer/src/test/java/io/netty/buffer/ByteBufStreamTest.java b/buffer/src/test/java/io/netty/buffer/ByteBufStreamTest.java
index 989e160b22..e0f4f50337 100644
--- a/buffer/src/test/java/io/netty/buffer/ByteBufStreamTest.java
+++ b/buffer/src/test/java/io/netty/buffer/ByteBufStreamTest.java
@@ -187,30 +187,34 @@ public class ByteBufStreamTest {
 
         String s = in.readLine();
         assertNull(s);
+        in.close();
 
+        ByteBuf buf2 = Unpooled.buffer();
         int charCount = 7; //total chars in the string below without new line characters
         byte[] abc = "\na\n\nb\r\nc\nd\ne".getBytes(utf8);
-        buf.writeBytes(abc);
-        in.mark(charCount);
-        assertEquals("", in.readLine());
-        assertEquals("a", in.readLine());
-        assertEquals("", in.readLine());
-        assertEquals("b", in.readLine());
-        assertEquals("c", in.readLine());
-        assertEquals("d", in.readLine());
-        assertEquals("e", in.readLine());
+        buf2.writeBytes(abc);
+
+        ByteBufInputStream in2 = new ByteBufInputStream(buf2, true);
+        in2.mark(charCount);
+        assertEquals("", in2.readLine());
+        assertEquals("a", in2.readLine());
+        assertEquals("", in2.readLine());
+        assertEquals("b", in2.readLine());
+        assertEquals("c", in2.readLine());
+        assertEquals("d", in2.readLine());
+        assertEquals("e", in2.readLine());
         assertNull(in.readLine());
 
-        in.reset();
+        in2.reset();
         int count = 0;
-        while (in.readLine() != null) {
+        while (in2.readLine() != null) {
             ++count;
             if (count > charCount) {
                 fail("readLine() should have returned null");
             }
         }
         assertEquals(charCount, count);
-        in.close();
+        in2.close();
     }
 
     @Test
@@ -247,4 +251,46 @@ public class ByteBufStreamTest {
         buf2.release();
         in2.close();
     }
+
+    @Test
+    public void testReadLineLengthRespected1() throws Exception {
+        // case1
+        ByteBuf buf = Unpooled.buffer(16);
+        buf.writeBytes(new byte[] { 1, 2, 3, 4, 5, 6 });
+
+        ByteBufInputStream in = new ByteBufInputStream(buf, 0);
+
+        assertNull(in.readLine());
+        buf.release();
+        in.close();
+    }
+
+    @Test
+    public void testReadLineLengthRespected2() throws Exception {
+        ByteBuf buf2 = Unpooled.buffer(16);
+        buf2.writeBytes(new byte[] { 'A', 'B', '\n', 'C', 'E', 'F'});
+
+        ByteBufInputStream in2 = new ByteBufInputStream(buf2, 4);
+
+        assertEquals("AB", in2.readLine());
+        assertEquals("C", in2.readLine());
+        assertNull(in2.readLine());
+        buf2.release();
+        in2.close();
+    }
+
+    @Test(expected = EOFException.class)
+    public void testReadByteLengthRespected() throws Exception {
+        // case1
+        ByteBuf buf = Unpooled.buffer(16);
+        buf.writeBytes(new byte[] { 1, 2, 3, 4, 5, 6 });
+
+        ByteBufInputStream in = new ByteBufInputStream(buf, 0);
+        try {
+            in.readByte();
+        } finally {
+            buf.release();
+            in.close();
+        }
+    }
 }