Repository: netty/netty
Issue: http/2 message => http/1.1 message adapter for the new http/2 api

## problem

http/2 and http/1.1 have similar protocols, and it's useful to be able to implement a single server against a single interface.  There's an injection from http/1.1 messages to http/2 ones, so it makes sense to make folks program against http/1.1 and upgrade them under the hood.
## proposed solution

a `MessageToMessageDecoder<Http2StreamFrame>` which turns every kind of `Http2StreamFrame` domain object into an `HttpContent` domain object.
## what's next

I'm working on building this for myself, and I would love to contribute it back to netty.  Please let me know what you think!  Right now I'm able to avoid all aggregation, and the adapters that I've written so far are:

```
HeadersFrame is endStream => LastHttpContent with Trailers
                         is not endStream => HttpConversionUtil.toHttpRequest or toHttpResponse
Http2DataFrame is endStream => LastHttpContent
                            is not endStream => HttpContent
```

Right now I'm not handling goaway, reset, or stream id, and I figure we can add those as we go along.

The opposite is mostly symmetric, except that there are some http/1.1 messages which will decompose to more than one http/2 message, but that's easy to encode.


Issue comments:
I've added this to the checklist in #3667. What you've proposed sounds good.

GOAWAY and RST_STREAM can simply be ignored since the channel will closed automatically (they are propagated for additional information to those that want it; HTTP/1 doesn't want it). Stream ID should always be null on a child channel (at least for now), so there should be nothing to handle there either.

Not sure if this is covered somewhere or is understood but just to clarify on the following....

> HeadersFrame is endStream => LastHttpContent with Trailers

A HEADERS frame with EOS doesn't necessarily mean the headers are trailers. This frame could just be the first HEADERS frame and there are no DATA frames to follow.

OK, that's good to know.  So that would be converted to a FullHttpRequest with an empty content body?


Pull request review comments:
+1

fix formating 

fix formating

I think we should retain as late as possible to mimize the risk of leak data in case of exceptions.

fix formating. 

see above

Add `.` on EOL.

also assert the return value of all write calls (also in other tests).

also call channel.finish() and assert the return value

`FullHttpResponse` can also have `trailingHeaders()`

looks like this is covered in `encodeLastContent` ... just started looking but do you see an issue with this?

LastHttpContent is not mutually exclusive with HttpResponse (as is seen with FullHttpResponse). Instead of if-elses, I'd suggest having separate ifs for each part of the message, and don't treat FullHttpResponse specially.

```
if (obj instanceof HttpMessage) {
  out.add(headers);
}
if (obj instanceof HttpContent) {
  if (hasContent) {
    out.add(content /* no eos */);
  }
}
if (obj instanceof LastHttpContent) {
  if (lastContent.trailingHeaders().isEmpty()) {
    out.add(emptyDataFrameWithEOS);
  } else {
    out.add(trailers);
  }
}
```

It would be possible to optimize it to avoid `emptyDataFrameWithEOS`, but I question how much that matters.

I'm not sure if we should attempt to insert "filler". Seems more expected to handle the objects as they are...

``` java
LastHttpContent last = ...;
if (last.trailingHeaders().isEmpty()) {
  out.add(new dataframe(last.content(), true));
} else {
  out.add(new dataframe(last.content(), false));
  out.add(new headersframe(convert(last.trailingHeaders(), ...), true));
}
```

Ah, you're right. It's fine.

consider renaming `toHttpRequest` -> `toFullHttpRequest` and renaming this `toHttpChunkedRequest` -> `toHttpRequest`. The "chunked" terminology may be confused with "chunked" encoding as defined in https://tools.ietf.org/html/rfc7230#section-4.1

the suggestion in https://github.com/netty/netty/pull/5204#discussion_r62067945 by @ejona86 would also help mitigate this issue.

this will create problems if an exception occurs (e.g. during headers conversion). sending a stream error with stream id 0 is not allowed by the spec https://tools.ietf.org/html/rfc7540#section-6.4. We should also get the "real" stream ID to ease understanding.

@ejona86 - what is the best way to do this?

`0` -> `id` (assuming id eventually represents the "real" stream id)

@Scottmitch, I've got to go to a meeting, so I can't dig too deeply. But there shouldn't be much reason to need the id with the childchan API. If we really have to, we could grab it from an attribute (which currently doesn't exist) on the channel.

Is the intention of this conditional to distinguish between the "headers" and the "trailers"? Perhaps a better way to do this is to check the stream state. @ejona86 - is there a way to get the stream state in the new API?

Also as a side note @nmittler I don't think we are currently enforcing this (are we?):

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

> All HTTP/2 requests MUST include exactly one valid value for the
>    ":method", ":scheme", and ":path" pseudo-header fields, unless it is
>    a CONNECT request (Section 8.3).  An HTTP request that omits
>    mandatory pseudo-header fields is malformed (Section 8.1.2.6).

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

