Repository: netty/netty
Issue: HttpConversionUtil - Incorrect conversion of Cookie header
Labels: defect

Netty Version: master (latest snapshot: a6816bd59ef2923d82af5b7a7f3b722507a56a3a)

The browser (I used Chrome 46.0.2490.80 for testing) sent an HTTP/2 request containing multiple cookies to a netty service, the `HttpConversionUtil` generated multiple `Cookie` header entries in the HTTP/1.x request. According to [one stackoverflow post](http://stackoverflow.com/questions/16305814/are-multiple-cookie-headers-allowed-in-an-http-request), this is incorrect behaviour. The `HttpConversionUtil` should generate a single `Cookie` header containing all cookie values.

// cc @Scottmitch @nmittler - I could provide a patch, but I'm not too sure the best approach to take. Considerations about how to extract the cookies, and reformat them into a single header have to be made. If you could give me some tips, I might be able to come up with a PR.


Issue comments:
@Scottmitch @nmittler please have a look guys :)

I wonder if `HttpConversionUtil` should just group all multi-valued headers into single entries (i.e. not treat `Cookie` as special here). It wouldn't be invalid and I suspect would perform better end-to-end. 

@Scottmitch WDYT?

@nmittler +1

@nmittler - As far as I can tell, that won't work.  What is true for one header is not necessarily true for others. Specifically I can think of the `Set-Cookie` header.  In this case (depending which version/browser you use) sending each cookie in a separate `Set-Cookie` header is actually the correct thing to do (log into or out of facebook to see an example of this).

Yup, looking at https://tools.ietf.org/html/rfc7230#section-3.2.2 ...

`Set-Cooke` is the exception.  It also sounds for Netty to function as a proxy, it should not merge headers.

And as described in http://tools.ietf.org/html/rfc6265#section-5.4: `the user agent MUST NOT attach more than one Cookie header field`.

@blucas Are you saying that Chrome is sending multiple `Cookie` headers, or that it's sending a multi-valued `Cookie` header that Netty is then splitting into separate headers? The former would appear to be a chrome bug.

+1 for not merging headers :smiley: 

@nmittler I haven't checked that yet. My assumption is that it is a Netty related issue, as Firefox is also affected by this.  I will check now and get back to you

