[#2544] Correctly parse Multipart-mixed POST HTTP request in case of entity ends with odd number of 0x0D. Port of @fredericBregier 's work.
Motivation: When an attribute is ending with an odd number of CR (0x0D), the decoder add an extra CR in the decoded attribute and should not. Modifications: Each time a CR is detected, the next byte was tested to be LF or not. If not, in a number of places, the CR byte was lost while it should not be. When a CR is detected, if the next byte is not LF, the CR byte should be saved as the position point to the next byte (not LF). When a CR is detected, if there is not yet other available bytes, the position is reset to the position of CR (since a LF could follow). A new Junit test case is added, using DECODER and variable number of CR in the final attribute (testMultipartCodecWithCRasEndOfAttribute). Result: The attribute is now correctly decoded with the right number of CR ending bytes.
This commit is contained in:
parent
dde0df9955
commit
129c17aaa4
@ -817,9 +817,15 @@ public class HttpPostMultipartRequestDecoder implements InterfaceHttpPostRequest
|
||||
while (undecodedChunk.readable()) {
|
||||
byte nextByte = undecodedChunk.readByte();
|
||||
if (nextByte == HttpConstants.CR) {
|
||||
nextByte = undecodedChunk.readByte();
|
||||
// check but do not changed readerIndex
|
||||
nextByte = undecodedChunk.getByte(undecodedChunk.readerIndex());
|
||||
if (nextByte == HttpConstants.LF) {
|
||||
// force read
|
||||
undecodedChunk.readByte();
|
||||
return line.toString(charset);
|
||||
} else {
|
||||
// Write CR (not followed by LF)
|
||||
line.writeByte(HttpConstants.CR);
|
||||
}
|
||||
} else if (nextByte == HttpConstants.LF) {
|
||||
return line.toString(charset);
|
||||
@ -858,6 +864,10 @@ public class HttpPostMultipartRequestDecoder implements InterfaceHttpPostRequest
|
||||
if (nextByte == HttpConstants.LF) {
|
||||
sao.setReadPosition(0);
|
||||
return line.toString(charset);
|
||||
} else {
|
||||
// Write CR (not followed by LF)
|
||||
sao.pos--;
|
||||
line.writeByte(HttpConstants.CR);
|
||||
}
|
||||
} else {
|
||||
line.writeByte(nextByte);
|
||||
@ -1010,6 +1020,11 @@ public class HttpPostMultipartRequestDecoder implements InterfaceHttpPostRequest
|
||||
if (nextByte == HttpConstants.LF) {
|
||||
sao.setReadPosition(0);
|
||||
return sb.toString();
|
||||
} else {
|
||||
// error CR without LF
|
||||
// delimiter not found so break here !
|
||||
undecodedChunk.readerIndex(readerIndex);
|
||||
throw new NotEnoughDataDecoderException();
|
||||
}
|
||||
} else {
|
||||
// error since CR must be followed by LF
|
||||
@ -1037,6 +1052,11 @@ public class HttpPostMultipartRequestDecoder implements InterfaceHttpPostRequest
|
||||
if (nextByte == HttpConstants.LF) {
|
||||
sao.setReadPosition(0);
|
||||
return sb.toString();
|
||||
} else {
|
||||
// error CR without LF
|
||||
// delimiter not found so break here !
|
||||
undecodedChunk.readerIndex(readerIndex);
|
||||
throw new NotEnoughDataDecoderException();
|
||||
}
|
||||
} else {
|
||||
// error CR without LF
|
||||
@ -1328,7 +1348,13 @@ public class HttpPostMultipartRequestDecoder implements InterfaceHttpPostRequest
|
||||
newLine = true;
|
||||
index = 0;
|
||||
lastPosition = undecodedChunk.readerIndex() - 2;
|
||||
} else {
|
||||
// Unread second nextByte
|
||||
lastPosition = undecodedChunk.readerIndex() - 1;
|
||||
undecodedChunk.readerIndex(lastPosition);
|
||||
}
|
||||
} else {
|
||||
lastPosition = undecodedChunk.readerIndex() - 1;
|
||||
}
|
||||
} else if (nextByte == HttpConstants.LF) {
|
||||
newLine = true;
|
||||
@ -1347,7 +1373,13 @@ public class HttpPostMultipartRequestDecoder implements InterfaceHttpPostRequest
|
||||
newLine = true;
|
||||
index = 0;
|
||||
lastPosition = undecodedChunk.readerIndex() - 2;
|
||||
} else {
|
||||
// Unread second nextByte
|
||||
lastPosition = undecodedChunk.readerIndex() - 1;
|
||||
undecodedChunk.readerIndex(lastPosition);
|
||||
}
|
||||
} else {
|
||||
lastPosition = undecodedChunk.readerIndex() - 1;
|
||||
}
|
||||
} else if (nextByte == HttpConstants.LF) {
|
||||
newLine = true;
|
||||
@ -1432,6 +1464,10 @@ public class HttpPostMultipartRequestDecoder implements InterfaceHttpPostRequest
|
||||
newLine = true;
|
||||
index = 0;
|
||||
lastrealpos = sao.pos - 2;
|
||||
} else {
|
||||
// Unread last nextByte
|
||||
sao.pos--;
|
||||
lastrealpos = sao.pos;
|
||||
}
|
||||
}
|
||||
} else if (nextByte == HttpConstants.LF) {
|
||||
@ -1451,6 +1487,10 @@ public class HttpPostMultipartRequestDecoder implements InterfaceHttpPostRequest
|
||||
newLine = true;
|
||||
index = 0;
|
||||
lastrealpos = sao.pos - 2;
|
||||
} else {
|
||||
// Unread last nextByte
|
||||
sao.pos--;
|
||||
lastrealpos = sao.pos;
|
||||
}
|
||||
}
|
||||
} else if (nextByte == HttpConstants.LF) {
|
||||
|
@ -72,6 +72,50 @@ public class HttpPostRequestDecoderTest {
|
||||
}
|
||||
}
|
||||
|
||||
// See https://github.com/netty/netty/issues/2544
|
||||
@Test
|
||||
public void testMultipartCodecWithCRasEndOfAttribute() throws Exception {
|
||||
final String boundary = "dLV9Wyq26L_-JQxk6ferf-RT153LhOO";
|
||||
|
||||
// Force to use memory-based data.
|
||||
final DefaultHttpDataFactory inMemoryFactory = new DefaultHttpDataFactory(false);
|
||||
// Build test case
|
||||
String extradata = "aaaa";
|
||||
String[] datas = new String[5];
|
||||
for (int i = 0; i < 4; i++) {
|
||||
datas[i] = extradata;
|
||||
for (int j = 0; j < i; j++) {
|
||||
datas[i] += '\r';
|
||||
}
|
||||
}
|
||||
|
||||
for (int i = 0; i < 4; i++) {
|
||||
final DefaultHttpRequest req = new DefaultHttpRequest(HttpVersion.HTTP_1_1, HttpMethod.POST,
|
||||
"http://localhost");
|
||||
req.headers().add(HttpHeaders.Names.CONTENT_TYPE, "multipart/form-data; boundary=" + boundary);
|
||||
|
||||
final String body =
|
||||
"--" + boundary + "\r\n" +
|
||||
"Content-Disposition: form-data; name=\"file" + i + "\"\r\n" +
|
||||
"Content-Type: image/gif\r\n" +
|
||||
"\r\n" +
|
||||
datas[i] + "\r\n" +
|
||||
"--" + boundary + "--\r\n";
|
||||
|
||||
req.setContent(ChannelBuffers.wrappedBuffer(body.getBytes(CharsetUtil.UTF_8.name())));
|
||||
// Create decoder instance to test.
|
||||
final HttpPostRequestDecoder decoder = new HttpPostRequestDecoder(inMemoryFactory, req);
|
||||
assertFalse(decoder.getBodyHttpDatas().isEmpty());
|
||||
// Check correctness: data size
|
||||
InterfaceHttpData httpdata = decoder.getBodyHttpData("file" + i);
|
||||
assertNotNull(httpdata);
|
||||
Attribute attribute = (Attribute) httpdata;
|
||||
byte[] datar = attribute.get();
|
||||
assertNotNull(datar);
|
||||
assertEquals(datas[i].getBytes(CharsetUtil.UTF_8.name()).length, datar.length);
|
||||
}
|
||||
}
|
||||
|
||||
// See https://github.com/netty/netty/issues/2542
|
||||
@Test
|
||||
public void testQuotedBoundary() throws Exception {
|
||||
|
Loading…
Reference in New Issue
Block a user