Repository: netty/netty
Issue: HTTP2 Multiplex API support for unknown/custom frames

### Expected behavior

The HTTP/2 spec explicitly permits extension of the protocol in the form of new frame types:
http://httpwg.org/specs/rfc7540.html#rfc.section.5.5

I would like/expect to be able to write/read custom frame types using the HTTP2 Multiplex API.  That is, if I write a Http2StreamFrame to a child channel with e.g., `frameType == (byte) 20`, I would be able to read the frame on the server's corresponding child channel.

### Actual behavior

The HTTP2 Multiplex API allows any object of type `Http2StreamFrame`:
https://github.com/netty/netty/blob/501662a77f8166382eefd18a8620983d27f59d4a/codec-http2/src/main/java/io/netty/handler/codec/http2/Http2MultiplexCodec.java#L993

The corresponding write, however, rejects any object that is of type `Http2Frame` and not one of the spec-defined frame types:
https://github.com/netty/netty/blob/501662a77f8166382eefd18a8620983d27f59d4a/codec-http2/src/main/java/io/netty/handler/codec/http2/Http2FrameCodec.java#L299-L304

Writing a custom implementation of `Http2StreamFrame` or `Http2UnknownFrame` to a child channel will be rejected by the `Http2FrameCodec`.

Issue comments:
@Bennett-Lynch PTAL https://github.com/netty/netty/pull/7867
Fixed..
Thanks for the quick fix!
Np 

> Am 13.04.2018 um 17:33 schrieb Bennett Lynch <notifications@github.com>:
> 
> Thanks for the quick fix!
> 
> —
> You are receiving this because you modified the open/close state.
> Reply to this email directly, view it on GitHub, or mute the thread.
> 

@normanmaurer Were you able to confirm a corresponding inbound read from the outbound writeUnknownFrame() unit test you added?  I'm unable to detect a read on either the connection or child channel pipeline, and I'm having trouble pinpointing where it might be getting lost in the codecs.
In my simple unit test it seemed to work... that said maybe there is a bug somewhere. Please re-open this issue if you think there may be one and I will try to find some time to check
@normanmaurer I'm hesitant to re-open as I might be doing something wrong.

When trying to write the following to a client's Http2 multiplex child channel:
```
Http2UnknownFrame unknownFrame = new DefaultHttp2UnknownFrame(
        (byte) 20,
        new Http2Flags().ack(false),
        Unpooled.buffer().writeInt(1)
);
unknownFrame.stream(childChannel.stream());
```
I get the following error:
```
java.lang.IllegalArgumentException: Message must be an Http2StreamFrame: DefaultHttp2UnknownFrame(frameType=20, stream=3, flags=value = 0 (), content=UnpooledByteBufAllocator$InstrumentedUnpooledUnsafeHeapByteBuf(ridx: 0, widx: 4, cap: 256))
	at io.netty.handler.codec.http2.Http2MultiplexCodec$DefaultHttp2StreamChannel$Http2ChannelUnsafe.write(Http2MultiplexCodec.java:1028)
	...
```

So I cloned `DefaultHttp2UnknownFrame` just to implement `Http2StreamFrame`.  The write on the client side then succeeds.  The server then fails to read the frame with the following error:
```
java.lang.NullPointerException: null
	at io.netty.handler.codec.http2.Http2FrameCodec$FrameListener.requireStream(Http2FrameCodec.java:575)
	at io.netty.handler.codec.http2.Http2FrameCodec$FrameListener.onUnknownFrame(Http2FrameCodec.java:497)
	at io.netty.handler.codec.http2.DefaultHttp2ConnectionDecoder.onUnknownFrame0(DefaultHttp2ConnectionDecoder.java:171)
	at io.netty.handler.codec.http2.DefaultHttp2ConnectionDecoder$FrameReadListener.onUnknownFrame(DefaultHttp2ConnectionDecoder.java:517)
	at io.netty.handler.codec.http2.Http2InboundFrameLogger$1.onUnknownFrame(Http2InboundFrameLogger.java:133)
	at io.netty.handler.codec.http2.DefaultHttp2FrameReader.readUnknownFrame(DefaultHttp2FrameReader.java:616)
	at io.netty.handler.codec.http2.DefaultHttp2FrameReader.processPayloadState(DefaultHttp2FrameReader.java:281)
	at io.netty.handler.codec.http2.DefaultHttp2FrameReader.readFrame(DefaultHttp2FrameReader.java:160)
	...
```

