[#1293] Fix IllegalBufferAccessException in HttpPostRequestDecoder

* Also let HttpPostRequestDecoder extends Iterator and let its Exceptiosn extend DecoderException
This commit is contained in:
Norman Maurer 2013-05-06 21:36:30 +02:00
parent a170f05b4b
commit 97bdabad9c
2 changed files with 41 additions and 23 deletions

View File

@ -15,7 +15,9 @@
*/ */
package io.netty.handler.codec.http.multipart; package io.netty.handler.codec.http.multipart;
import io.netty.buffer.BufUtil;
import io.netty.buffer.ByteBuf; import io.netty.buffer.ByteBuf;
import io.netty.handler.codec.DecoderException;
import io.netty.handler.codec.http.HttpConstants; import io.netty.handler.codec.http.HttpConstants;
import io.netty.handler.codec.http.HttpContent; import io.netty.handler.codec.http.HttpContent;
import io.netty.handler.codec.http.HttpHeaders; import io.netty.handler.codec.http.HttpHeaders;
@ -32,6 +34,7 @@ import java.io.UnsupportedEncodingException;
import java.net.URLDecoder; import java.net.URLDecoder;
import java.nio.charset.Charset; import java.nio.charset.Charset;
import java.util.ArrayList; import java.util.ArrayList;
import java.util.Iterator;
import java.util.List; import java.util.List;
import java.util.Map; import java.util.Map;
import java.util.TreeMap; import java.util.TreeMap;
@ -41,7 +44,7 @@ import static io.netty.buffer.Unpooled.*;
/** /**
* This decoder will decode Body and can handle POST BODY. * This decoder will decode Body and can handle POST BODY.
*/ */
public class HttpPostRequestDecoder { public class HttpPostRequestDecoder implements Iterator<InterfaceHttpData> {
/** /**
* Factory used to create InterfaceHttpData * Factory used to create InterfaceHttpData
*/ */
@ -347,8 +350,11 @@ public class HttpPostRequestDecoder {
* if there is a problem with the charset decoding or other * if there is a problem with the charset decoding or other
* errors * errors
*/ */
public void offer(HttpContent content) throws ErrorDataDecoderException { public HttpPostRequestDecoder offer(HttpContent content) throws ErrorDataDecoderException {
ByteBuf chunked = content.content(); // Maybe we should better not copy here for performance reasons but this will need
// more care by teh caller to release the content in a correct manner later
// So maybe something to optimize on a later stage
ByteBuf chunked = content.content().copy();
if (undecodedChunk == null) { if (undecodedChunk == null) {
undecodedChunk = chunked; undecodedChunk = chunked;
} else { } else {
@ -361,6 +367,7 @@ public class HttpPostRequestDecoder {
isLastChunk = true; isLastChunk = true;
} }
parseBody(); parseBody();
return this;
} }
/** /**
@ -483,7 +490,7 @@ public class HttpPostRequestDecoder {
if (read == '&') { if (read == '&') {
currentStatus = MultiPartStatus.DISPOSITION; currentStatus = MultiPartStatus.DISPOSITION;
ampersandpos = currentpos - 1; ampersandpos = currentpos - 1;
setFinalBuffer(undecodedChunk.slice(firstpos, ampersandpos - firstpos)); setFinalBuffer(undecodedChunk.slice(firstpos, ampersandpos - firstpos).retain());
firstpos = currentpos; firstpos = currentpos;
contRead = true; contRead = true;
} else if (read == HttpConstants.CR) { } else if (read == HttpConstants.CR) {
@ -493,7 +500,7 @@ public class HttpPostRequestDecoder {
if (read == HttpConstants.LF) { if (read == HttpConstants.LF) {
currentStatus = MultiPartStatus.PREEPILOGUE; currentStatus = MultiPartStatus.PREEPILOGUE;
ampersandpos = currentpos - 2; ampersandpos = currentpos - 2;
setFinalBuffer(undecodedChunk.slice(firstpos, ampersandpos - firstpos)); setFinalBuffer(undecodedChunk.slice(firstpos, ampersandpos - firstpos).retain());
firstpos = currentpos; firstpos = currentpos;
contRead = false; contRead = false;
} else { } else {
@ -506,7 +513,7 @@ public class HttpPostRequestDecoder {
} else if (read == HttpConstants.LF) { } else if (read == HttpConstants.LF) {
currentStatus = MultiPartStatus.PREEPILOGUE; currentStatus = MultiPartStatus.PREEPILOGUE;
ampersandpos = currentpos - 1; ampersandpos = currentpos - 1;
setFinalBuffer(undecodedChunk.slice(firstpos, ampersandpos - firstpos)); setFinalBuffer(undecodedChunk.slice(firstpos, ampersandpos - firstpos).retain());
firstpos = currentpos; firstpos = currentpos;
contRead = false; contRead = false;
} }
@ -520,7 +527,7 @@ public class HttpPostRequestDecoder {
// special case // special case
ampersandpos = currentpos; ampersandpos = currentpos;
if (ampersandpos > firstpos) { if (ampersandpos > firstpos) {
setFinalBuffer(undecodedChunk.slice(firstpos, ampersandpos - firstpos)); setFinalBuffer(undecodedChunk.slice(firstpos, ampersandpos - firstpos).retain());
} else if (!currentAttribute.isCompleted()) { } else if (!currentAttribute.isCompleted()) {
setFinalBuffer(EMPTY_BUFFER); setFinalBuffer(EMPTY_BUFFER);
} }
@ -531,7 +538,7 @@ public class HttpPostRequestDecoder {
if (contRead && currentAttribute != null) { if (contRead && currentAttribute != null) {
// reset index except if to continue in case of FIELD getStatus // reset index except if to continue in case of FIELD getStatus
if (currentStatus == MultiPartStatus.FIELD) { if (currentStatus == MultiPartStatus.FIELD) {
currentAttribute.addContent(undecodedChunk.slice(firstpos, currentpos - firstpos), false); currentAttribute.addContent(undecodedChunk.slice(firstpos, currentpos - firstpos).retain(), false);
firstpos = currentpos; firstpos = currentpos;
} }
undecodedChunk.readerIndex(firstpos); undecodedChunk.readerIndex(firstpos);
@ -603,7 +610,7 @@ public class HttpPostRequestDecoder {
if (read == '&') { if (read == '&') {
currentStatus = MultiPartStatus.DISPOSITION; currentStatus = MultiPartStatus.DISPOSITION;
ampersandpos = currentpos - 1; ampersandpos = currentpos - 1;
setFinalBuffer(undecodedChunk.slice(firstpos, ampersandpos - firstpos)); setFinalBuffer(undecodedChunk.slice(firstpos, ampersandpos - firstpos).retain());
firstpos = currentpos; firstpos = currentpos;
contRead = true; contRead = true;
} else if (read == HttpConstants.CR) { } else if (read == HttpConstants.CR) {
@ -614,7 +621,7 @@ public class HttpPostRequestDecoder {
currentStatus = MultiPartStatus.PREEPILOGUE; currentStatus = MultiPartStatus.PREEPILOGUE;
ampersandpos = currentpos - 2; ampersandpos = currentpos - 2;
sao.setReadPosition(0); sao.setReadPosition(0);
setFinalBuffer(undecodedChunk.slice(firstpos, ampersandpos - firstpos)); setFinalBuffer(undecodedChunk.slice(firstpos, ampersandpos - firstpos).retain());
firstpos = currentpos; firstpos = currentpos;
contRead = false; contRead = false;
break loop; break loop;
@ -632,7 +639,7 @@ public class HttpPostRequestDecoder {
currentStatus = MultiPartStatus.PREEPILOGUE; currentStatus = MultiPartStatus.PREEPILOGUE;
ampersandpos = currentpos - 1; ampersandpos = currentpos - 1;
sao.setReadPosition(0); sao.setReadPosition(0);
setFinalBuffer(undecodedChunk.slice(firstpos, ampersandpos - firstpos)); setFinalBuffer(undecodedChunk.slice(firstpos, ampersandpos - firstpos).retain());
firstpos = currentpos; firstpos = currentpos;
contRead = false; contRead = false;
break loop; break loop;
@ -649,7 +656,7 @@ public class HttpPostRequestDecoder {
// special case // special case
ampersandpos = currentpos; ampersandpos = currentpos;
if (ampersandpos > firstpos) { if (ampersandpos > firstpos) {
setFinalBuffer(undecodedChunk.slice(firstpos, ampersandpos - firstpos)); setFinalBuffer(undecodedChunk.slice(firstpos, ampersandpos - firstpos).retain());
} else if (!currentAttribute.isCompleted()) { } else if (!currentAttribute.isCompleted()) {
setFinalBuffer(EMPTY_BUFFER); setFinalBuffer(EMPTY_BUFFER);
} }
@ -660,7 +667,7 @@ public class HttpPostRequestDecoder {
if (contRead && currentAttribute != null) { if (contRead && currentAttribute != null) {
// reset index except if to continue in case of FIELD getStatus // reset index except if to continue in case of FIELD getStatus
if (currentStatus == MultiPartStatus.FIELD) { if (currentStatus == MultiPartStatus.FIELD) {
currentAttribute.addContent(undecodedChunk.slice(firstpos, currentpos - firstpos), false); currentAttribute.addContent(undecodedChunk.slice(firstpos, currentpos - firstpos).retain(), false);
firstpos = currentpos; firstpos = currentpos;
} }
undecodedChunk.readerIndex(firstpos); undecodedChunk.readerIndex(firstpos);
@ -1560,7 +1567,7 @@ public class HttpPostRequestDecoder {
} }
} }
} }
ByteBuf buffer = undecodedChunk.slice(readerIndex, lastPosition - readerIndex); ByteBuf buffer = undecodedChunk.slice(readerIndex, lastPosition - readerIndex).retain();
if (found) { if (found) {
// found so lastPosition is correct and final // found so lastPosition is correct and final
try { try {
@ -1678,7 +1685,7 @@ public class HttpPostRequestDecoder {
} }
} }
lastPosition = sao.getReadPosition(lastrealpos); lastPosition = sao.getReadPosition(lastrealpos);
ByteBuf buffer = undecodedChunk.slice(readerIndex, lastPosition - readerIndex); ByteBuf buffer = undecodedChunk.slice(readerIndex, lastPosition - readerIndex).retain();
if (found) { if (found) {
// found so lastPosition is correct and final // found so lastPosition is correct and final
try { try {
@ -1776,14 +1783,16 @@ public class HttpPostRequestDecoder {
// delimiter or simple one) // delimiter or simple one)
// so go back of delimiter size // so go back of delimiter size
try { try {
currentAttribute.addContent(undecodedChunk.slice(readerIndex, lastPosition - readerIndex), true); currentAttribute.addContent(
undecodedChunk.slice(readerIndex, lastPosition - readerIndex).retain(), true);
} catch (IOException e) { } catch (IOException e) {
throw new ErrorDataDecoderException(e); throw new ErrorDataDecoderException(e);
} }
undecodedChunk.readerIndex(lastPosition); undecodedChunk.readerIndex(lastPosition);
} else { } else {
try { try {
currentAttribute.addContent(undecodedChunk.slice(readerIndex, lastPosition - readerIndex), false); currentAttribute.addContent(
undecodedChunk.slice(readerIndex, lastPosition - readerIndex).retain(), false);
} catch (IOException e) { } catch (IOException e) {
throw new ErrorDataDecoderException(e); throw new ErrorDataDecoderException(e);
} }
@ -1879,14 +1888,16 @@ public class HttpPostRequestDecoder {
// delimiter or simple one) // delimiter or simple one)
// so go back of delimiter size // so go back of delimiter size
try { try {
currentAttribute.addContent(undecodedChunk.slice(readerIndex, lastPosition - readerIndex), true); currentAttribute.addContent(
undecodedChunk.slice(readerIndex, lastPosition - readerIndex).retain(), true);
} catch (IOException e) { } catch (IOException e) {
throw new ErrorDataDecoderException(e); throw new ErrorDataDecoderException(e);
} }
undecodedChunk.readerIndex(lastPosition); undecodedChunk.readerIndex(lastPosition);
} else { } else {
try { try {
currentAttribute.addContent(undecodedChunk.slice(readerIndex, lastPosition - readerIndex), false); currentAttribute.addContent(
undecodedChunk.slice(readerIndex, lastPosition - readerIndex).retain(), false);
} catch (IOException e) { } catch (IOException e) {
throw new ErrorDataDecoderException(e); throw new ErrorDataDecoderException(e);
} }
@ -2026,11 +2037,16 @@ public class HttpPostRequestDecoder {
return array; return array;
} }
@Override
public void remove() {
throw new UnsupportedOperationException();
}
/** /**
* Exception when try reading data from request in chunked format, and not * Exception when try reading data from request in chunked format, and not
* enough data are available (need more chunks) * enough data are available (need more chunks)
*/ */
public static class NotEnoughDataDecoderException extends Exception { public static class NotEnoughDataDecoderException extends DecoderException {
private static final long serialVersionUID = -7846841864603865638L; private static final long serialVersionUID = -7846841864603865638L;
public NotEnoughDataDecoderException() { public NotEnoughDataDecoderException() {
@ -2052,14 +2068,14 @@ public class HttpPostRequestDecoder {
/** /**
* Exception when the body is fully decoded, even if there is still data * Exception when the body is fully decoded, even if there is still data
*/ */
public static class EndOfDataDecoderException extends Exception { public static class EndOfDataDecoderException extends DecoderException {
private static final long serialVersionUID = 1336267941020800769L; private static final long serialVersionUID = 1336267941020800769L;
} }
/** /**
* Exception when an error occurs while decoding * Exception when an error occurs while decoding
*/ */
public static class ErrorDataDecoderException extends Exception { public static class ErrorDataDecoderException extends DecoderException {
private static final long serialVersionUID = 5020247425493164465L; private static final long serialVersionUID = 5020247425493164465L;
public ErrorDataDecoderException() { public ErrorDataDecoderException() {
@ -2081,7 +2097,7 @@ public class HttpPostRequestDecoder {
/** /**
* Exception when an unappropriated getMethod was called on a request * Exception when an unappropriated getMethod was called on a request
*/ */
public static class IncompatibleDataDecoderException extends Exception { public static class IncompatibleDataDecoderException extends DecoderException {
private static final long serialVersionUID = -953268047926250267L; private static final long serialVersionUID = -953268047926250267L;
public IncompatibleDataDecoderException() { public IncompatibleDataDecoderException() {

View File

@ -15,6 +15,7 @@
*/ */
package io.netty.example.http.upload; package io.netty.example.http.upload;
import io.netty.buffer.BufUtil;
import io.netty.buffer.ByteBuf; import io.netty.buffer.ByteBuf;
import io.netty.channel.Channel; import io.netty.channel.Channel;
import io.netty.channel.ChannelFuture; import io.netty.channel.ChannelFuture;
@ -297,6 +298,7 @@ public class HttpUploadServerHandler extends ChannelInboundMessageHandlerAdapter
} }
} }
} }
//BufUtil.release(data);
} }
private void writeResponse(Channel channel) { private void writeResponse(Channel channel) {