Ok, so I still don't know if it is the browser's or netty's (possibly twitter hpack's) fault.  All I can tell you is
- Both Firefox and Chrome exhibit the issue
- Single call to [addFragment](https://github.com/netty/netty/blob/master/codec-http2/src/main/java/io/netty/handler/codec/http2/DefaultHttp2FrameReader.java#L669) which has `endOfHeaders` set to true.

Maybe @Scottmitch could shed some light on this one?

BTW, looking at chrome://net-internals for the request generated this stack:

```
t= 38394 [st= 35693]    HTTP2_SESSION_SEND_HEADERS
                        --> fin = true
                        --> :authority: www.somesite.net
                            :method: GET
                            :path: /managemain/menu.cfm
                            :scheme: https
                            accept: text/html,application/xhtml+xml,application/xml;q=0.9,image/webp,*/*;q=0.8
                            accept-encoding: gzip, deflate, sdch
                            accept-language: en-US,en;q=0.8
                            cache-control: max-age=0
                            cookie: [118 bytes were stripped]
                            upgrade-insecure-requests: 1
                            user-agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_7_5) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/46.0.2490.80 Safari/537.36
                        --> priority = 0
                        --> stream_id = 3
                        --> unidirectional = false
```

Chrome did the `[118 bytes were stripped]` not me.

@blucas - Can you post your server code somewhere (or some reduced set of it)? I just modified the example http2 helloworld server and client to both use the HTTP/1.x translation layer and added a cookie from the client (`request.headers().add(HttpHeaderNames.COOKIE, "foo1, foo2");`) and this is what I see on the server side:

```
DefaultFullHttpRequest(decodeResult: success, version: HTTP/1.1, content: UnpooledUnsafeHeapByteBuf(ridx: 0, widx: 0, cap: 0))
GET /whatever HTTP/1.1
x-http2-scheme: http
host: 127.0.0.1:8080
accept-encoding: gzip
accept-encoding: deflate
cookie: foo1, foo2
x-http2-stream-id: 3
content-length: 0
```

It looks like CSV headers are preserved (`cookie`), and multiple headers as different entries (`accept-encoding`) are preserved.  Are you thinking we should always force everything to be a single CSV separated value unless otherwise prohibited from doing this by the spec?

@Scottmitch - I don't have a reproducer handy for you to test with, but all you need to do is make your HTTP/2 client send multiple `cookie` headers to your netty HTTP/2 -> HTTP/1.x translation server.

> Are you thinking we should always force everything to be a single CSV separated value unless otherwise prohibited from doing this by the spec?

No. We shouldn't force everything to use single CSV.

I took a look at the HTTP/2 spec, and finally think I've found what I'm trying to explain. 

> To allow for better compression efficiency, the Cookie header field MAY be split into separate header fields, each with one or more cookie-pairs.

So, according to the [spec](https://http2.github.io/http2-spec/#CompressCookie), it is perfectly valid for a HTTP/2 client to send multiple `cookie` headers (thus Firefox and Chrome are complying with the spec).  //cc @nmittler - I believe this answers your question.

> If there are multiple Cookie header fields after decompression, these MUST be concatenated into a single octet string using the two-octet delimiter of 0x3B, 0x20 (the ASCII string "; ") before being passed into a non-HTTP/2 context, such as an HTTP/1.1 connection, or a generic HTTP server application.

So I interpret that as Netty's `HttpConversionUtil.Http2ToHttpHeaderTranslator` should be responsible for concatenating one or more HTTP/2 `cookie` headers into a single HTTP/1.x `Cookie` header.

```
// Example 1 - Client sends multiple single value HTTP/2 cookie headers
cookie: a=b
cookie: c=d
cookie: e=f

// Netty should translate that to a single HTTP/1.x Cookie header
cookie: a=b; c=d; e=f

// Example 2 - Client sends single and multi-value HTTP/2 cookie headers
cookie: a=b; c=d
cookie: e=f

// Netty should translate that to a single HTTP/1.x Cookie header
cookie: a=b; c=d; e=f
```

I hope that makes more sense :smiley: 

@Scottmitch - gentle ping :)

@Scottmitch ^^ 

@blucas - Sorry for the delay and thank you for the reference to the specification. I will submit a PR soon.


Pull request review comments:
yikes ... was this missing? :)

Not new code, but why does the `if` statement above check for the value of `trailers`? Seems like checking the name equals `te` is sufficient, no?

final?

yikes :)

Are we guaranteed that we need to skip exactly 2 characters?

Maybe check for the exceptional case first? Then you don't need the else.

