diff --git a/codec-http/src/main/java/io/netty/handler/codec/http/multipart/HttpPostRequestDecoder.java b/codec-http/src/main/java/io/netty/handler/codec/http/multipart/HttpPostRequestDecoder.java index 143efa8057..7fc1b52e68 100644 --- a/codec-http/src/main/java/io/netty/handler/codec/http/multipart/HttpPostRequestDecoder.java +++ b/codec-http/src/main/java/io/netty/handler/codec/http/multipart/HttpPostRequestDecoder.java @@ -538,7 +538,7 @@ public class HttpPostRequestDecoder { if (read == '&') { currentStatus = MultiPartStatus.DISPOSITION; ampersandpos = currentpos - 1; - setFinalBuffer(undecodedChunk.slice(firstpos, ampersandpos - firstpos).retain()); + setFinalBuffer(undecodedChunk.slice(firstpos, ampersandpos - firstpos).copy()); firstpos = currentpos; contRead = true; } else if (read == HttpConstants.CR) { @@ -548,7 +548,7 @@ public class HttpPostRequestDecoder { if (read == HttpConstants.LF) { currentStatus = MultiPartStatus.PREEPILOGUE; ampersandpos = currentpos - 2; - setFinalBuffer(undecodedChunk.slice(firstpos, ampersandpos - firstpos).retain()); + setFinalBuffer(undecodedChunk.slice(firstpos, ampersandpos - firstpos).copy()); firstpos = currentpos; contRead = false; } else { @@ -561,7 +561,7 @@ public class HttpPostRequestDecoder { } else if (read == HttpConstants.LF) { currentStatus = MultiPartStatus.PREEPILOGUE; ampersandpos = currentpos - 1; - setFinalBuffer(undecodedChunk.slice(firstpos, ampersandpos - firstpos).retain()); + setFinalBuffer(undecodedChunk.slice(firstpos, ampersandpos - firstpos).copy()); firstpos = currentpos; contRead = false; } @@ -575,7 +575,7 @@ public class HttpPostRequestDecoder { // special case ampersandpos = currentpos; if (ampersandpos > firstpos) { - setFinalBuffer(undecodedChunk.slice(firstpos, ampersandpos - firstpos).retain()); + setFinalBuffer(undecodedChunk.slice(firstpos, ampersandpos - firstpos).copy()); } else if (!currentAttribute.isCompleted()) { setFinalBuffer(EMPTY_BUFFER); } @@ -586,7 +586,8 @@ public class HttpPostRequestDecoder { if (contRead && currentAttribute != null) { // reset index except if to continue in case of FIELD getStatus if (currentStatus == MultiPartStatus.FIELD) { - currentAttribute.addContent(undecodedChunk.slice(firstpos, currentpos - firstpos).retain(), false); + currentAttribute.addContent(undecodedChunk.slice(firstpos, currentpos - firstpos).copy(), + false); firstpos = currentpos; } undecodedChunk.readerIndex(firstpos); @@ -658,7 +659,7 @@ public class HttpPostRequestDecoder { if (read == '&') { currentStatus = MultiPartStatus.DISPOSITION; ampersandpos = currentpos - 1; - setFinalBuffer(undecodedChunk.slice(firstpos, ampersandpos - firstpos).retain()); + setFinalBuffer(undecodedChunk.slice(firstpos, ampersandpos - firstpos).copy()); firstpos = currentpos; contRead = true; } else if (read == HttpConstants.CR) { @@ -669,7 +670,7 @@ public class HttpPostRequestDecoder { currentStatus = MultiPartStatus.PREEPILOGUE; ampersandpos = currentpos - 2; sao.setReadPosition(0); - setFinalBuffer(undecodedChunk.slice(firstpos, ampersandpos - firstpos).retain()); + setFinalBuffer(undecodedChunk.slice(firstpos, ampersandpos - firstpos).copy()); firstpos = currentpos; contRead = false; break loop; @@ -687,7 +688,7 @@ public class HttpPostRequestDecoder { currentStatus = MultiPartStatus.PREEPILOGUE; ampersandpos = currentpos - 1; sao.setReadPosition(0); - setFinalBuffer(undecodedChunk.slice(firstpos, ampersandpos - firstpos).retain()); + setFinalBuffer(undecodedChunk.slice(firstpos, ampersandpos - firstpos).copy()); firstpos = currentpos; contRead = false; break loop; @@ -704,7 +705,7 @@ public class HttpPostRequestDecoder { // special case ampersandpos = currentpos; if (ampersandpos > firstpos) { - setFinalBuffer(undecodedChunk.slice(firstpos, ampersandpos - firstpos).retain()); + setFinalBuffer(undecodedChunk.slice(firstpos, ampersandpos - firstpos).copy()); } else if (!currentAttribute.isCompleted()) { setFinalBuffer(EMPTY_BUFFER); } @@ -715,7 +716,8 @@ public class HttpPostRequestDecoder { if (contRead && currentAttribute != null) { // reset index except if to continue in case of FIELD getStatus if (currentStatus == MultiPartStatus.FIELD) { - currentAttribute.addContent(undecodedChunk.slice(firstpos, currentpos - firstpos).retain(), false); + currentAttribute.addContent(undecodedChunk.slice(firstpos, currentpos - firstpos).copy(), + false); firstpos = currentpos; } undecodedChunk.readerIndex(firstpos); @@ -1646,7 +1648,7 @@ public class HttpPostRequestDecoder { } } } - ByteBuf buffer = undecodedChunk.slice(readerIndex, lastPosition - readerIndex).retain(); + ByteBuf buffer = undecodedChunk.slice(readerIndex, lastPosition - readerIndex).copy(); if (found) { // found so lastPosition is correct and final try { @@ -1764,7 +1766,7 @@ public class HttpPostRequestDecoder { } } lastPosition = sao.getReadPosition(lastrealpos); - ByteBuf buffer = undecodedChunk.slice(readerIndex, lastPosition - readerIndex).retain(); + ByteBuf buffer = undecodedChunk.slice(readerIndex, lastPosition - readerIndex).copy(); if (found) { // found so lastPosition is correct and final try { @@ -1863,7 +1865,7 @@ public class HttpPostRequestDecoder { // so go back of delimiter size try { currentAttribute.addContent( - undecodedChunk.slice(readerIndex, lastPosition - readerIndex).retain(), true); + undecodedChunk.slice(readerIndex, lastPosition - readerIndex).copy(), true); } catch (IOException e) { throw new ErrorDataDecoderException(e); } @@ -1871,7 +1873,7 @@ public class HttpPostRequestDecoder { } else { try { currentAttribute.addContent( - undecodedChunk.slice(readerIndex, lastPosition - readerIndex).retain(), false); + undecodedChunk.slice(readerIndex, lastPosition - readerIndex).copy(), false); } catch (IOException e) { throw new ErrorDataDecoderException(e); } @@ -1968,7 +1970,7 @@ public class HttpPostRequestDecoder { // so go back of delimiter size try { currentAttribute.addContent( - undecodedChunk.slice(readerIndex, lastPosition - readerIndex).retain(), true); + undecodedChunk.slice(readerIndex, lastPosition - readerIndex).copy(), true); } catch (IOException e) { throw new ErrorDataDecoderException(e); } @@ -1976,7 +1978,7 @@ public class HttpPostRequestDecoder { } else { try { currentAttribute.addContent( - undecodedChunk.slice(readerIndex, lastPosition - readerIndex).retain(), false); + undecodedChunk.slice(readerIndex, lastPosition - readerIndex).copy(), false); } catch (IOException e) { throw new ErrorDataDecoderException(e); } diff --git a/codec-http/src/test/java/io/netty/handler/codec/http/multipart/HttpPostRequestDecoderTest.java b/codec-http/src/test/java/io/netty/handler/codec/http/multipart/HttpPostRequestDecoderTest.java index 52c7ce7de8..090b6463b1 100644 --- a/codec-http/src/test/java/io/netty/handler/codec/http/multipart/HttpPostRequestDecoderTest.java +++ b/codec-http/src/test/java/io/netty/handler/codec/http/multipart/HttpPostRequestDecoderTest.java @@ -23,6 +23,10 @@ import io.netty.handler.codec.http.DefaultHttpRequest; import io.netty.handler.codec.http.HttpHeaders; import io.netty.handler.codec.http.HttpMethod; import io.netty.handler.codec.http.HttpVersion; +import io.netty.buffer.ByteBuf; +import io.netty.buffer.ByteBufAllocator; +import io.netty.buffer.UnpooledByteBufAllocator; +import io.netty.handler.codec.http.LastHttpContent; import io.netty.util.CharsetUtil; import org.junit.Test; @@ -119,4 +123,62 @@ public class HttpPostRequestDecoderTest { final HttpPostRequestDecoder decoder = new HttpPostRequestDecoder(inMemoryFactory, req); assertFalse(decoder.getBodyHttpDatas().isEmpty()); } + + // See https://github.com/netty/netty/issues/1848 + @Test + public void testNoZeroOut() throws Exception { + final String boundary = "E832jQp_Rq2ErFmAduHSR8YlMSm0FCY"; + + final DefaultHttpDataFactory aMemFactory = new DefaultHttpDataFactory(false); + + DefaultHttpRequest aRequest = new DefaultHttpRequest(HttpVersion.HTTP_1_1, + HttpMethod.POST, + "http://localhost"); + aRequest.headers().set(HttpHeaders.Names.CONTENT_TYPE, + "multipart/form-data; boundary=" + boundary); + aRequest.headers().set(HttpHeaders.Names.TRANSFER_ENCODING, + HttpHeaders.Values.CHUNKED); + + HttpPostRequestDecoder aDecoder = new HttpPostRequestDecoder(aMemFactory, aRequest); + + final String aData = "some data would be here. the data should be long enough that it " + + "will be longer than the original buffer length of 256 bytes in " + + "the HttpPostRequestDecoder in order to trigger the issue. Some more " + + "data just to be on the safe side."; + + final String body = + "--" + boundary + "\r\n" + + "Content-Disposition: form-data; name=\"root\"\r\n" + + "Content-Type: text/plain\r\n" + + "\r\n" + + aData + + "\r\n" + + "--" + boundary + "--\r\n"; + + byte[] aBytes = body.getBytes(); + + int split = 125; + + ByteBufAllocator aAlloc = new UnpooledByteBufAllocator(true); + ByteBuf aSmallBuf = aAlloc.heapBuffer(split, split); + ByteBuf aLargeBuf = aAlloc.heapBuffer(aBytes.length - split, aBytes.length - split); + + aSmallBuf.writeBytes(aBytes, 0, split); + aLargeBuf.writeBytes(aBytes, split, aBytes.length - split); + + aDecoder.offer(new DefaultHttpContent(aSmallBuf)); + aDecoder.offer(new DefaultHttpContent(aLargeBuf)); + + aDecoder.offer(LastHttpContent.EMPTY_LAST_CONTENT); + + assertTrue("Should have a piece of data", aDecoder.hasNext()); + + InterfaceHttpData aDecodedData = aDecoder.next(); + + assertEquals(InterfaceHttpData.HttpDataType.Attribute, aDecodedData.getHttpDataType()); + + Attribute aAttr = (Attribute) aDecodedData; + + assertEquals(aData, aAttr.getValue()); + } }