I'm a little bit confused by this as I am explicitly setting the stream on the client side.  I'm not sure where that's getting lost when going across the wire. DATA frames are being sent successfully on the same child channel.

If it looks like I'm doing something blatantly wrong, could you kindly point it out?  If not, then maybe this is worth re-opening.

And apologies for not having a concrete reproducible example. My attempts are embedded in some separate application logic, and there's no client/server multiplex example in this repository for me to experiment with.

Pull request review comments:

Patch that resolved the issue:
diff --git a/codec-http2/src/main/java/io/netty/handler/codec/http2/Http2UnknownFrame.java b/codec-http2/src/main/java/io/netty/handler/codec/http2/Http2UnknownFrame.java
index 51eeafaf0d9..41e578703a5 100644
--- a/codec-http2/src/main/java/io/netty/handler/codec/http2/Http2UnknownFrame.java
+++ b/codec-http2/src/main/java/io/netty/handler/codec/http2/Http2UnknownFrame.java
@@ -20,13 +20,12 @@
 import io.netty.util.internal.UnstableApi;
 
 @UnstableApi
-public interface Http2UnknownFrame extends Http2Frame, ByteBufHolder {
+public interface Http2UnknownFrame extends Http2StreamFrame, ByteBufHolder {
 
+    @Override
     Http2FrameStream stream();
 
-    /**
-     * Set the {@link Http2FrameStream} object for this frame.
-     */
+    @Override
     Http2UnknownFrame stream(Http2FrameStream stream);
 
     byte frameType();


Test patch:
diff --git a/codec-http2/src/test/java/io/netty/handler/codec/http2/Http2MultiplexCodecTest.java b/codec-http2/src/test/java/io/netty/handler/codec/http2/Http2MultiplexCodecTest.java
index 7a0e8c6a3a0..f07b218e095 100644
--- a/codec-http2/src/test/java/io/netty/handler/codec/http2/Http2MultiplexCodecTest.java
+++ b/codec-http2/src/test/java/io/netty/handler/codec/http2/Http2MultiplexCodecTest.java
@@ -104,6 +104,43 @@ public void tearDown() throws Exception {
     // TODO(buchgr): GOAWAY Logic
     // TODO(buchgr): Test ChannelConfig.setMaxMessagesPerRead
 
+    @Test
+    public void writeUnknownFrame() {
+        childChannelInitializer.handler = new ChannelInboundHandlerAdapter() {
+            @Override
+            public void channelActive(ChannelHandlerContext ctx) {
+                ctx.writeAndFlush(new DefaultHttp2HeadersFrame(new DefaultHttp2Headers()));
+                ctx.writeAndFlush(new DefaultHttp2UnknownFrame((byte) 99, new Http2Flags()));
+                ctx.fireChannelActive();
+            }
+        };
+
+        Channel childChannel = newOutboundStream();
+        assertTrue(childChannel.isActive());
+
+        Http2FrameStream stream = readOutboundHeadersAndAssignId();
+        parentChannel.runPendingTasks();
+
+        Http2UnknownFrame frame = parentChannel.readOutbound();
+        assertEquals(stream, frame.stream());
+        assertEquals(99, frame.frameType());
+        assertEquals(new Http2Flags(), frame.flags());
+        frame.release();
+    }
+
+    @Test
+    public void readUnkownFrame() {
+        LastInboundHandler inboundHandler = streamActiveAndWriteHeaders(inboundStream);
+        codec.onHttp2Frame(new DefaultHttp2UnknownFrame((byte) 99, new Http2Flags()).stream(inboundStream));
+        codec.onChannelReadComplete();
+
+        // headers and unknown frame
+        verifyFramesMultiplexedToCorrectChannel(inboundStream, inboundHandler, 2);
+
+        Channel childChannel = newOutboundStream();
+        assertTrue(childChannel.isActive());
+    }
+
     @Test
     public void headerAndDataFramesShouldBeDelivered() {
         LastInboundHandler inboundHandler = new LastInboundHandler();