Yes sir it was :(

We were previously using all `AsciiString` in tests apparently so the value comparisons using POJO `equals` was sufficient.

It is defined/declared in an interface, so it is final by definition.

> /Users/scott_mitchell/git/netty/common/src/main/java/io/netty/util/ByteProcessor.java:126:5: Redundant 'final' modifier.

https://tools.ietf.org/html/rfc7540#section-8.1.2.2

> The only exception to this is the TE header field, which MAY be
>    present in an HTTP/2 request; when it is, it MUST NOT contain any
>    value other than "trailers".

Yip :frowning: A few of the new unit tests now test this.

Right, I'm just not sure why this logic cares about the value.  It would we would only care if we were going to enforce that requirement (i.e. throw if the value is NOT `trailers`).

I believe so. Let me add a reference to the cookie RFC to clarify.

https://tools.ietf.org/html/rfc6265#section-4.2.1

> cookie-header = "Cookie:" OWS cookie-string OWS
>    cookie-string = cookie-pair *( ";" SP cookie-pair )

https://tools.ietf.org/html/rfc6265#section-4.1.1

> cookie-pair       = cookie-name "=" cookie-value

ah right ... didn't notice that this was an interface. Sorry for the noise :)

done

I guess we are enforcing it by not including it in the resulting headers, but we can throw to be more explicit if you prefer.

I think throwing is probably the right thing to do ... otherwise, AFAICT we're silently dropping this header on the floor. WDYT?

done.


Patch that resolved the issue:
diff --git a/codec-http2/src/main/java/io/netty/handler/codec/http2/DefaultHttp2Headers.java b/codec-http2/src/main/java/io/netty/handler/codec/http2/DefaultHttp2Headers.java
index 5b54fd61877..63bb5617872 100644
--- a/codec-http2/src/main/java/io/netty/handler/codec/http2/DefaultHttp2Headers.java
+++ b/codec-http2/src/main/java/io/netty/handler/codec/http2/DefaultHttp2Headers.java
@@ -14,17 +14,16 @@
  */
 package io.netty.handler.codec.http2;
 
+import static io.netty.handler.codec.http2.Http2Error.PROTOCOL_ERROR;
+import static io.netty.handler.codec.http2.Http2Exception.connectionError;
+import static io.netty.util.AsciiString.CASE_SENSITIVE_HASHER;
+import static io.netty.util.AsciiString.isUpperCase;
 import io.netty.handler.codec.CharSequenceValueConverter;
 import io.netty.handler.codec.DefaultHeaders;
 import io.netty.util.AsciiString;
 import io.netty.util.ByteProcessor;
 import io.netty.util.internal.PlatformDependent;
 
-import static io.netty.handler.codec.http2.Http2Error.PROTOCOL_ERROR;
-import static io.netty.handler.codec.http2.Http2Exception.connectionError;
-import static io.netty.util.AsciiString.CASE_SENSITIVE_HASHER;
-import static io.netty.util.AsciiString.isUpperCase;
-
 public class DefaultHttp2Headers
         extends DefaultHeaders<CharSequence, CharSequence, Http2Headers> implements Http2Headers {
     private static final ByteProcessor HTTP2_NAME_VALIDATOR_PROCESSOR = new ByteProcessor() {
@@ -113,6 +112,20 @@ public Http2Headers clear() {
         return super.clear();
     }
 
+    @Override
+    public boolean equals(Object o) {
+        if (!(o instanceof Http2Headers)) {
+            return false;
+        }
+
+        return equals((Http2Headers) o, CASE_SENSITIVE_HASHER);
+    }
+
+    @Override
+    public int hashCode() {
+        return hashCode(CASE_SENSITIVE_HASHER);
+    }
+
     @Override
     public Http2Headers method(CharSequence value) {
         set(PseudoHeaderName.METHOD.value(), value);
diff --git a/codec-http2/src/main/java/io/netty/handler/codec/http2/HttpConversionUtil.java b/codec-http2/src/main/java/io/netty/handler/codec/http2/HttpConversionUtil.java
index 87e2123e14b..c81166490d2 100644
--- a/codec-http2/src/main/java/io/netty/handler/codec/http2/HttpConversionUtil.java
+++ b/codec-http2/src/main/java/io/netty/handler/codec/http2/HttpConversionUtil.java
@@ -14,6 +14,18 @@
  */
 package io.netty.handler.codec.http2;
 
+import static io.netty.handler.codec.http.HttpScheme.HTTP;
+import static io.netty.handler.codec.http.HttpScheme.HTTPS;
+import static io.netty.handler.codec.http.HttpUtil.isAsteriskForm;
+import static io.netty.handler.codec.http.HttpUtil.isOriginForm;
+import static io.netty.handler.codec.http2.Http2Error.PROTOCOL_ERROR;
+import static io.netty.handler.codec.http2.Http2Exception.connectionError;
+import static io.netty.handler.codec.http2.Http2Exception.streamError;
+import static io.netty.util.AsciiString.EMPTY_STRING;
+import static io.netty.util.ByteProcessor.FIND_SEMI_COLON;
+import static io.netty.util.internal.ObjectUtil.checkNotNull;
+import static io.netty.util.internal.StringUtil.isNullOrEmpty;
+import static io.netty.util.internal.StringUtil.length;
 import io.netty.handler.codec.http.DefaultFullHttpRequest;
 import io.netty.handler.codec.http.DefaultFullHttpResponse;
 import io.netty.handler.codec.http.FullHttpMessage;
@@ -35,18 +47,6 @@
 import java.util.Iterator;
 import java.util.Map.Entry;
 
-import static io.netty.handler.codec.http.HttpScheme.HTTP;
-import static io.netty.handler.codec.http.HttpScheme.HTTPS;
-import static io.netty.handler.codec.http.HttpUtil.isAsteriskForm;
-import static io.netty.handler.codec.http.HttpUtil.isOriginForm;
-import static io.netty.handler.codec.http2.Http2Error.PROTOCOL_ERROR;
-import static io.netty.handler.codec.http2.Http2Exception.connectionError;
-import static io.netty.handler.codec.http2.Http2Exception.streamError;
-import static io.netty.util.AsciiString.EMPTY_STRING;
-import static io.netty.util.internal.ObjectUtil.checkNotNull;
-import static io.netty.util.internal.StringUtil.isNullOrEmpty;
-import static io.netty.util.internal.StringUtil.length;
-
 /**
  * Provides utility methods and constants for the HTTP/2 to HTTP conversion
  */
@@ -330,9 +330,33 @@ public static void toHttp2Headers(HttpHeaders inHeaders, Http2Headers out) throw
             final AsciiString aName = AsciiString.of(entry.getKey()).toLowerCase();
             if (!HTTP_TO_HTTP2_HEADER_BLACKLIST.contains(aName)) {
                 // https://tools.ietf.org/html/rfc7540#section-8.1.2.2 makes a special exception for TE
-                if (!aName.contentEqualsIgnoreCase(HttpHeaderNames.TE) ||
-                    AsciiString.contentEqualsIgnoreCase(entry.getValue(), HttpHeaderValues.TRAILERS)) {
-                    out.add(aName, AsciiString.of(entry.getValue()));
+                if (aName.contentEqualsIgnoreCase(HttpHeaderNames.TE) &&
+                    !AsciiString.contentEqualsIgnoreCase(entry.getValue(), HttpHeaderValues.TRAILERS)) {
+                    throw new IllegalArgumentException("Invalid value for " + HttpHeaderNames.TE + ": " +
+                                                        entry.getValue());
+                }
+                if (aName.contentEqualsIgnoreCase(HttpHeaderNames.COOKIE)) {
+                    AsciiString value = AsciiString.of(entry.getValue());
+                    // split up cookies to allow for better compression
+                    // https://tools.ietf.org/html/rfc7540#section-8.1.2.5
+                    int index = value.forEachByte(FIND_SEMI_COLON);
+                    if (index != -1) {
+                        int start = 0;
+                        do {
+                            out.add(HttpHeaderNames.COOKIE, value.subSequence(start, index, false));
+                            // skip 2 characters "; " (see https://tools.ietf.org/html/rfc6265#section-4.2.1)
+                            start = index + 2;
+                        } while (start < value.length() &&
+                                (index = value.forEachByte(start, value.length() - start, FIND_SEMI_COLON)) != -1);
+                        if (start >= value.length()) {
+                            throw new IllegalArgumentException("cookie value is of unexpected format: " + value);
+                        }
+                        out.add(HttpHeaderNames.COOKIE, value.subSequence(start, value.length(), false));
+                    } else {
+                        out.add(HttpHeaderNames.COOKIE, value);
+                    }
+                } else {
+                    out.add(aName, entry.getValue());
                 }
             }
         }
@@ -449,7 +473,15 @@ public void translate(Entry<CharSequence, CharSequence> entry) throws Http2Excep
                     throw streamError(streamId, PROTOCOL_ERROR,
                             "Invalid HTTP/2 header '%s' encountered in translation to HTTP/1.x", name);
                 }
-                output.add(AsciiString.of(name), AsciiString.of(value));
+                if (HttpHeaderNames.COOKIE.equals(name)) {
+                    // combine the cookie values into 1 header entry.
+                    // https://tools.ietf.org/html/rfc7540#section-8.1.2.5
+                    String existingCookie = output.get(HttpHeaderNames.COOKIE);
+                    output.set(HttpHeaderNames.COOKIE,
+                               (existingCookie != null) ? (existingCookie + "; " + value) : value);
+                } else {
+                    output.add(name, value);
+                }
             }
         }
     }