> Malformed requests or responses that are
>    detected MUST be treated as a stream error (Section 5.4.2) of type
>    PROTOCOL_ERROR.
> 
> For malformed requests, a server MAY send an HTTP response prior to
>    closing or resetting the stream.  Clients MUST NOT accept a malformed
>    response.  Note that these requirements are intended to protect
>    against several types of common attacks against HTTP; they are
>    deliberately strict because being permissive can expose
>    implementations to these vulnerabilities.

So maybe its good we don't enforce this on the server ... either it seems best to avoid using the presence/absence of these headers fields to indicate state of the stream.

typically the `validateHeaders` is exposed and configurable by the user. validating headers can add non-trivial overhead that some users decide to forgo.

also expose the `validateHeaders` so it can be configured by users (same as https://github.com/netty/netty/pull/5204#discussion_r62077393)

expose the `validateHeaders` so it can be configured by users (same as https://github.com/netty/netty/pull/5204#discussion_r62077393)

@ejona86 - is the idea that the exception should propagate through the child channel's pipeline and translated to the appropriate type of HTTP/2 frame by the http/2 codec under the hood? In the long term this seems like a good thing (assuming we can distinguish between connection/stream errors if necessary)... in the near term our Http2Exception requires the StreamId ... and if the connection stream id is used a stream error will be translated into a connection error (see [Http2Exception](https://github.com/netty/netty/blob/4.1/codec-http2/src/main/java/io/netty/handler/codec/http2/Http2Exception.java#L124)) ... we should have some story to handle this in the near term ... wdyt?

> Perhaps a better way to do this is to check the stream state. @ejona86 - is there a way to get the stream state in the new API?

No, there isn't, and currently we don't track this sort of state anywhere (even in the old API). It could be something we add, but I'd hope to not need it, especially given how hard it will be to resolve races. If the handler wasn't shareable it'd be trivial to deal with, but shareable is a laudable goal.

I don't have too much concern about relying on :method for requests and :status for responses, since they are so core to HTTP and _must_ be present in HTTP/1 during parsing. That would even behave correctly in the face of 1xx status codes where you end up having multiple sets of "headers". (For those unaware of this particular dark corner of HTTP, multiple header sets is a real thing in both [HTTP/1.1](https://tools.ietf.org/html/rfc7231#section-6.2) and [HTTP/2](https://tools.ietf.org/html/rfc7540#section-8.1). Yay!)

Another option is to add a bit to Http2HeadersFrame indicating whether it is a "header" or "trailer," but that sounds a bit icky because it has to agree with endStream.

So I looked into this some. The value has two usages: stream exceptions and converting to non-childchan HTTP/2. Converting to non-childchan HTTP/2 doesn't seem important.

Exceptions on a stream's channel have to be handled like normal exceptions; in this case with exceptionCaught. If uncaught it just logs. Applications will commonly write exceptionCaught to close the channel, which then becomes [Http2ResetFrame(CANCEL)](https://github.com/netty/netty/blob/4.1/codec-http2/src/main/java/io/netty/handler/codec/http2/Http2MultiplexCodec.java#L413)). An application could write its own Http2ResetFrame with special error code if it wanted.

So the Http2Exceptions aren't special with h2childchan. That's true even if you're on the parent channel. And in fact it's true in the non-childchan API as well when you are later in the pipeline than the Http2ConnectionHandler; it's just that that almost never happens.

> No, there isn't, and currently we don't track this sort of state anywhere (even in the old API)

In the existing API we generally pass around the stream ID or the Http2Stream object. If you have the stream ID then you can use `Http2Connection.stream(id)` to get the Http2Stream. The Http2Stream allows access to the [State](https://github.com/netty/netty/blob/4.1/codec-http2/src/main/java/io/netty/handler/codec/http2/Http2Stream.java#L68).

> I don't have too much concern about relying on :method for requests and :status for responses, since they are so core to HTTP and must be present in HTTP/1 during parsing

Consider the following sequence of messages:

Headers (:method=get) StreamId=3 EOS=0
Data StreamId=3 EOS=0
Headers (:method=get) StreamId=3 EOS=1

This is likely a separate issue but where do we enforce that these "special" headers can only exist "exactly once" before the "data" (unless its a connect request)?

Yes this seems reasonable ... only concern is if a childchan uses an API which intends to throw a "Connection Error" (which would typically be converted into a GO_AWAY) ... although it seems like most of these scenarios should be on the parent channel decoding stuff before it gets to the childchan

nit: `this(true)`

done, thanks!

> The Http2Stream allows access to the State.

But that state is 1) not directly usable because it has already been modified in response to the frames, 2) this logic using the state may be delayed, even on the same thread and 3) this logic using the state may be run from a different thread from where the state is maintained/changed.

For point #1, consider these new streams:

