andreacavalli/netty5
1
0
Fork 0
Code Issues Pull Requests Projects Releases Wiki Activity

XmlFrameDecoder is corrupt

Browse Source 
Motivation:

Two problems:
1. Decoder assumption that as soon as it finds </ element it can decrement opened xml brackets counter. It can lead to bugs when closing bracket is not in byteBuf yet.
2. Not proper handling of more than two root elements in XML document. First element will be processed properly, second one not. It is caused by assumption that byteBuf readerIndex is 0 at the begging of decoding.

Modifications:

Both problems were resolved by fixes:
1. decrement opened brackets count only if </ > enclosing bracket is found
2. consider readerIndex higher than 0 when counting output frame length

Result:

Both problems were resolved
This commit is contained in:
tczerwinski 2015-07-29 13:16:53 +02:00 committed by Norman Maurer
parent 623d9d7202
commit b33c7b12a4
2 changed files with 28 additions and 10 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

23
codec/src/main/java/io/netty/handler/codec/xml/XmlFrameDecoder.java
View File

@ -111,8 +111,16 @@ public class XmlFrameDecoder extends ByteToMessageDecoder {
if (i < bufferLength - 1) {
final byte peekAheadByte = in.getByte(i + 1);
if (peekAheadByte == '/') {
// found </, decrementing openBracketsCount
openBracketsCount--;
// found </, we must check if it is enclosed
int peekFurtherAheadIndex = i + 2;
while (peekFurtherAheadIndex <= bufferLength - 1) {
//if we have </ and enclosing > we can decrement openBracketsCount
if (in.getByte(peekFurtherAheadIndex) == '>') {
openBracketsCount--;
break;
}
peekFurtherAheadIndex++;
}
} else if (isValidStartCharForXmlElement(peekAheadByte)) {
atLeastOneXmlElementFound = true;
// char after < is a valid xml element start char,
@ -166,14 +174,15 @@ public class XmlFrameDecoder extends ByteToMessageDecoder {
}
final int readerIndex = in.readerIndex();
int xmlElementLength = length - readerIndex;
if (openBracketsCount == 0 && length > 0) {
if (length >= bufferLength) {
length = in.readableBytes();
if (openBracketsCount == 0 && xmlElementLength > 0) {
if (readerIndex + xmlElementLength >= bufferLength) {
xmlElementLength = in.readableBytes();
}
final ByteBuf frame =
extractFrame(in, readerIndex + leadingWhiteSpaceCount, length - leadingWhiteSpaceCount);
in.skipBytes(length);
extractFrame(in, readerIndex + leadingWhiteSpaceCount, xmlElementLength - leadingWhiteSpaceCount);
in.skipBytes(xmlElementLength);
out.add(frame);
}
}

15
codec/src/test/java/io/netty/handler/codec/xml/XmlFrameDecoderTest.java
View File

@ -101,6 +101,12 @@ public class XmlFrameDecoderTest {
testDecodeWithXml(" \n\r \t<xxx/>\ttrash", "<xxx/>", CorruptedFrameException.class);
}
@Test
public void testDecodeInvalidXml() {
testDecodeWithXml("<a></", new Object[0]);
testDecodeWithXml("<a></a", new Object[0]);
}
@Test
public void testDecodeWithCDATABlock() {
final String xml = "<book>" +
@ -119,18 +125,21 @@ public class XmlFrameDecoderTest {
}
@Test
public void testDecodeWithTwoMessages() {
public void testDecodeWithMultipleMessages() {
final String input = "<root xmlns=\"http://www.acme.com/acme\" status=\"loginok\" " +
"timestamp=\"1362410583776\"/>\n\n" +
"<root xmlns=\"http://www.acme.com/acme\" status=\"start\" time=\"0\" " +
"timestamp=\"1362410584794\">\n<child active=\"1\" status=\"started\" id=\"935449\" " +
"msgnr=\"2\"/>\n</root>";
"msgnr=\"2\"/>\n</root>" +
"<root xmlns=\"http://www.acme.com/acme\" status=\"logout\" timestamp=\"1362410584795\"/>";
final String frame1 = "<root xmlns=\"http://www.acme.com/acme\" status=\"loginok\" " +
"timestamp=\"1362410583776\"/>";
final String frame2 = "<root xmlns=\"http://www.acme.com/acme\" status=\"start\" time=\"0\" " +
"timestamp=\"1362410584794\">\n<child active=\"1\" status=\"started\" id=\"935449\" " +
"msgnr=\"2\"/>\n</root>";
testDecodeWithXml(input, frame1, frame2);
final String frame3 = "<root xmlns=\"http://www.acme.com/acme\" status=\"logout\" " +
"timestamp=\"1362410584795\"/>";
testDecodeWithXml(input, frame1, frame2, frame3);
}
@Test