HttpPostRequestDecoder: retain instead of copy when first buf is last (#10209)

Motivations
-----------
There is no need to copy the "offered" ByteBuf in HttpPostRequestDecoder
when the first HttpContent ByteBuf is also the last (LastHttpContent) as
the full content can immediately be decoded. No extra bookeeping needed.

Modifications
-------------
HttpPostMultipartRequestDecoder
 - Retain the first ByteBuf when it is both the first HttpContent offered
to the decoder and is also LastHttpContent.
 - Retain slices of the final buffers values

Results
-------
ByteBufs of FullHttpMessage decoded by HttpPostRequestDecoder are no longer
unnecessarily copied. Attributes are extracted as retained slices when
the content is multi-part. Non-multi-part content continues to return
Unpooled buffers.

Partially addresses issue #10200

codec-http/src/main/java/io/netty/handler/codec/http/multipart/HttpPostMultipartRequestDecoder.java

@@ -321,18 +321,23 @@ public class HttpPostMultipartRequestDecoder implements InterfaceHttpPostRequest
     public HttpPostMultipartRequestDecoder offer(HttpContent content) {
         checkDestroyed();
 
+        if (content instanceof LastHttpContent) {
+            isLastChunk = true;
+        }
+
+        ByteBuf buf = content.content();
+        if (undecodedChunk == null) {
+            undecodedChunk = isLastChunk
+                // Take a slice instead of copying when the first chunk is also the last
+                // as undecodedChunk.writeBytes will never be called.
+                ? buf.retainedSlice()
                 // Maybe we should better not copy here for performance reasons but this will need
                 // more care by the caller to release the content in a correct manner later
                 // So maybe something to optimize on a later stage
-        ByteBuf buf = content.content();
+                : buf.copy();
-        if (undecodedChunk == null) {
-            undecodedChunk = buf.copy();
         } else {
             undecodedChunk.writeBytes(buf);
         }
-        if (content instanceof LastHttpContent) {
-            isLastChunk = true;
-        }
         parseBody();
         if (undecodedChunk != null && undecodedChunk.writerIndex() > discardThreshold) {
             undecodedChunk.discardReadBytes();
@@ -1315,7 +1320,7 @@ public class HttpPostMultipartRequestDecoder implements InterfaceHttpPostRequest
         if (prevByte == HttpConstants.CR) {
             lastPosition--;
         }
-        ByteBuf content = undecodedChunk.copy(startReaderIndex, lastPosition - startReaderIndex);
+        ByteBuf content = undecodedChunk.retainedSlice(startReaderIndex, lastPosition - startReaderIndex);
         try {
             httpData.addContent(content, delimiterFound);
         } catch (IOException e) {
@@ -1364,7 +1369,7 @@ public class HttpPostMultipartRequestDecoder implements InterfaceHttpPostRequest
             lastRealPos--;
         }
         final int lastPosition = sao.getReadPosition(lastRealPos);
-        final ByteBuf content = undecodedChunk.copy(startReaderIndex, lastPosition - startReaderIndex);
+        final ByteBuf content = undecodedChunk.retainedSlice(startReaderIndex, lastPosition - startReaderIndex);
         try {
             httpData.addContent(content, delimiterFound);
         } catch (IOException e) {

codec-http/src/main/java/io/netty/handler/codec/http/multipart/HttpPostStandardRequestDecoder.java

@@ -26,7 +26,10 @@ import io.netty.handler.codec.http.multipart.HttpPostRequestDecoder.EndOfDataDec
 import io.netty.handler.codec.http.multipart.HttpPostRequestDecoder.ErrorDataDecoderException;
 import io.netty.handler.codec.http.multipart.HttpPostRequestDecoder.MultiPartStatus;
 import io.netty.handler.codec.http.multipart.HttpPostRequestDecoder.NotEnoughDataDecoderException;
+import io.netty.util.ByteProcessor;
+import io.netty.util.CharsetUtil;
 import io.netty.util.internal.PlatformDependent;
+import io.netty.util.internal.StringUtil;
 
 import java.io.IOException;
 import java.nio.charset.Charset;
@@ -280,18 +283,23 @@ public class HttpPostStandardRequestDecoder implements InterfaceHttpPostRequestD
     public HttpPostStandardRequestDecoder offer(HttpContent content) {
         checkDestroyed();
 
+        if (content instanceof LastHttpContent) {
+            isLastChunk = true;
+        }
+
+        ByteBuf buf = content.content();
+        if (undecodedChunk == null) {
+            undecodedChunk = isLastChunk
+                // Take a slice instead of copying when the first chunk is also the last
+                // as undecodedChunk.writeBytes will never be called.
+                ? buf.retainedSlice()
                 // Maybe we should better not copy here for performance reasons but this will need
                 // more care by the caller to release the content in a correct manner later
                 // So maybe something to optimize on a later stage
-        ByteBuf buf = content.content();
+                : buf.copy();
-        if (undecodedChunk == null) {
-            undecodedChunk = buf.copy();
         } else {
             undecodedChunk.writeBytes(buf);
         }
-        if (content instanceof LastHttpContent) {
-            isLastChunk = true;
-        }
         parseBody();
         if (undecodedChunk != null && undecodedChunk.writerIndex() > discardThreshold) {
             undecodedChunk.discardReadBytes();
@@ -429,7 +437,7 @@ public class HttpPostStandardRequestDecoder implements InterfaceHttpPostRequestD
                     if (read == '&') {
                         currentStatus = MultiPartStatus.DISPOSITION;
                         ampersandpos = currentpos - 1;
-                        setFinalBuffer(undecodedChunk.copy(firstpos, ampersandpos - firstpos));
+                        setFinalBuffer(undecodedChunk.retainedSlice(firstpos, ampersandpos - firstpos));
                         firstpos = currentpos;
                         contRead = true;
                     } else if (read == HttpConstants.CR) {
@@ -439,7 +447,7 @@ public class HttpPostStandardRequestDecoder implements InterfaceHttpPostRequestD
                             if (read == HttpConstants.LF) {
                                 currentStatus = MultiPartStatus.PREEPILOGUE;
                                 ampersandpos = currentpos - 2;
-                                setFinalBuffer(undecodedChunk.copy(firstpos, ampersandpos - firstpos));
+                                setFinalBuffer(undecodedChunk.retainedSlice(firstpos, ampersandpos - firstpos));
                                 firstpos = currentpos;
                                 contRead = false;
                             } else {
@@ -452,7 +460,7 @@ public class HttpPostStandardRequestDecoder implements InterfaceHttpPostRequestD
                     } else if (read == HttpConstants.LF) {
                         currentStatus = MultiPartStatus.PREEPILOGUE;
                         ampersandpos = currentpos - 1;
-                        setFinalBuffer(undecodedChunk.copy(firstpos, ampersandpos - firstpos));
+                        setFinalBuffer(undecodedChunk.retainedSlice(firstpos, ampersandpos - firstpos));
                         firstpos = currentpos;
                         contRead = false;
                     }
@@ -466,7 +474,7 @@ public class HttpPostStandardRequestDecoder implements InterfaceHttpPostRequestD
                 // special case
                 ampersandpos = currentpos;
                 if (ampersandpos > firstpos) {
-                    setFinalBuffer(undecodedChunk.copy(firstpos, ampersandpos - firstpos));
+                    setFinalBuffer(undecodedChunk.retainedSlice(firstpos, ampersandpos - firstpos));
                 } else if (!currentAttribute.isCompleted()) {
                     setFinalBuffer(EMPTY_BUFFER);
                 }
@@ -474,7 +482,7 @@ public class HttpPostStandardRequestDecoder implements InterfaceHttpPostRequestD
                 currentStatus = MultiPartStatus.EPILOGUE;
             } else if (contRead && currentAttribute != null && currentStatus == MultiPartStatus.FIELD) {
                 // reset index except if to continue in case of FIELD getStatus
-                currentAttribute.addContent(undecodedChunk.copy(firstpos, currentpos - firstpos),
+                currentAttribute.addContent(undecodedChunk.retainedSlice(firstpos, currentpos - firstpos),
                                             false);
                 firstpos = currentpos;
             }
@@ -546,7 +554,7 @@ public class HttpPostStandardRequestDecoder implements InterfaceHttpPostRequestD
                     if (read == '&') {
                         currentStatus = MultiPartStatus.DISPOSITION;
                         ampersandpos = currentpos - 1;
-                        setFinalBuffer(undecodedChunk.copy(firstpos, ampersandpos - firstpos));
+                        setFinalBuffer(undecodedChunk.retainedSlice(firstpos, ampersandpos - firstpos));
                         firstpos = currentpos;
                         contRead = true;
                     } else if (read == HttpConstants.CR) {
@@ -557,7 +565,7 @@ public class HttpPostStandardRequestDecoder implements InterfaceHttpPostRequestD
                                 currentStatus = MultiPartStatus.PREEPILOGUE;
                                 ampersandpos = currentpos - 2;
                                 sao.setReadPosition(0);
-                                setFinalBuffer(undecodedChunk.copy(firstpos, ampersandpos - firstpos));
+                                setFinalBuffer(undecodedChunk.retainedSlice(firstpos, ampersandpos - firstpos));
                                 firstpos = currentpos;
                                 contRead = false;
                                 break loop;
@@ -575,7 +583,7 @@ public class HttpPostStandardRequestDecoder implements InterfaceHttpPostRequestD
                         currentStatus = MultiPartStatus.PREEPILOGUE;
                         ampersandpos = currentpos - 1;
                         sao.setReadPosition(0);
-                        setFinalBuffer(undecodedChunk.copy(firstpos, ampersandpos - firstpos));
+                        setFinalBuffer(undecodedChunk.retainedSlice(firstpos, ampersandpos - firstpos));
                         firstpos = currentpos;
                         contRead = false;
                         break loop;
@@ -592,7 +600,7 @@ public class HttpPostStandardRequestDecoder implements InterfaceHttpPostRequestD
                 // special case
                 ampersandpos = currentpos;
                 if (ampersandpos > firstpos) {
-                    setFinalBuffer(undecodedChunk.copy(firstpos, ampersandpos - firstpos));
+                    setFinalBuffer(undecodedChunk.retainedSlice(firstpos, ampersandpos - firstpos));
                 } else if (!currentAttribute.isCompleted()) {
                     setFinalBuffer(EMPTY_BUFFER);
                 }
@@ -600,7 +608,7 @@ public class HttpPostStandardRequestDecoder implements InterfaceHttpPostRequestD
                 currentStatus = MultiPartStatus.EPILOGUE;
             } else if (contRead && currentAttribute != null && currentStatus == MultiPartStatus.FIELD) {
                 // reset index except if to continue in case of FIELD getStatus
-                currentAttribute.addContent(undecodedChunk.copy(firstpos, currentpos - firstpos),
+                currentAttribute.addContent(undecodedChunk.retainedSlice(firstpos, currentpos - firstpos),
                                             false);
                 firstpos = currentpos;
             }
@@ -622,8 +630,10 @@ public class HttpPostStandardRequestDecoder implements InterfaceHttpPostRequestD
 
     private void setFinalBuffer(ByteBuf buffer) throws IOException {
         currentAttribute.addContent(buffer, true);
-        String value = decodeAttribute(currentAttribute.getByteBuf().toString(charset), charset);
+        ByteBuf decodedBuf = decodeAttribute(currentAttribute.getByteBuf(), charset);
-        currentAttribute.setValue(value);
+        if (decodedBuf != null) { // override content only when ByteBuf needed decoding
+            currentAttribute.setContent(decodedBuf);
+        }
         addHttpData(currentAttribute);
         currentAttribute = null;
     }
@@ -641,6 +651,28 @@ public class HttpPostStandardRequestDecoder implements InterfaceHttpPostRequestD
         }
     }
 
+    private static ByteBuf decodeAttribute(ByteBuf b, Charset charset) {
+        int firstEscaped = b.forEachByte(new ByteProcessor.IndexOfProcessor((byte) '%'));
+        if (firstEscaped == -1) {
+            return null; // nothing to decode
+        }
+
+        ByteBuf buf = b.alloc().buffer(b.readableBytes());
+        UrlDecoder urlDecode = new UrlDecoder(buf);
+        int idx = b.forEachByte(urlDecode);
+        if (urlDecode.nextEscapedIdx != 0) { // incomplete hex byte
+            if (idx == -1) {
+                idx = b.readableBytes() - 1;
+            }
+            idx -= urlDecode.nextEscapedIdx - 1;
+            buf.release();
+            throw new ErrorDataDecoderException(
+                String.format("Invalid hex byte at index '%d' in string: '%s'", idx, b.toString(charset)));
+        }
+
+        return buf;
+    }
+
     /**
      * Destroy the {@link HttpPostStandardRequestDecoder} and release all it resources. After this method
      * was called it is not possible to operate on it anymore.
@@ -677,4 +709,39 @@ public class HttpPostStandardRequestDecoder implements InterfaceHttpPostRequestD
 
         factory.removeHttpDataFromClean(request, data);
     }
+
+    private static final class UrlDecoder implements ByteProcessor {
+
+        private final ByteBuf output;
+        private int nextEscapedIdx;
+        private byte hiByte;
+
+        UrlDecoder(ByteBuf output) {
+            this.output = output;
+        }
+
+        @Override
+        public boolean process(byte value) {
+            if (nextEscapedIdx != 0) {
+                if (nextEscapedIdx == 1) {
+                    hiByte = value;
+                    ++nextEscapedIdx;
+                } else {
+                    int hi = StringUtil.decodeHexNibble((char) hiByte);
+                    int lo = StringUtil.decodeHexNibble((char) value);
+                    if (hi == -1 || lo == -1) {
+                        ++nextEscapedIdx;
+                        return false;
+                    }
+                    output.writeByte((hi << 4) + lo);
+                    nextEscapedIdx = 0;
+                }
+            } else if (value == '%') {
+                nextEscapedIdx = 1;
+            } else {
+                output.writeByte(value);
+            }
+            return true;
+        }
+    }
 }

codec-http/src/test/java/io/netty/handler/codec/http/multipart/HttpPostRequestDecoderTest.java

@@ -25,7 +25,6 @@ import io.netty.handler.codec.http.DefaultHttpContent;
 import io.netty.handler.codec.http.DefaultHttpRequest;
 import io.netty.handler.codec.http.DefaultLastHttpContent;
 import io.netty.handler.codec.http.FullHttpRequest;
-import io.netty.handler.codec.http.HttpContent;
 import io.netty.handler.codec.http.HttpHeaderNames;
 import io.netty.handler.codec.http.HttpHeaderValues;
 import io.netty.handler.codec.http.HttpMethod;
@@ -39,13 +38,11 @@ import java.net.URLEncoder;
 import java.nio.charset.UnsupportedCharsetException;
 import java.util.Arrays;
 
-import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.*;
-import static org.junit.Assert.assertFalse;
-import static org.junit.Assert.assertNotNull;
-import static org.junit.Assert.assertTrue;
-import static org.junit.Assert.fail;
 
-/** {@link HttpPostRequestDecoder} test case. */
+/**
+ * {@link HttpPostRequestDecoder} test case.
+ */
 public class HttpPostRequestDecoderTest {
 
     @Test
@@ -315,19 +312,37 @@ public class HttpPostRequestDecoderTest {
         int firstChunk = 10;
         int middleChunk = 1024;
 
-        HttpContent part1 = new DefaultHttpContent(Unpooled.wrappedBuffer(
+        byte[] payload1 = payload.substring(0, firstChunk).getBytes();
-                payload.substring(0, firstChunk).getBytes()));
+        byte[] payload2 = payload.substring(firstChunk, firstChunk + middleChunk).getBytes();
-        HttpContent part2 = new DefaultHttpContent(Unpooled.wrappedBuffer(
+        byte[] payload3 = payload.substring(firstChunk + middleChunk, firstChunk + middleChunk * 2).getBytes();
-                payload.substring(firstChunk, firstChunk + middleChunk).getBytes()));
+        byte[] payload4 = payload.substring(firstChunk + middleChunk * 2).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);
+        ByteBuf buf1 = Unpooled.directBuffer(payload1.length);
-        decoder.offer(part2);
+        ByteBuf buf2 = Unpooled.directBuffer(payload2.length);
-        decoder.offer(part3);
+        ByteBuf buf3 = Unpooled.directBuffer(payload3.length);
-        decoder.offer(part4);
+        ByteBuf buf4 = Unpooled.directBuffer(payload4.length);
+
+        buf1.writeBytes(payload1);
+        buf2.writeBytes(payload2);
+        buf3.writeBytes(payload3);
+        buf4.writeBytes(payload4);
+
+        decoder.offer(new DefaultHttpContent(buf1));
+        decoder.offer(new DefaultHttpContent(buf2));
+        decoder.offer(new DefaultHttpContent(buf3));
+        decoder.offer(new DefaultLastHttpContent(buf4));
+
+        assertFalse(decoder.getBodyHttpDatas().isEmpty());
+        assertEquals(139, decoder.getBodyHttpDatas().size());
+
+        Attribute attr = (Attribute) decoder.getBodyHttpData("town");
+        assertEquals("794649819", attr.getValue());
+
+        decoder.destroy();
+        buf1.release();
+        buf2.release();
+        buf3.release();
+        buf4.release();
     }
 
     // See https://github.com/netty/netty/issues/3326
@@ -539,8 +554,8 @@ public class HttpPostRequestDecoderTest {
         String filenameEncoded = URLEncoder.encode(filename, encoding);
 
         final String body = "--" + boundary + "\r\n" +
-          "Content-Disposition: form-data; name=\"file\"; filename*=" + encoding + "''" + filenameEncoded + "\r\n" +
+                "Content-Disposition: form-data; name=\"file\"; filename*=" + encoding + "''" + filenameEncoded +
-          "\r\n" +
+                "\r\n\r\n" +
                 "foo\r\n" +
                 "\r\n" +
                 "--" + boundary + "--";
@@ -697,7 +712,7 @@ public class HttpPostRequestDecoderTest {
     public void testMultipartRequest() throws Exception {
         String BOUNDARY = "01f136d9282f";
 
-        ByteBuf byteBuf = Unpooled.wrappedBuffer(("--" + BOUNDARY + "\n" +
+        byte[] bodyBytes = ("--" + BOUNDARY + "\n" +
             "Content-Disposition: form-data; name=\"msg_id\"\n" +
             "\n" +
             "15200\n" +
@@ -705,7 +720,9 @@ public class HttpPostRequestDecoderTest {
             "Content-Disposition: form-data; name=\"msg\"\n" +
             "\n" +
             "test message\n" +
-                "--" + BOUNDARY + "--").getBytes());
+            "--" + BOUNDARY + "--").getBytes();
+        ByteBuf byteBuf = Unpooled.directBuffer(bodyBytes.length);
+        byteBuf.writeBytes(bodyBytes);
 
         FullHttpRequest req = new DefaultFullHttpRequest(HttpVersion.HTTP_1_0, HttpMethod.POST, "/up", byteBuf);
         req.headers().add(HttpHeaderNames.CONTENT_TYPE, "multipart/form-data; boundary=" + BOUNDARY);
@@ -718,11 +735,16 @@ public class HttpPostRequestDecoderTest {
         assertTrue(decoder.isMultipart());
         assertFalse(decoder.getBodyHttpDatas().isEmpty());
         assertEquals(2, decoder.getBodyHttpDatas().size());
-        assertEquals("test message", ((Attribute) decoder.getBodyHttpData("msg")).getValue());
+
-        assertEquals("15200", ((Attribute) decoder.getBodyHttpData("msg_id")).getValue());
+        Attribute attrMsg = (Attribute) decoder.getBodyHttpData("msg");
+        assertTrue(attrMsg.getByteBuf().isDirect());
+        assertEquals("test message", attrMsg.getValue());
+        Attribute attrMsgId = (Attribute) decoder.getBodyHttpData("msg_id");
+        assertTrue(attrMsgId.getByteBuf().isDirect());
+        assertEquals("15200", attrMsgId.getValue());
 
         decoder.destroy();
-        assertEquals(1, req.refCnt());
+        assertTrue(req.release());
     }
 
     @Test(expected = HttpPostRequestDecoder.ErrorDataDecoderException.class)
@@ -747,7 +769,7 @@ public class HttpPostRequestDecoderTest {
     }
 
     private static void testNotLeakWhenWrapIllegalArgumentException(ByteBuf buf) {
-        buf.writeCharSequence("==", CharsetUtil.US_ASCII);
+        buf.writeCharSequence("a=b&foo=%22bar%22&==", CharsetUtil.US_ASCII);
         FullHttpRequest request = new DefaultFullHttpRequest(HttpVersion.HTTP_1_1, HttpMethod.POST, "/", buf);
         try {
             new HttpPostStandardRequestDecoder(request);
@@ -800,7 +822,109 @@ public class HttpPostRequestDecoderTest {
         assertTrue("the item should be a FileUpload", part1 instanceof FileUpload);
         FileUpload fileUpload = (FileUpload) part1;
         assertEquals("the filename should be decoded", filename, fileUpload.getFilename());
+
         decoder.destroy();
         req.release();
     }
+
+    @Test
+    public void testDecodeFullHttpRequestWithUrlEncodedBody() throws Exception {
+        byte[] bodyBytes = "foo=bar&a=b&empty=&city=%3c%22new%22%20york%20city%3e".getBytes();
+        ByteBuf content = Unpooled.directBuffer(bodyBytes.length);
+        content.writeBytes(bodyBytes);
+
+        FullHttpRequest req = new DefaultFullHttpRequest(HttpVersion.HTTP_1_1, HttpMethod.POST, "/", content);
+        HttpPostRequestDecoder decoder = new HttpPostRequestDecoder(req);
+        assertFalse(decoder.getBodyHttpDatas().isEmpty());
+
+        assertFalse(decoder.getBodyHttpDatas().isEmpty());
+        assertEquals(4, decoder.getBodyHttpDatas().size());
+
+        Attribute attr = (Attribute) decoder.getBodyHttpData("foo");
+        assertTrue(attr.getByteBuf().isDirect());
+        assertEquals("bar", attr.getValue());
+
+        attr = (Attribute) decoder.getBodyHttpData("a");
+        assertTrue(attr.getByteBuf().isDirect());
+        assertEquals("b", attr.getValue());
+
+        attr = (Attribute) decoder.getBodyHttpData("empty");
+        assertTrue(attr.getByteBuf().isDirect());
+        assertEquals("", attr.getValue());
+
+        attr = (Attribute) decoder.getBodyHttpData("city");
+        assertTrue(attr.getByteBuf().isDirect());
+        assertEquals("<\"new\" york city>", attr.getValue());
+
+        decoder.destroy();
+        req.release();
+    }
+
+    @Test
+    public void testDecodeFullHttpRequestWithUrlEncodedBodyWithBrokenHexByte0() {
+        byte[] bodyBytes = "foo=bar&a=b&empty=%&city=paris".getBytes();
+        ByteBuf content = Unpooled.directBuffer(bodyBytes.length);
+        content.writeBytes(bodyBytes);
+
+        FullHttpRequest req = new DefaultFullHttpRequest(HttpVersion.HTTP_1_1, HttpMethod.POST, "/", content);
+        try {
+            new HttpPostRequestDecoder(req);
+            fail("Was expecting an ErrorDataDecoderException");
+        } catch (HttpPostRequestDecoder.ErrorDataDecoderException e) {
+            assertEquals("Invalid hex byte at index '0' in string: '%'", e.getMessage());
+        } finally {
+            req.release();
+        }
+    }
+
+    @Test
+    public void testDecodeFullHttpRequestWithUrlEncodedBodyWithBrokenHexByte1() {
+        byte[] bodyBytes = "foo=bar&a=b&empty=%2&city=london".getBytes();
+        ByteBuf content = Unpooled.directBuffer(bodyBytes.length);
+        content.writeBytes(bodyBytes);
+
+        FullHttpRequest req = new DefaultFullHttpRequest(HttpVersion.HTTP_1_1, HttpMethod.POST, "/", content);
+        try {
+            new HttpPostRequestDecoder(req);
+            fail("Was expecting an ErrorDataDecoderException");
+        } catch (HttpPostRequestDecoder.ErrorDataDecoderException e) {
+            assertEquals("Invalid hex byte at index '0' in string: '%2'", e.getMessage());
+        } finally {
+            req.release();
+        }
+    }
+
+    @Test
+    public void testDecodeFullHttpRequestWithUrlEncodedBodyWithInvalidHexNibbleHi() {
+        byte[] bodyBytes = "foo=bar&a=b&empty=%Zc&city=london".getBytes();
+        ByteBuf content = Unpooled.directBuffer(bodyBytes.length);
+        content.writeBytes(bodyBytes);
+
+        FullHttpRequest req = new DefaultFullHttpRequest(HttpVersion.HTTP_1_1, HttpMethod.POST, "/", content);
+        try {
+            new HttpPostRequestDecoder(req);
+            fail("Was expecting an ErrorDataDecoderException");
+        } catch (HttpPostRequestDecoder.ErrorDataDecoderException e) {
+            assertEquals("Invalid hex byte at index '0' in string: '%Zc'", e.getMessage());
+        } finally {
+            req.release();
+        }
+    }
+
+    @Test
+    public void testDecodeFullHttpRequestWithUrlEncodedBodyWithInvalidHexNibbleLo() {
+        byte[] bodyBytes = "foo=bar&a=b&empty=%2g&city=london".getBytes();
+        ByteBuf content = Unpooled.directBuffer(bodyBytes.length);
+        content.writeBytes(bodyBytes);
+
+        FullHttpRequest req = new DefaultFullHttpRequest(HttpVersion.HTTP_1_1, HttpMethod.POST, "/", content);
+        try {
+            new HttpPostRequestDecoder(req);
+            fail("Was expecting an ErrorDataDecoderException");
+        } catch (HttpPostRequestDecoder.ErrorDataDecoderException e) {
+            assertEquals("Invalid hex byte at index '0' in string: '%2g'", e.getMessage());
+        } finally {
+            req.release();
+        }
+    }
 }