IDLE: (isn't actually tracked yet; it will [never](https://github.com/netty/netty/blob/bd6040a/codec-http2/src/main/java/io/netty/handler/codec/http2/DefaultHttp2Connection.java#L311) show up as IDLE)
OPEN: Headers (:method=get) StreamId=3 EOS=0 # These are headers
OPEN: Headers () StreamId=3 EOS=1 # These are trailers
HALF_CLOSED_REMOTE: (yes, it happens [afterward](https://github.com/netty/netty/blob/83c349f/codec-http2/src/main/java/io/netty/handler/codec/http2/DefaultHttp2ConnectionDecoder.java#L324))

IDLE: (isn't actually tracked yet; it won't show up as IDLE)
HALF_CLOSED_REMOTE: Headers (:method=get) StreamId=5 EOS=1 # These are headers
(yes, this state happens [immediately](https://github.com/netty/netty/blob/bd6040a/codec-http2/src/main/java/io/netty/handler/codec/http2/DefaultHttp2Connection.java#L315))

So the State is _never_ useful for this distinction today. That could possibly be fixed, but the child channel has its own queue. The application could simply not read from the child channel immediately and the state will be HALF_CLOSED_REMOTE in both cases. When the child channel is using a different event loop, we'd expect HALF_CLOSED_REMOTE in both cases.

> This is likely a separate issue but where do we enforce that these "special" headers can only exist "exactly once" before the "data" (unless its a connect request)?

It looks like we should be prohibiting that (still taking into account 1xx):

> Pseudo-header fields are only valid in the context in which they are defined. Pseudo-header fields defined for requests MUST NOT appear in responses; pseudo-header fields defined for responses MUST NOT appear in requests. Pseudo-header fields MUST NOT appear in trailers. Endpoints MUST treat a request or response that contains undefined or invalid pseudo-header fields as malformed (Section 8.1.2.6).

I would hope the Http2Exceptions would go away (or be an implementation detail) in the child chan API (for both child channel and parent channel). They stop being helpful for control flow when you have multiple handlers, since the exceptions are propagated toward the tail of the pipeline, not the head. They could still be used for notifications of failures, but it seems they couldn't trigger RST_STREAM and GOAWAY like they do today.

Agreed the state as it exists today won't be very useful for this purpose

`full.content().readableBytes() == 0` -> `!full.content().isReadable()`

why not make non-static and just use `validateHeaders` as a member variable instead of as a parameter?

its not clear why we should try to simulate the user sending a data frame to make the encoding work. Seems more intuitive to just encode what the user provides and if their logic is flawed they should fix it. Can we just mimic the logic in [HttpToHttp2ConnectionHandler.write](https://github.com/netty/netty/blob/4.1/codec-http2/src/main/java/io/netty/handler/codec/http2/HttpToHttp2ConnectionHandler.java) or improve the logic their if it is not good enough?  It would be nice to share code or at least use consistent logic if possible.

We need to handle the case where a user sends a `LastHttpContent.EMPTY_LAST_CONTENT` in order to close their streamed message, but there's no need to send a message otherwise.  I don't think this is incorrect behavior.

It looks like in HttpToHttp2ConnectionHandler, it sends a data frame regardless of whether there's content or not, which seems unnecessary.  I can clean it up, do you mind if we push to a subsequent review though?

>  I don't think this is incorrect behavior.

Agreed. Thanks for clarifying.

>  I can clean it up, do you mind if we push to a subsequent review though?

Followup PR sgtm thanks!

@mosesn - I just merged this an noticed a leak in the build log. can you add another commit (or pr) where these `request` objects are released in a finally block for all these tests?

yep, how can I check if I'm leak-free?

The best thing we have ATM is [-Dio.netty.leakDetectionLevel=paranoid](http://netty.io/wiki/reference-counted-objects.html#leak-detection-levels).

thanks :)

@mosesn run a few times with `-Pleak` and see if you see any `LEAK:` logs in the console

note that you can also use [releaseLater](http://netty.io/wiki/reference-counted-objects.html#fixing-leaks-in-unit-tests) instead of using a `try`/`finally` block.

I've run it a few times, but can't repro locally.  Here's what I'm doing:

``` bash
mvn test -pl codec-http2 -Dtest=Http2ServerDowngraderTest -Pleak
```

Is there anything I'm missing?  Is there a way I can see the failed build, so I can fix based on that?

@mosesn - the build won't fail locally, but you should see log statements that look like this https://garage.netty.io/teamcity/viewLog.html?tab=buildLog&logTab=tree&filter=debug&expand=all&buildId=8253#_focus=18593

Weird, I don't see that failure locally.  Do you see it consistently?  Do I need to change a log level somewhere?

Not sure why, but it just started working locally.  Working on a fix now–looks like it's just for one test.

I made a PR, but I'm not sure if this is the right fix: #5230.


Patch that resolved the issue:
diff --git a/codec-http2/src/main/java/io/netty/handler/codec/http2/Http2ServerDowngrader.java b/codec-http2/src/main/java/io/netty/handler/codec/http2/Http2ServerDowngrader.java
new file mode 100644
index 00000000000..189e2cd52c2
--- /dev/null
+++ b/codec-http2/src/main/java/io/netty/handler/codec/http2/Http2ServerDowngrader.java
@@ -0,0 +1,126 @@
+/*
+ * Copyright 2016 The Netty Project
+ *
+ * The Netty Project licenses this file to you under the Apache License,
+ * version 2.0 (the "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at:
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+ * License for the specific language governing permissions and limitations
+ * under the License.
+ */
+
+package io.netty.handler.codec.http2;
+
+import io.netty.buffer.Unpooled;
+import io.netty.channel.ChannelHandlerContext;
+import io.netty.handler.codec.MessageToMessageCodec;
+import io.netty.handler.codec.http.DefaultHttpContent;
+import io.netty.handler.codec.http.DefaultLastHttpContent;
+import io.netty.handler.codec.http.FullHttpRequest;
+import io.netty.handler.codec.http.FullHttpResponse;
+import io.netty.handler.codec.http.HttpContent;
+import io.netty.handler.codec.http.HttpObject;
+import io.netty.handler.codec.http.HttpResponse;
+import io.netty.handler.codec.http.HttpVersion;
+import io.netty.handler.codec.http.LastHttpContent;
+import io.netty.util.ReferenceCountUtil;
+
+import java.util.List;
+
+/**
+ * This is a server-side adapter so that an http2 codec can be downgraded to
+ * appear as if it's speaking http/1.1.
+ *
+ * In particular, this handler converts from {@link Http2StreamFrame} to {@link
+ * HttpObject}, and back.  For simplicity, it converts to chunked encoding
+ * unless the entire stream is a single header.
+ */
+public class Http2ServerDowngrader extends MessageToMessageCodec<Http2StreamFrame, HttpObject> {
+
+    private final boolean validateHeaders;
+
+    public Http2ServerDowngrader(boolean validateHeaders) {
+        this.validateHeaders = validateHeaders;
+    }
+
+    public Http2ServerDowngrader() {
+        this(true);
+    }
+
+    @Override
+    public boolean acceptInboundMessage(Object msg) throws Exception {
+        return (msg instanceof Http2HeadersFrame) || (msg instanceof Http2DataFrame);
+    }
+
+    @Override
+    protected void decode(ChannelHandlerContext ctx, Http2StreamFrame frame, List<Object> out) throws Exception {
+        if (frame instanceof Http2HeadersFrame) {
+            int id = 0; // not really the id
+            Http2HeadersFrame headersFrame = (Http2HeadersFrame) frame;
+            Http2Headers headers = headersFrame.headers();
+
+            if (headersFrame.isEndStream()) {
+                if (headers.method() == null) {
+                    LastHttpContent last = new DefaultLastHttpContent(Unpooled.EMPTY_BUFFER, validateHeaders);
+                    HttpConversionUtil.addHttp2ToHttpHeaders(id, headers, last.trailingHeaders(),
+                                                             HttpVersion.HTTP_1_1, true, true);
+                    out.add(last);
+                } else {
+                    FullHttpRequest full = HttpConversionUtil.toFullHttpRequest(id, headers, ctx.alloc(),
+                            validateHeaders);
+                    out.add(full);
+                }
+            } else {
+                out.add(HttpConversionUtil.toHttpRequest(id, headersFrame.headers(), validateHeaders));
+            }
+
+        } else if (frame instanceof Http2DataFrame) {
+            Http2DataFrame dataFrame = (Http2DataFrame) frame;
+            if (dataFrame.isEndStream()) {
+                out.add(new DefaultLastHttpContent(dataFrame.content(), validateHeaders));
+            } else {
+                out.add(new DefaultHttpContent(dataFrame.content()));
+            }
+        }
+        ReferenceCountUtil.retain(frame);
+    }
+
+    private void encodeLastContent(LastHttpContent last, List<Object> out) {
+        boolean needFiller = !(last instanceof FullHttpResponse) && last.trailingHeaders().isEmpty();
+        if (last.content().isReadable() || needFiller) {
+            out.add(new DefaultHttp2DataFrame(last.content(), last.trailingHeaders().isEmpty()));
+        }
+        if (!last.trailingHeaders().isEmpty()) {
+            Http2Headers headers = HttpConversionUtil.toHttp2Headers(last.trailingHeaders(), validateHeaders);
+            out.add(new DefaultHttp2HeadersFrame(headers, true));
+        }
+    }
+
+    @Override
+    protected void encode(ChannelHandlerContext ctx, HttpObject obj, List<Object> out) throws Exception {
+        if (obj instanceof HttpResponse) {
+            Http2Headers headers = HttpConversionUtil.toHttp2Headers((HttpResponse) obj, validateHeaders);
+            boolean noMoreFrames = false;
+            if (obj instanceof FullHttpResponse) {
+                FullHttpResponse full = (FullHttpResponse) obj;
+                noMoreFrames = !full.content().isReadable() && full.trailingHeaders().isEmpty();
+            }
+
+            out.add(new DefaultHttp2HeadersFrame(headers, noMoreFrames));
+        }
+
+        if (obj instanceof LastHttpContent) {
+            LastHttpContent last = (LastHttpContent) obj;
+            encodeLastContent(last, out);
+        } else if (obj instanceof HttpContent) {
+            HttpContent cont = (HttpContent) obj;
+            out.add(new DefaultHttp2DataFrame(cont.content(), false));
+        }
+        ReferenceCountUtil.retain(obj);
+    }
+}
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 80bac5eec75..bc81b6f1729 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
@@ -17,6 +17,7 @@
 import io.netty.buffer.ByteBufAllocator;
 import io.netty.handler.codec.http.DefaultFullHttpRequest;
 import io.netty.handler.codec.http.DefaultFullHttpResponse;
+import io.netty.handler.codec.http.DefaultHttpRequest;
 import io.netty.handler.codec.http.FullHttpMessage;
 import io.netty.handler.codec.http.FullHttpRequest;
 import io.netty.handler.codec.http.FullHttpResponse;
@@ -228,7 +229,7 @@ public static FullHttpResponse toHttpResponse(int streamId, Http2Headers http2He
      * @return A new request object which represents headers/data
      * @throws Http2Exception see {@link #addHttp2ToHttpHeaders(int, Http2Headers, FullHttpMessage, boolean)}
      */
-    public static FullHttpRequest toHttpRequest(int streamId, Http2Headers http2Headers, ByteBufAllocator alloc,
+    public static FullHttpRequest toFullHttpRequest(int streamId, Http2Headers http2Headers, ByteBufAllocator alloc,
                                                 boolean validateHttpHeaders)
                     throws Http2Exception {
         // HTTP/2 does not define a way to carry the version identifier that is included in the HTTP/1.1 request line.
@@ -250,6 +251,37 @@ public static FullHttpRequest toHttpRequest(int streamId, Http2Headers http2Head
         return msg;
     }
 
+    /**
+     * Create a new object to contain the request data.
+     *
+     * @param streamId The stream associated with the request
+     * @param http2Headers The initial set of HTTP/2 headers to create the request with
+     * @param validateHttpHeaders <ul>
+     *        <li>{@code true} to validate HTTP headers in the http-codec</li>
+     *        <li>{@code false} not to validate HTTP headers in the http-codec</li>
+     *        </ul>
+     * @return A new request object which represents headers for a chunked request
+     * @throws Http2Exception see {@link #addHttp2ToHttpHeaders(int, Http2Headers, FullHttpMessage, boolean)}
+     */
+    public static HttpRequest toHttpRequest(int streamId, Http2Headers http2Headers, boolean validateHttpHeaders)
+                    throws Http2Exception {
+        // HTTP/2 does not define a way to carry the version identifier that is included in the HTTP/1.1 request line.
+        final CharSequence method = checkNotNull(http2Headers.method(),
+                "method header cannot be null in conversion to HTTP/1.x");
+        final CharSequence path = checkNotNull(http2Headers.path(),
+                "path header cannot be null in conversion to HTTP/1.x");
+        HttpRequest msg = new DefaultHttpRequest(HttpVersion.HTTP_1_1, HttpMethod.valueOf(method.toString()),
+                path.toString(), validateHttpHeaders);
+        try {
+            addHttp2ToHttpHeaders(streamId, http2Headers, msg.headers(), msg.protocolVersion(), false, true);
+        } catch (Http2Exception e) {
+            throw e;
+        } catch (Throwable t) {
+            throw streamError(streamId, PROTOCOL_ERROR, t, "HTTP/2 to HTTP/1.x headers conversion error");
+        }
+        return msg;
+    }
+
     /**
      * Translate and add HTTP/2 headers to HTTP/1.x headers.
      *
diff --git a/codec-http2/src/main/java/io/netty/handler/codec/http2/InboundHttp2ToHttpAdapter.java b/codec-http2/src/main/java/io/netty/handler/codec/http2/InboundHttp2ToHttpAdapter.java
index 68c912aa7c1..37d2f1cd3c5 100644
--- a/codec-http2/src/main/java/io/netty/handler/codec/http2/InboundHttp2ToHttpAdapter.java
+++ b/codec-http2/src/main/java/io/netty/handler/codec/http2/InboundHttp2ToHttpAdapter.java
@@ -150,7 +150,7 @@ protected void fireChannelRead(ChannelHandlerContext ctx, FullHttpMessage msg, b
     protected FullHttpMessage newMessage(Http2Stream stream, Http2Headers headers, boolean validateHttpHeaders,
                                          ByteBufAllocator alloc)
             throws Http2Exception {
-        return connection.isServer() ? HttpConversionUtil.toHttpRequest(stream.id(), headers, alloc,
+        return connection.isServer() ? HttpConversionUtil.toFullHttpRequest(stream.id(), headers, alloc,
                 validateHttpHeaders) : HttpConversionUtil.toHttpResponse(stream.id(), headers, alloc,
                                                                          validateHttpHeaders);
     }


Test patch:
diff --git a/codec-http2/src/test/java/io/netty/handler/codec/http2/Http2ServerDowngraderTest.java b/codec-http2/src/test/java/io/netty/handler/codec/http2/Http2ServerDowngraderTest.java
new file mode 100644
index 00000000000..e7ca6702e73
--- /dev/null
+++ b/codec-http2/src/test/java/io/netty/handler/codec/http2/Http2ServerDowngraderTest.java
@@ -0,0 +1,317 @@
+/*
+ * Copyright 2016 The Netty Project
+ *
+ * The Netty Project licenses this file to you under the Apache License,
+ * version 2.0 (the "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at:
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+ * License for the specific language governing permissions and limitations
+ * under the License.
+ */
+
+package io.netty.handler.codec.http2;
+
+import io.netty.buffer.ByteBuf;
+import io.netty.buffer.Unpooled;
+import io.netty.channel.embedded.EmbeddedChannel;
+import io.netty.handler.codec.http.DefaultFullHttpResponse;
+import io.netty.handler.codec.http.DefaultHttpContent;
+import io.netty.handler.codec.http.DefaultHttpResponse;
+import io.netty.handler.codec.http.DefaultLastHttpContent;
+import io.netty.handler.codec.http.FullHttpRequest;
+import io.netty.handler.codec.http.FullHttpResponse;
+import io.netty.handler.codec.http.HttpContent;
+import io.netty.handler.codec.http.HttpHeaders;
+import io.netty.handler.codec.http.HttpMethod;
+import io.netty.handler.codec.http.HttpRequest;
+import io.netty.handler.codec.http.HttpResponse;
+import io.netty.handler.codec.http.HttpResponseStatus;
+import io.netty.handler.codec.http.HttpVersion;
+import io.netty.handler.codec.http.LastHttpContent;
+import io.netty.util.CharsetUtil;
+
+import org.junit.Test;
+
+import static org.hamcrest.CoreMatchers.is;
+import static org.hamcrest.CoreMatchers.nullValue;
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertThat;
+import static org.junit.Assert.assertTrue;
+import static org.junit.Assert.assertFalse;
+
+public class Http2ServerDowngraderTest {
+
+    @Test
+    public void testUpgradeEmptyFullResponse() throws Exception {
+        EmbeddedChannel ch = new EmbeddedChannel(new Http2ServerDowngrader());
+        assertTrue(ch.writeOutbound(new DefaultFullHttpResponse(HttpVersion.HTTP_1_1, HttpResponseStatus.OK)));
+
+        Http2HeadersFrame headersFrame = ch.readOutbound();
+        assertThat(headersFrame.headers().status().toString(), is("200"));
+        assertTrue(headersFrame.isEndStream());
+
+        assertThat(ch.readOutbound(), is(nullValue()));
+        assertFalse(ch.finish());
+    }
+
+    @Test
+    public void testUpgradeNonEmptyFullResponse() throws Exception {
+        EmbeddedChannel ch = new EmbeddedChannel(new Http2ServerDowngrader());
+        ByteBuf hello = Unpooled.copiedBuffer("hello world", CharsetUtil.UTF_8);
+        assertTrue(ch.writeOutbound(new DefaultFullHttpResponse(HttpVersion.HTTP_1_1, HttpResponseStatus.OK, hello)));
+
+        Http2HeadersFrame headersFrame = ch.readOutbound();
+        assertThat(headersFrame.headers().status().toString(), is("200"));
+        assertFalse(headersFrame.isEndStream());
+
+        Http2DataFrame dataFrame = ch.readOutbound();
+        assertThat(dataFrame.content().toString(CharsetUtil.UTF_8), is("hello world"));
+        assertTrue(dataFrame.isEndStream());
+
+        assertThat(ch.readOutbound(), is(nullValue()));
+        assertFalse(ch.finish());
+    }
+
+    @Test
+    public void testUpgradeEmptyFullResponseWithTrailers() throws Exception {
+        EmbeddedChannel ch = new EmbeddedChannel(new Http2ServerDowngrader());
+        FullHttpResponse response = new DefaultFullHttpResponse(HttpVersion.HTTP_1_1, HttpResponseStatus.OK);
+        HttpHeaders trailers = response.trailingHeaders();
+        trailers.set("key", "value");
+        assertTrue(ch.writeOutbound(response));
+
+        Http2HeadersFrame headersFrame = ch.readOutbound();
+        assertThat(headersFrame.headers().status().toString(), is("200"));
+        assertFalse(headersFrame.isEndStream());
+
+        Http2HeadersFrame trailersFrame = ch.readOutbound();
+        assertThat(trailersFrame.headers().get("key").toString(), is("value"));
+        assertTrue(trailersFrame.isEndStream());
+
+        assertThat(ch.readOutbound(), is(nullValue()));
+        assertFalse(ch.finish());
+    }
+
+    @Test
+    public void testUpgradeNonEmptyFullResponseWithTrailers() throws Exception {
+        EmbeddedChannel ch = new EmbeddedChannel(new Http2ServerDowngrader());
+        ByteBuf hello = Unpooled.copiedBuffer("hello world", CharsetUtil.UTF_8);
+        FullHttpResponse response = new DefaultFullHttpResponse(HttpVersion.HTTP_1_1, HttpResponseStatus.OK, hello);
+        HttpHeaders trailers = response.trailingHeaders();
+        trailers.set("key", "value");
+        assertTrue(ch.writeOutbound(response));
+
+        Http2HeadersFrame headersFrame = ch.readOutbound();
+        assertThat(headersFrame.headers().status().toString(), is("200"));
+        assertFalse(headersFrame.isEndStream());
+
+        Http2DataFrame dataFrame = ch.readOutbound();
+        assertThat(dataFrame.content().toString(CharsetUtil.UTF_8), is("hello world"));
+        assertFalse(dataFrame.isEndStream());
+
+        Http2HeadersFrame trailersFrame = ch.readOutbound();
+        assertThat(trailersFrame.headers().get("key").toString(), is("value"));
+        assertTrue(trailersFrame.isEndStream());
+
+        assertThat(ch.readOutbound(), is(nullValue()));
+        assertFalse(ch.finish());
+    }
+
+    @Test
+    public void testUpgradeHeaders() throws Exception {
+        EmbeddedChannel ch = new EmbeddedChannel(new Http2ServerDowngrader());
+        HttpResponse response = new DefaultHttpResponse(HttpVersion.HTTP_1_1, HttpResponseStatus.OK);
+        assertTrue(ch.writeOutbound(response));
+
+        Http2HeadersFrame headersFrame = ch.readOutbound();
+        assertThat(headersFrame.headers().status().toString(), is("200"));
+        assertFalse(headersFrame.isEndStream());
+
+        assertThat(ch.readOutbound(), is(nullValue()));
+        assertFalse(ch.finish());
+    }
+
+    @Test
+    public void testUpgradeChunk() throws Exception {
+        EmbeddedChannel ch = new EmbeddedChannel(new Http2ServerDowngrader());
+        ByteBuf hello = Unpooled.copiedBuffer("hello world", CharsetUtil.UTF_8);
+        HttpContent content = new DefaultHttpContent(hello);
+        assertTrue(ch.writeOutbound(content));
+
+        Http2DataFrame dataFrame = ch.readOutbound();
+        assertThat(dataFrame.content().toString(CharsetUtil.UTF_8), is("hello world"));
+        assertFalse(dataFrame.isEndStream());
+
+        assertThat(ch.readOutbound(), is(nullValue()));
+        assertFalse(ch.finish());
+    }
+
+    @Test
+    public void testUpgradeEmptyEnd() throws Exception {
+        EmbeddedChannel ch = new EmbeddedChannel(new Http2ServerDowngrader());
+        LastHttpContent end = LastHttpContent.EMPTY_LAST_CONTENT;
+        assertTrue(ch.writeOutbound(end));
+
+        Http2DataFrame emptyFrame = ch.readOutbound();
+        assertThat(emptyFrame.content().readableBytes(), is(0));
+        assertTrue(emptyFrame.isEndStream());
+
+        assertThat(ch.readOutbound(), is(nullValue()));
+        assertFalse(ch.finish());
+    }
+
+    @Test
+    public void testUpgradeDataEnd() throws Exception {
+        EmbeddedChannel ch = new EmbeddedChannel(new Http2ServerDowngrader());
+        ByteBuf hello = Unpooled.copiedBuffer("hello world", CharsetUtil.UTF_8);
+        LastHttpContent end = new DefaultLastHttpContent(hello, true);
+        assertTrue(ch.writeOutbound(end));
+
+        Http2DataFrame dataFrame = ch.readOutbound();
+        assertThat(dataFrame.content().toString(CharsetUtil.UTF_8), is("hello world"));
+        assertTrue(dataFrame.isEndStream());
+
+        assertThat(ch.readOutbound(), is(nullValue()));
+        assertFalse(ch.finish());
+    }
+
+    @Test
+    public void testUpgradeTrailers() throws Exception {
+        EmbeddedChannel ch = new EmbeddedChannel(new Http2ServerDowngrader());
+        LastHttpContent trailers = new DefaultLastHttpContent(Unpooled.EMPTY_BUFFER, true);
+        HttpHeaders headers = trailers.trailingHeaders();
+        headers.set("key", "value");
+        assertTrue(ch.writeOutbound(trailers));
+
+        Http2HeadersFrame headerFrame = ch.readOutbound();
+        assertThat(headerFrame.headers().get("key").toString(), is("value"));
+        assertTrue(headerFrame.isEndStream());
+
+        assertThat(ch.readOutbound(), is(nullValue()));
+        assertFalse(ch.finish());
+    }
+
+    @Test
+    public void testUpgradeDataEndWithTrailers() throws Exception {
+        EmbeddedChannel ch = new EmbeddedChannel(new Http2ServerDowngrader());
+        ByteBuf hello = Unpooled.copiedBuffer("hello world", CharsetUtil.UTF_8);
+        LastHttpContent trailers = new DefaultLastHttpContent(hello, true);
+        HttpHeaders headers = trailers.trailingHeaders();
+        headers.set("key", "value");
+        assertTrue(ch.writeOutbound(trailers));
+
+        Http2DataFrame dataFrame = ch.readOutbound();
+        assertThat(dataFrame.content().toString(CharsetUtil.UTF_8), is("hello world"));
+        assertFalse(dataFrame.isEndStream());
+
+        Http2HeadersFrame headerFrame = ch.readOutbound();
+        assertThat(headerFrame.headers().get("key").toString(), is("value"));
+        assertTrue(headerFrame.isEndStream());
+
+        assertThat(ch.readOutbound(), is(nullValue()));
+        assertFalse(ch.finish());
+    }
+
+    @Test
+    public void testDowngradeHeaders() throws Exception {
+        EmbeddedChannel ch = new EmbeddedChannel(new Http2ServerDowngrader());
+        Http2Headers headers = new DefaultHttp2Headers();
+        headers.path("/");
+        headers.method("GET");
+
+        assertTrue(ch.writeInbound(new DefaultHttp2HeadersFrame(headers)));
+
+        HttpRequest request = ch.readInbound();
+        assertThat(request.uri(), is("/"));
+        assertThat(request.method(), is(HttpMethod.GET));
+        assertThat(request.protocolVersion(), is(HttpVersion.HTTP_1_1));
+        assertFalse(request instanceof FullHttpRequest);
+
+        assertThat(ch.readInbound(), is(nullValue()));
+        assertFalse(ch.finish());
+    }
+
+    @Test
+    public void testDowngradeFullHeaders() throws Exception {
+        EmbeddedChannel ch = new EmbeddedChannel(new Http2ServerDowngrader());
+        Http2Headers headers = new DefaultHttp2Headers();
+        headers.path("/");
+        headers.method("GET");
+
+        assertTrue(ch.writeInbound(new DefaultHttp2HeadersFrame(headers, true)));
+
+        FullHttpRequest request = ch.readInbound();
+        assertThat(request.uri(), is("/"));
+        assertThat(request.method(), is(HttpMethod.GET));
+        assertThat(request.protocolVersion(), is(HttpVersion.HTTP_1_1));
+        assertThat(request.content().readableBytes(), is(0));
+        assertTrue(request.trailingHeaders().isEmpty());
+
+        assertThat(ch.readInbound(), is(nullValue()));
+        assertFalse(ch.finish());
+    }
+
+    @Test
+    public void testDowngradeTrailers() throws Exception {
+        EmbeddedChannel ch = new EmbeddedChannel(new Http2ServerDowngrader());
+        Http2Headers headers = new DefaultHttp2Headers();
+        headers.set("key", "value");
+        assertTrue(ch.writeInbound(new DefaultHttp2HeadersFrame(headers, true)));
+
+        LastHttpContent trailers = ch.readInbound();
+        assertThat(trailers.content().readableBytes(), is(0));
+        assertThat(trailers.trailingHeaders().get("key").toString(), is("value"));
+        assertFalse(trailers instanceof FullHttpRequest);
+
+        assertThat(ch.readInbound(), is(nullValue()));
+        assertFalse(ch.finish());
+    }
+
+    @Test
+    public void testDowngradeData() throws Exception {
+        EmbeddedChannel ch = new EmbeddedChannel(new Http2ServerDowngrader());
+        ByteBuf hello = Unpooled.copiedBuffer("hello world", CharsetUtil.UTF_8);
+        assertTrue(ch.writeInbound(new DefaultHttp2DataFrame(hello)));
+
+        HttpContent content = ch.readInbound();
+        assertThat(content.content().toString(CharsetUtil.UTF_8), is("hello world"));
+        assertFalse(content instanceof LastHttpContent);
+
+        assertThat(ch.readInbound(), is(nullValue()));
+        assertFalse(ch.finish());
+    }
+
+    @Test
+    public void testDowngradeEndData() throws Exception {
+        EmbeddedChannel ch = new EmbeddedChannel(new Http2ServerDowngrader());
+        ByteBuf hello = Unpooled.copiedBuffer("hello world", CharsetUtil.UTF_8);
+        assertTrue(ch.writeInbound(new DefaultHttp2DataFrame(hello, true)));
+
+        LastHttpContent content = ch.readInbound();
+        assertThat(content.content().toString(CharsetUtil.UTF_8), is("hello world"));
+        assertTrue(content.trailingHeaders().isEmpty());
+
+        assertThat(ch.readInbound(), is(nullValue()));
+        assertFalse(ch.finish());
+    }
+
+    @Test
+    public void testPassThroughOther() throws Exception {
+        EmbeddedChannel ch = new EmbeddedChannel(new Http2ServerDowngrader());
+        Http2ResetFrame reset = new DefaultHttp2ResetFrame(0);
+        Http2GoAwayFrame goaway = new DefaultHttp2GoAwayFrame(0);
+        assertTrue(ch.writeInbound(reset));
+        assertTrue(ch.writeInbound(goaway));
+
+        assertEquals(ch.readInbound(), reset);
+        assertEquals(ch.readInbound(), goaway);
+
+        assertThat(ch.readInbound(), is(nullValue()));
+        assertFalse(ch.finish());
+    }
+}