Repository: netty/netty
Issue: NPE in InboundHttp2ToHttpAdapter When a HTTP/2 PUSH_PROMISE comes in
Labels: defect

### Expected behavior
No null pointer exceptions

### Actual behavior
When a HTTP/2 PUSH_PROMISE comes in, we call **onPushPromiseRead** in InboundHttp2ToHttpAdapter. 
This makes us create a FullHttpMessage, which eventually calls "HttpConversionUtil.parseStatus", which occurs always NPE because status is null.

io.netty.handler.codec.http2.Http2Exception: Unrecognized HTTP status code 'null' encountered in translation to HTTP/1.x
        at io.netty.handler.codec.http2.Http2Exception.connectionError(Http2Exception.java:99)
        at io.netty.handler.codec.http2.HttpConversionUtil.parseStatus(HttpConversionUtil.java:182)
        at io.netty.handler.codec.http2.HttpConversionUtil.toHttpResponse(HttpConversionUtil.java:204)
        at io.netty.handler.codec.http2.InboundHttp2ToHttpAdapter.newMessage(InboundHttp2ToHttpAdapter.java:157)
        at io.netty.handler.codec.http2.InboundHttp2ToHttpAdapter.processHeadersBegin(InboundHttp2ToHttpAdapter.java:190)
        at io.netty.handler.codec.http2.InboundHttp2ToHttpAdapter.onPushPromiseRead(InboundHttp2ToHttpAdapter.java:299)
        at io.netty.handler.codec.http2.Http2FrameListenerDecorator.onPushPromiseRead(Http2FrameListenerDecorator.java:85)
        at io.netty.handler.codec.http2.DefaultHttp2ConnectionDecoder$FrameReadListener.onPushPromiseRead(DefaultHttp2ConnectionDecoder.java:478)
        at io.netty.handler.codec.http2.Http2InboundFrameLogger$1.onPushPromiseRead(Http2InboundFrameLogger.java:112)
        at io.netty.handler.codec.http2.DefaultHttp2FrameReader$3.processFragment(DefaultHttp2FrameReader.java:565)
        at io.netty.handler.codec.http2.DefaultHttp2FrameReader.readPushPromiseFrame(DefaultHttp2FrameReader.java:573)
        at io.netty.handler.codec.http2.DefaultHttp2FrameReader.processPayloadState(DefaultHttp2FrameReader.java:266)
        at io.netty.handler.codec.http2.DefaultHttp2FrameReader.readFrame(DefaultHttp2FrameReader.java:160)
        at io.netty.handler.codec.http2.Http2InboundFrameLogger.readFrame(Http2InboundFrameLogger.java:41)
        at io.netty.handler.codec.http2.DefaultHttp2ConnectionDecoder.decodeFrame(DefaultHttp2ConnectionDecoder.java:116)
        at io.netty.handler.codec.http2.Http2ConnectionHandler$FrameDecoder.decode(Http2ConnectionHandler.java:353)
        at io.netty.handler.codec.http2.Http2ConnectionHandler.decode(Http2ConnectionHandler.java:413)
        at io.netty.handler.codec.ByteToMessageDecoder.decodeRemovalReentryProtection(ByteToMessageDecoder.java:489)
        at io.netty.handler.codec.ByteToMessageDecoder.callDecode(ByteToMessageDecoder.java:428)
        at io.netty.handler.codec.ByteToMessageDecoder.channelRead(ByteToMessageDecoder.java:265)
        at io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:362)
        at io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:348)
        at io.netty.channel.AbstractChannelHandlerContext.fireChannelRead(AbstractChannelHandlerContext.java:340)
        at io.netty.handler.ssl.SslHandler.unwrap(SslHandler.java:1273)
        at io.netty.handler.ssl.SslHandler.decode(SslHandler.java:1084)
        at io.netty.handler.codec.ByteToMessageDecoder.decodeRemovalReentryProtection(ByteToMessageDecoder.java:489)
        at io.netty.handler.codec.ByteToMessageDecoder.callDecode(ByteToMessageDecoder.java:428)
        at io.netty.handler.codec.ByteToMessageDecoder.channelRead(ByteToMessageDecoder.java:265)
        at io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:362)
        at io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:348)
        at io.netty.channel.AbstractChannelHandlerContext.fireChannelRead(AbstractChannelHandlerContext.java:340)
        at io.netty.channel.DefaultChannelPipeline$HeadContext.channelRead(DefaultChannelPipeline.java:1334)
        at io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:362)
        at io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:348)
        at io.netty.channel.DefaultChannelPipeline.fireChannelRead(DefaultChannelPipeline.java:926)
        at io.netty.channel.nio.AbstractNioByteChannel$NioByteUnsafe.read(AbstractNioByteChannel.java:134)
        at io.netty.channel.nio.NioEventLoop.processSelectedKey(NioEventLoop.java:644)
        at io.netty.channel.nio.NioEventLoop.processSelectedKeysOptimized(NioEventLoop.java:579)
        at io.netty.channel.nio.NioEventLoop.processSelectedKeys(NioEventLoop.java:496)
        at io.netty.channel.nio.NioEventLoop.run(NioEventLoop.java:458)
        at io.netty.util.concurrent.SingleThreadEventExecutor$5.run(SingleThreadEventExecutor.java:858)
        at io.netty.util.concurrent.DefaultThreadFactory$DefaultRunnableDecorator.run(DefaultThreadFactory.java:138)
        at java.lang.Thread.run(Thread.java:748)
