[#2305] Fix issue related to decoding post request raized an exception due to a split of information by chunk not correctly taken into account by the decoder

Motivation:

If the last item analyzed in a previous received HttpChunk/HttpContent was a part of an attribute's name, the read index was not set to the new right place and therefore raizing an exception in some case (since the "new" name analyzed is empty, which is not allowed so the exception).

What appears there is that the read index should be reset to the last valid position encountered whatever the case. Currently it was set when only when there is an attribute not already finished (name is ok, but content is possibly not).

Therefore the issue is that elements could be rescanned multiple times (including completed elements) and moreover some bad decoding can occur such as when in a middle of an attribute's name.

Modifications:

To fix this issue, since "firstpos" contains the last "valid" read index of the decoding (when finding a '&', '=', 'CR/LF'), we should add the setting of the read index for the following cases:

'lastchunk' encountered, therefore finishing the current buffer
any other cases than current attribute is not finished (name not found yet in particular)
So adding for this 2 cases:

undecodedChunk.readerIndex(firstpos);

Result:

Now the decoding is done once, content is added from chunk/content to chunk/content, name is decoded correctly even if in the middle of 2 chunks/contents.
A Junit test code was added: testChunkCorrect that should not raized any exception.
This commit is contained in:
Frederic Bregier 2014-03-13 18:20:12 +01:00 committed by Norman Maurer
parent 1c074eabe5
commit 55bafbd9c2
2 changed files with 61 additions and 2 deletions

View File

@ -581,6 +581,7 @@ public class HttpPostRequestDecoder {
} }
firstpos = currentpos; firstpos = currentpos;
currentStatus = MultiPartStatus.EPILOGUE; currentStatus = MultiPartStatus.EPILOGUE;
undecodedChunk.readerIndex(firstpos);
return; return;
} }
if (contRead && currentAttribute != null) { if (contRead && currentAttribute != null) {
@ -592,7 +593,8 @@ public class HttpPostRequestDecoder {
} }
undecodedChunk.readerIndex(firstpos); undecodedChunk.readerIndex(firstpos);
} else { } else {
// end of line so keep index // end of line or end of block so keep index to last valid position
undecodedChunk.readerIndex(firstpos);
} }
} catch (ErrorDataDecoderException e) { } catch (ErrorDataDecoderException e) {
// error while decoding // error while decoding
@ -711,6 +713,7 @@ public class HttpPostRequestDecoder {
} }
firstpos = currentpos; firstpos = currentpos;
currentStatus = MultiPartStatus.EPILOGUE; currentStatus = MultiPartStatus.EPILOGUE;
undecodedChunk.readerIndex(firstpos);
return; return;
} }
if (contRead && currentAttribute != null) { if (contRead && currentAttribute != null) {
@ -722,7 +725,8 @@ public class HttpPostRequestDecoder {
} }
undecodedChunk.readerIndex(firstpos); undecodedChunk.readerIndex(firstpos);
} else { } else {
// end of line so keep index // end of line or end of block so keep index to last valid position
undecodedChunk.readerIndex(firstpos);
} }
} catch (ErrorDataDecoderException e) { } catch (ErrorDataDecoderException e) {
// error while decoding // error while decoding

View File

@ -23,6 +23,7 @@ import io.netty.handler.codec.DecoderResult;
import io.netty.handler.codec.http.DefaultFullHttpRequest; import io.netty.handler.codec.http.DefaultFullHttpRequest;
import io.netty.handler.codec.http.DefaultHttpContent; import io.netty.handler.codec.http.DefaultHttpContent;
import io.netty.handler.codec.http.DefaultHttpRequest; import io.netty.handler.codec.http.DefaultHttpRequest;
import io.netty.handler.codec.http.HttpContent;
import io.netty.handler.codec.http.HttpHeaders; import io.netty.handler.codec.http.HttpHeaders;
import io.netty.handler.codec.http.HttpMethod; import io.netty.handler.codec.http.HttpMethod;
import io.netty.handler.codec.http.HttpVersion; import io.netty.handler.codec.http.HttpVersion;
@ -186,4 +187,58 @@ public class HttpPostRequestDecoderTest {
aDecodedData.release(); aDecodedData.release();
aDecoder.destroy(); aDecoder.destroy();
} }
// See https://github.com/netty/netty/issues/2305
@Test
public void testChunkCorrect() throws Exception {
String payload = "town=794649819&town=784444184&town=794649672&town=794657800&town=" +
"794655734&town=794649377&town=794652136&town=789936338&town=789948986&town=" +
"789949643&town=786358677&town=794655880&town=786398977&town=789901165&town=" +
"789913325&town=789903418&town=789903579&town=794645251&town=794694126&town=" +
"794694831&town=794655274&town=789913656&town=794653956&town=794665634&town=" +
"789936598&town=789904658&town=789899210&town=799696252&town=794657521&town=" +
"789904837&town=789961286&town=789958704&town=789948839&town=789933899&town=" +
"793060398&town=794659180&town=794659365&town=799724096&town=794696332&town=" +
"789953438&town=786398499&town=794693372&town=789935439&town=794658041&town=" +
"789917595&town=794655427&town=791930372&town=794652891&town=794656365&town=" +
"789960339&town=794645586&town=794657688&town=794697211&town=789937427&town=" +
"789902813&town=789941130&town=794696907&town=789904328&town=789955151&town=" +
"789911570&town=794655074&town=789939531&town=789935242&town=789903835&town=" +
"789953800&town=794649962&town=789939841&town=789934819&town=789959672&town=" +
"794659043&town=794657035&town=794658938&town=794651746&town=794653732&town=" +
"794653881&town=786397909&town=794695736&town=799724044&town=794695926&town=" +
"789912270&town=794649030&town=794657946&town=794655370&town=794659660&town=" +
"794694617&town=799149862&town=789953234&town=789900476&town=794654995&town=" +
"794671126&town=789908868&town=794652942&town=789955605&town=789901934&town=" +
"789950015&town=789937922&town=789962576&town=786360170&town=789954264&town=" +
"789911738&town=789955416&town=799724187&town=789911879&town=794657462&town=" +
"789912561&town=789913167&town=794655195&town=789938266&town=789952099&town=" +
"794657160&town=789949414&town=794691293&town=794698153&town=789935636&town=" +
"789956374&town=789934635&town=789935475&town=789935085&town=794651425&town=" +
"794654936&town=794655680&town=789908669&town=794652031&town=789951298&town=" +
"789938382&town=794651503&town=794653330&town=817675037&town=789951623&town=" +
"789958999&town=789961555&town=794694050&town=794650241&town=794656286&town=" +
"794692081&town=794660090&town=794665227&town=794665136&town=794669931";
DefaultHttpRequest defaultHttpRequest =
new DefaultHttpRequest(HttpVersion.HTTP_1_1, HttpMethod.POST, "/");
HttpPostRequestDecoder decoder = new HttpPostRequestDecoder(defaultHttpRequest);
int firstChunk = 10;
int middleChunk = 1024;
HttpContent part1 = new DefaultHttpContent(Unpooled.wrappedBuffer(
payload.substring(0, firstChunk).getBytes()));
HttpContent part2 = new DefaultHttpContent(Unpooled.wrappedBuffer(
payload.substring(firstChunk, firstChunk + middleChunk).getBytes()));
HttpContent part3 = new DefaultHttpContent(Unpooled.wrappedBuffer(
payload.substring(firstChunk + middleChunk, firstChunk + middleChunk * 2).getBytes()));
HttpContent part4 = new DefaultHttpContent(Unpooled.wrappedBuffer(
payload.substring(firstChunk + middleChunk * 2).getBytes()));
decoder.offer(part1);
decoder.offer(part2);
decoder.offer(part3);
decoder.offer(part4);
}
} }