diff --git a/common/src/main/java/io/netty/util/AsciiString.java b/common/src/main/java/io/netty/util/AsciiString.java
index 38ce3243e8e..ceb5bf0ca59 100644
--- a/common/src/main/java/io/netty/util/AsciiString.java
+++ b/common/src/main/java/io/netty/util/AsciiString.java
@@ -276,7 +276,7 @@ public int forEachByte(int index, int length, ByteProcessor visitor) throws Exce
     }
 
     private int forEachByte0(int index, int length, ByteProcessor visitor) throws Exception {
-        final int len = offset + length;
+        final int len = offset + index + length;
         for (int i = offset + index; i < len; ++i) {
             if (!visitor.process(value[i])) {
                 return i - offset;
diff --git a/common/src/main/java/io/netty/util/ByteProcessor.java b/common/src/main/java/io/netty/util/ByteProcessor.java
index 2847813c6f0..ef5929e8b64 100644
--- a/common/src/main/java/io/netty/util/ByteProcessor.java
+++ b/common/src/main/java/io/netty/util/ByteProcessor.java
@@ -120,6 +120,16 @@ public boolean process(byte value) throws Exception {
         }
     };
 
+    /**
+     * Aborts on a {@code CR (';')}.
+     */
+    ByteProcessor FIND_SEMI_COLON = new ByteProcessor() {
+        @Override
+        public boolean process(byte value) throws Exception {
+            return value != ';';
+        }
+    };
+
     /**
      * @return {@code true} if the processor wants to continue the loop and handle the next byte in the buffer.
      *         {@code false} if the processor wants to stop handling bytes and abort the loop.


Test patch:
diff --git a/codec-http2/src/test/java/io/netty/handler/codec/http2/HttpToHttp2ConnectionHandlerTest.java b/codec-http2/src/test/java/io/netty/handler/codec/http2/HttpToHttp2ConnectionHandlerTest.java
index 55d9404fc72..f8727b0dc2b 100644
--- a/codec-http2/src/test/java/io/netty/handler/codec/http2/HttpToHttp2ConnectionHandlerTest.java
+++ b/codec-http2/src/test/java/io/netty/handler/codec/http2/HttpToHttp2ConnectionHandlerTest.java
@@ -141,6 +141,27 @@ public void testHeadersOnlyRequest() throws Exception {
         verifyHeadersOnly(http2Headers, writePromise, clientChannel.writeAndFlush(request, writePromise));
     }
 
+    @Test
+    public void testMultipleCookieEntriesAreCombined() throws Exception {
+        bootstrapEnv(2, 1, 0);
+        final FullHttpRequest request = new DefaultFullHttpRequest(HTTP_1_1, GET,
+                "http://my-user_name@www.example.org:5555/example");
+        final HttpHeaders httpHeaders = request.headers();
+        httpHeaders.setInt(HttpConversionUtil.ExtensionHeaderNames.STREAM_ID.text(), 5);
+        httpHeaders.set(HttpHeaderNames.HOST, "my-user_name@www.example.org:5555");
+        httpHeaders.set(HttpConversionUtil.ExtensionHeaderNames.SCHEME.text(), "http");
+        httpHeaders.set(HttpHeaderNames.COOKIE, "a=b; c=d; e=f");
+        final Http2Headers http2Headers =
+                new DefaultHttp2Headers().method(new AsciiString("GET")).path(new AsciiString("/example"))
+                .authority(new AsciiString("www.example.org:5555")).scheme(new AsciiString("http"))
+                .add(HttpHeaderNames.COOKIE, "a=b")
+                .add(HttpHeaderNames.COOKIE, "c=d")
+                .add(HttpHeaderNames.COOKIE, "e=f");
+
+        ChannelPromise writePromise = newPromise();
+        verifyHeadersOnly(http2Headers, writePromise, clientChannel.writeAndFlush(request, writePromise));
+    }
+
     @Test
     public void testOriginFormRequestTargetHandled() throws Exception {
         bootstrapEnv(2, 1, 0);
diff --git a/codec-http2/src/test/java/io/netty/handler/codec/http2/InboundHttp2ToHttpAdapterTest.java b/codec-http2/src/test/java/io/netty/handler/codec/http2/InboundHttp2ToHttpAdapterTest.java
index c057fdda8e5..9d355a5b3d0 100644
--- a/codec-http2/src/test/java/io/netty/handler/codec/http2/InboundHttp2ToHttpAdapterTest.java
+++ b/codec-http2/src/test/java/io/netty/handler/codec/http2/InboundHttp2ToHttpAdapterTest.java
@@ -158,6 +158,75 @@ public void run() {
         }
     }
 
+    @Test
+    public void clientRequestSingleHeaderCookieSplitIntoMultipleEntries() throws Exception {
+        boostrapEnv(1, 1, 1);
+        final FullHttpRequest request = new DefaultFullHttpRequest(HttpVersion.HTTP_1_1, HttpMethod.GET,
+                "/some/path/resource2", true);
+        try {
+            HttpHeaders httpHeaders = request.headers();
+            httpHeaders.set(HttpConversionUtil.ExtensionHeaderNames.SCHEME.text(), "https");
+            httpHeaders.set(HttpHeaderNames.HOST, "example.org");
+            httpHeaders.setInt(HttpConversionUtil.ExtensionHeaderNames.STREAM_ID.text(), 3);
+            httpHeaders.setInt(HttpHeaderNames.CONTENT_LENGTH, 0);
+            httpHeaders.set(HttpHeaderNames.COOKIE, "a=b; c=d; e=f");
+            final Http2Headers http2Headers = new DefaultHttp2Headers().method(new AsciiString("GET")).
+                    scheme(new AsciiString("https")).authority(new AsciiString("example.org"))
+                    .path(new AsciiString("/some/path/resource2"))
+                    .add(HttpHeaderNames.COOKIE, "a=b")
+                    .add(HttpHeaderNames.COOKIE, "c=d")
+                    .add(HttpHeaderNames.COOKIE, "e=f");
+            runInChannel(clientChannel, new Http2Runnable() {
+                @Override
+                public void run() {
+                    frameWriter.writeHeaders(ctxClient(), 3, http2Headers, 0, true, newPromiseClient());
+                    ctxClient().flush();
+                }
+            });
+            awaitRequests();
+            ArgumentCaptor<FullHttpMessage> requestCaptor = ArgumentCaptor.forClass(FullHttpMessage.class);
+            verify(serverListener).messageReceived(requestCaptor.capture());
+            capturedRequests = requestCaptor.getAllValues();
+            assertEquals(request, capturedRequests.get(0));
+        } finally {
+            request.release();
+        }
+    }
+
+    @Test
+    public void clientRequestSingleHeaderCookieSplitIntoMultipleEntries2() throws Exception {
+        boostrapEnv(1, 1, 1);
+        final FullHttpRequest request = new DefaultFullHttpRequest(HttpVersion.HTTP_1_1, HttpMethod.GET,
+                "/some/path/resource2", true);
+        try {
+            HttpHeaders httpHeaders = request.headers();
+            httpHeaders.set(HttpConversionUtil.ExtensionHeaderNames.SCHEME.text(), "https");
+            httpHeaders.set(HttpHeaderNames.HOST, "example.org");
+            httpHeaders.setInt(HttpConversionUtil.ExtensionHeaderNames.STREAM_ID.text(), 3);
+            httpHeaders.setInt(HttpHeaderNames.CONTENT_LENGTH, 0);
+            httpHeaders.set(HttpHeaderNames.COOKIE, "a=b; c=d; e=f");
+            final Http2Headers http2Headers = new DefaultHttp2Headers().method(new AsciiString("GET")).
+                    scheme(new AsciiString("https")).authority(new AsciiString("example.org"))
+                    .path(new AsciiString("/some/path/resource2"))
+                    .add(HttpHeaderNames.COOKIE, "a=b; c=d")
+                    .add(HttpHeaderNames.COOKIE, "e=f");
+            runInChannel(clientChannel, new Http2Runnable() {
+                @Override
+                public void run() {
+                    frameWriter.writeHeaders(ctxClient(), 3, http2Headers, 0, true, newPromiseClient());
+                    ctxClient().flush();
+                }
+            });
+            awaitRequests();
+            ArgumentCaptor<FullHttpMessage> requestCaptor = ArgumentCaptor.forClass(FullHttpMessage.class);
+            verify(serverListener).messageReceived(requestCaptor.capture());
+            capturedRequests = requestCaptor.getAllValues();
+            assertEquals(request, capturedRequests.get(0));
+        } finally {
+            request.release();
+        }
+    }
+
     @Test
     public void clientRequestSingleHeaderNonAsciiShouldThrow() throws Exception {
         boostrapEnv(1, 1, 1);
diff --git a/common/src/test/java/io/netty/util/AsciiStringMemoryTest.java b/common/src/test/java/io/netty/util/AsciiStringMemoryTest.java
index 808ace76a7e..0f2468aed6a 100644
--- a/common/src/test/java/io/netty/util/AsciiStringMemoryTest.java
+++ b/common/src/test/java/io/netty/util/AsciiStringMemoryTest.java
@@ -17,6 +17,7 @@
 
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertNotEquals;
+import io.netty.util.ByteProcessor.IndexOfProcessor;
 
 import java.util.Random;
 import java.util.concurrent.atomic.AtomicReference;
@@ -102,6 +103,18 @@ public boolean process(byte value) throws Exception {
         assertEquals(bAsciiString.length(), bCount.get().intValue());
     }
 
+    @Test
+    public void forEachWithIndexEndTest() throws Exception {
+        assertNotEquals(-1, aAsciiString.forEachByte(aAsciiString.length() - 1,
+                1, new IndexOfProcessor(aAsciiString.byteAt(aAsciiString.length() - 1))));
+    }
+
+    @Test
+    public void forEachWithIndexBeginTest() throws Exception {
+        assertNotEquals(-1, aAsciiString.forEachByte(0,
+                1, new IndexOfProcessor(aAsciiString.byteAt(0))));
+    }
+
     @Test
     public void forEachDescTest() throws Exception {
         final AtomicReference<Integer> aCount = new AtomicReference<Integer>(0);
@@ -128,6 +141,18 @@ public boolean process(byte value) throws Exception {
         assertEquals(bAsciiString.length(), bCount.get().intValue());
     }
 
+    @Test
+    public void forEachDescWithIndexEndTest() throws Exception {
+        assertNotEquals(-1, bAsciiString.forEachByteDesc(bAsciiString.length() - 1,
+                1, new IndexOfProcessor(bAsciiString.byteAt(bAsciiString.length() - 1))));
+    }
+
+    @Test
+    public void forEachDescWithIndexBeginTest() throws Exception {
+        assertNotEquals(-1, bAsciiString.forEachByteDesc(0,
+                1, new IndexOfProcessor(bAsciiString.byteAt(0))));
+    }
+
     @Test
     public void subSequenceTest() {
         final int start = 12;