[#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
4ad3984c8b
commit
a0a8f1032b
@ -1287,9 +1287,15 @@ public class HttpPostRequestDecoder {
|
|||||||
while (undecodedChunk.isReadable()) {
|
while (undecodedChunk.isReadable()) {
|
||||||
byte nextByte = undecodedChunk.readByte();
|
byte nextByte = undecodedChunk.readByte();
|
||||||
if (nextByte == HttpConstants.CR) {
|
if (nextByte == HttpConstants.CR) {
|
||||||
nextByte = undecodedChunk.readByte();
|
// check but do not changed readerIndex
|
||||||
|
nextByte = undecodedChunk.getByte(undecodedChunk.readerIndex());
|
||||||
if (nextByte == HttpConstants.LF) {
|
if (nextByte == HttpConstants.LF) {
|
||||||
|
// Skip
|
||||||
|
undecodedChunk.skipBytes(1);
|
||||||
return line.toString(charset);
|
return line.toString(charset);
|
||||||
|
} else {
|
||||||
|
// Write CR (not followed by LF)
|
||||||
|
line.writeByte(HttpConstants.CR);
|
||||||
}
|
}
|
||||||
} else if (nextByte == HttpConstants.LF) {
|
} else if (nextByte == HttpConstants.LF) {
|
||||||
return line.toString(charset);
|
return line.toString(charset);
|
||||||
@ -1332,6 +1338,10 @@ public class HttpPostRequestDecoder {
|
|||||||
if (nextByte == HttpConstants.LF) {
|
if (nextByte == HttpConstants.LF) {
|
||||||
sao.setReadPosition(0);
|
sao.setReadPosition(0);
|
||||||
return line.toString(charset);
|
return line.toString(charset);
|
||||||
|
} else {
|
||||||
|
// Write CR (not followed by LF)
|
||||||
|
sao.pos--;
|
||||||
|
line.writeByte(HttpConstants.CR);
|
||||||
}
|
}
|
||||||
} else {
|
} else {
|
||||||
line.writeByte(nextByte);
|
line.writeByte(nextByte);
|
||||||
@ -1494,6 +1504,11 @@ public class HttpPostRequestDecoder {
|
|||||||
if (nextByte == HttpConstants.LF) {
|
if (nextByte == HttpConstants.LF) {
|
||||||
sao.setReadPosition(0);
|
sao.setReadPosition(0);
|
||||||
return sb.toString();
|
return sb.toString();
|
||||||
|
} else {
|
||||||
|
// error CR without LF
|
||||||
|
// delimiter not found so break here !
|
||||||
|
undecodedChunk.readerIndex(readerIndex);
|
||||||
|
throw new NotEnoughDataDecoderException();
|
||||||
}
|
}
|
||||||
} else {
|
} else {
|
||||||
// error since CR must be followed by LF
|
// error since CR must be followed by LF
|
||||||
@ -1522,6 +1537,11 @@ public class HttpPostRequestDecoder {
|
|||||||
if (nextByte == HttpConstants.LF) {
|
if (nextByte == HttpConstants.LF) {
|
||||||
sao.setReadPosition(0);
|
sao.setReadPosition(0);
|
||||||
return sb.toString();
|
return sb.toString();
|
||||||
|
} else {
|
||||||
|
// error CR without LF
|
||||||
|
// delimiter not found so break here !
|
||||||
|
undecodedChunk.readerIndex(readerIndex);
|
||||||
|
throw new NotEnoughDataDecoderException();
|
||||||
}
|
}
|
||||||
} else {
|
} else {
|
||||||
// error CR without LF
|
// error CR without LF
|
||||||
@ -1826,7 +1846,13 @@ public class HttpPostRequestDecoder {
|
|||||||
newLine = true;
|
newLine = true;
|
||||||
index = 0;
|
index = 0;
|
||||||
lastPosition = undecodedChunk.readerIndex() - 2;
|
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) {
|
} else if (nextByte == HttpConstants.LF) {
|
||||||
newLine = true;
|
newLine = true;
|
||||||
@ -1845,7 +1871,13 @@ public class HttpPostRequestDecoder {
|
|||||||
newLine = true;
|
newLine = true;
|
||||||
index = 0;
|
index = 0;
|
||||||
lastPosition = undecodedChunk.readerIndex() - 2;
|
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) {
|
} else if (nextByte == HttpConstants.LF) {
|
||||||
newLine = true;
|
newLine = true;
|
||||||
@ -1930,6 +1962,10 @@ public class HttpPostRequestDecoder {
|
|||||||
newLine = true;
|
newLine = true;
|
||||||
index = 0;
|
index = 0;
|
||||||
lastrealpos = sao.pos - 2;
|
lastrealpos = sao.pos - 2;
|
||||||
|
} else {
|
||||||
|
// Unread last nextByte
|
||||||
|
sao.pos--;
|
||||||
|
lastrealpos = sao.pos;
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
} else if (nextByte == HttpConstants.LF) {
|
} else if (nextByte == HttpConstants.LF) {
|
||||||
@ -1949,6 +1985,10 @@ public class HttpPostRequestDecoder {
|
|||||||
newLine = true;
|
newLine = true;
|
||||||
index = 0;
|
index = 0;
|
||||||
lastrealpos = sao.pos - 2;
|
lastrealpos = sao.pos - 2;
|
||||||
|
} else {
|
||||||
|
// Unread last nextByte
|
||||||
|
sao.pos--;
|
||||||
|
lastrealpos = sao.pos;
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
} else if (nextByte == HttpConstants.LF) {
|
} else if (nextByte == HttpConstants.LF) {
|
||||||
|
@ -129,6 +129,53 @@ public class HttpPostRequestDecoderTest {
|
|||||||
decoder.destroy();
|
decoder.destroy();
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// 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 DefaultFullHttpRequest req = new DefaultFullHttpRequest(HttpVersion.HTTP_1_1, HttpMethod.POST,
|
||||||
|
"http://localhost");
|
||||||
|
req.setDecoderResult(DecoderResult.SUCCESS);
|
||||||
|
req.headers().add(HttpHeaders.Names.CONTENT_TYPE, "multipart/form-data; boundary=" + boundary);
|
||||||
|
req.headers().add(HttpHeaders.Names.TRANSFER_ENCODING, HttpHeaders.Values.CHUNKED);
|
||||||
|
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.content().writeBytes(body.getBytes(CharsetUtil.UTF_8));
|
||||||
|
// 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).length, datar.length);
|
||||||
|
|
||||||
|
decoder.destroy();
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
// See https://github.com/netty/netty/issues/1848
|
// See https://github.com/netty/netty/issues/1848
|
||||||
@Test
|
@Test
|
||||||
public void testNoZeroOut() throws Exception {
|
public void testNoZeroOut() throws Exception {
|
||||||
|
Loading…
Reference in New Issue
Block a user