Correctly take length of ByteBufInputStream into account for readLine… (#9310)

* 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.
This commit is contained in:
Norman Maurer 2019-07-01 20:55:23 +02:00
parent fbd6cc2503
commit f0afd7cfcd
2 changed files with 67 additions and 16 deletions

View File

@ -206,7 +206,8 @@ public class ByteBufInputStream extends InputStream implements DataInput {
@Override @Override
public byte readByte() throws IOException { public byte readByte() throws IOException {
if (!buffer.isReadable()) { int available = available();
if (available == 0) {
throw new EOFException(); throw new EOFException();
} }
return buffer.readByte(); return buffer.readByte();
@ -248,22 +249,26 @@ public class ByteBufInputStream extends InputStream implements DataInput {
@Override @Override
public String readLine() throws IOException { public String readLine() throws IOException {
if (!buffer.isReadable()) { int available = available();
if (available == 0) {
return null; return null;
} }
if (lineBuf != null) { if (lineBuf != null) {
lineBuf.setLength(0); lineBuf.setLength(0);
} }
loop: do { loop: do {
int c = buffer.readUnsignedByte(); int c = buffer.readUnsignedByte();
--available;
switch (c) { switch (c) {
case '\n': case '\n':
break loop; break loop;
case '\r': case '\r':
if (buffer.isReadable() && (char) buffer.getUnsignedByte(buffer.readerIndex()) == '\n') { if (available > 0 && (char) buffer.getUnsignedByte(buffer.readerIndex()) == '\n') {
buffer.skipBytes(1); buffer.skipBytes(1);
--available;
} }
break loop; break loop;
@ -273,7 +278,7 @@ public class ByteBufInputStream extends InputStream implements DataInput {
} }
lineBuf.append((char) c); lineBuf.append((char) c);
} }
} while (buffer.isReadable()); } while (available > 0);
return lineBuf != null && lineBuf.length() > 0 ? lineBuf.toString() : StringUtil.EMPTY_STRING; return lineBuf != null && lineBuf.length() > 0 ? lineBuf.toString() : StringUtil.EMPTY_STRING;
} }

View File

@ -187,30 +187,34 @@ public class ByteBufStreamTest {
String s = in.readLine(); String s = in.readLine();
assertNull(s); assertNull(s);
in.close();
ByteBuf buf2 = Unpooled.buffer();
int charCount = 7; //total chars in the string below without new line characters int charCount = 7; //total chars in the string below without new line characters
byte[] abc = "\na\n\nb\r\nc\nd\ne".getBytes(utf8); byte[] abc = "\na\n\nb\r\nc\nd\ne".getBytes(utf8);
buf.writeBytes(abc); buf2.writeBytes(abc);
in.mark(charCount);
assertEquals("", in.readLine()); ByteBufInputStream in2 = new ByteBufInputStream(buf2, true);
assertEquals("a", in.readLine()); in2.mark(charCount);
assertEquals("", in.readLine()); assertEquals("", in2.readLine());
assertEquals("b", in.readLine()); assertEquals("a", in2.readLine());
assertEquals("c", in.readLine()); assertEquals("", in2.readLine());
assertEquals("d", in.readLine()); assertEquals("b", in2.readLine());
assertEquals("e", in.readLine()); assertEquals("c", in2.readLine());
assertEquals("d", in2.readLine());
assertEquals("e", in2.readLine());
assertNull(in.readLine()); assertNull(in.readLine());
in.reset(); in2.reset();
int count = 0; int count = 0;
while (in.readLine() != null) { while (in2.readLine() != null) {
++count; ++count;
if (count > charCount) { if (count > charCount) {
fail("readLine() should have returned null"); fail("readLine() should have returned null");
} }
} }
assertEquals(charCount, count); assertEquals(charCount, count);
in.close(); in2.close();
} }
@Test @Test
@ -247,4 +251,46 @@ public class ByteBufStreamTest {
buf2.release(); buf2.release();
in2.close(); 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();
}
}
} }