Caused by: java.lang.NullPointerException
        at io.netty.handler.codec.http.HttpResponseStatus.parseLine(HttpResponseStatus.java:458)
        at io.netty.handler.codec.http2.HttpConversionUtil.parseStatus(HttpConversionUtil.java:175)
        ... 41 more


### Steps to reproduce

### Minimal yet complete reproducer code (or URL to code)

### Netty version
4.1.15.Final

### JVM version (e.g. `java -version`)
java version "1.8.0_121"
Java(TM) SE Runtime Environment (build 1.8.0_121-b13)
Java HotSpot(TM) 64-Bit Server VM (build 25.121-b13, mixed mode)

### OS version (e.g. `uname -a`)
Linux UD-sunggonsong 4.4.0-93-generic #116-Ubuntu SMP Fri Aug 11 21:17:51 UTC 2017 x86_64 x86_64 x86_64 GNU/Linux


Issue comments:
Fixed 

Pull request review comments:
so created is more correct then ok ?
@normanmaurer 
https://tools.ietf.org/html/rfc7540#section-8.2.1, 
The PUSH_PROMISE frame contains a header block that contains a complete set of request header fields that the server attributes to the request.
In fact, the first time I used OK as the Http response status, I changed the status to Created by referring to the test case (In the previous test case, we used 201 CREATED).
Since it is not appropriate to define the response status on the rfc7540 spec, I think it is not important to be 200 or 201.
According to the rfc7540 specification,

> https://tools.ietf.org/html/rfc7540#section-8.2.1
> Server push is semantically equivalent to a server responding to a
> request; however, in this case, that request is also sent by the
> server, as a PUSH_PROMISE frame.

> https://tools.ietf.org/html/rfc7540#section-8.2.2
> After sending the PUSH_PROMISE frame, the server can begin delivering
>  the pushed response as a response (Section 8.1.2.4) on a server-
>   initiated stream that uses the promised stream identifier.
@normanmaurer The server sends the request headers in PUSH_PROMISE.
However, if you specify the response status in "onPushPromiseRead" to prevent NPE, 200 seems to be better than 201.
WDYT?


Patch that resolved the issue:
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 0c8ba812c62..8ab511ea42e 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
@@ -29,6 +29,7 @@
 import static io.netty.handler.codec.http2.Http2Error.INTERNAL_ERROR;
 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.http.HttpResponseStatus.OK;
 import static io.netty.util.internal.ObjectUtil.checkNotNull;
 
 /**
@@ -296,6 +297,14 @@ public void onPushPromiseRead(ChannelHandlerContext ctx, int streamId, int promi
             Http2Headers headers, int padding) throws Http2Exception {
         // A push promise should not be allowed to add headers to an existing stream
         Http2Stream promisedStream = connection.stream(promisedStreamId);
+        if (headers.status() == null) {
+            // A PUSH_PROMISE frame has no Http response status.
+            // https://tools.ietf.org/html/rfc7540#section-8.2.1
+            // Server push is semantically equivalent to a server responding to a
+            // request; however, in this case, that request is also sent by the
+            // server, as a PUSH_PROMISE frame.
+            headers.status(OK.codeAsText());
+        }
         FullHttpMessage msg = processHeadersBegin(ctx, promisedStream, headers, false, false, false);
         if (msg == null) {
             throw connectionError(PROTOCOL_ERROR, "Push Promise Frame received for pre-existing stream id %d",


Test patch:
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 33393afc0a3..07fa916cbc3 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
@@ -498,7 +498,12 @@ public void run() throws Http2Exception {
             assertEquals(request, capturedRequests.get(0));
 
             final Http2Headers http2Headers = new DefaultHttp2Headers().status(new AsciiString("200"));
-            final Http2Headers http2Headers2 = new DefaultHttp2Headers().status(new AsciiString("201"))
+            // The PUSH_PROMISE frame includes a header block that contains a
+            // complete set of request header fields that the server attributes to
+            // the request.
+            // https://tools.ietf.org/html/rfc7540#section-8.2.1
+            // Therefore, we should consider the case where there is no Http response status.
+            final Http2Headers http2Headers2 = new DefaultHttp2Headers()
                     .scheme(new AsciiString("https"))
                     .authority(new AsciiString("example.org"));
             runInChannel(serverConnectedChannel, new Http2Runnable() {