Ignore unrecognised frames.

[Lukasa]

RFC 7540 says:

Implementations MUST ignore unknown or unsupported values in all extensible protocol elements. Implementations MUST discard frames that have unknown or unsupported types.

Currently hyper-h2 contravenes this policy: it throws exceptions when it encounters unsupported frame types. This is not a good plan, especially as extension frames are in active development and could be encountered on the open web.

A good question would be: how do we handle these frames? One option is to define a new event (UnexpectedFrameReceived) that simply hands the hyperframe frame object up the stack to allow the calling code to do whatever it needs. Another option is to simply swallow all unexpected frames and not emit them to user code.

I lean towards doing the first, but I'd like to know what @Kriechi or @mhils think, as they're the most likely to be interested in actually accessing the unexpected frame data.

[Lukasa]

BTW if anyone wants to pick this up you should feel free: this is almost certainly a change that requires relatively little deep understanding of HTTP/2, which makes it a great contribution for anyone who is looking for something to do. I probably won't let this sit more than week before I work on it myself, but I'd like to give others an opportunity to do it first.

[haikuginger]

I'd like to grab this one, if I may. Given that the spec says MUST discard, I'd actually lean towards the second option, but given that the boundary of what an "implementation" is is a bit fuzzy, I'm fine the other way as well.

[Lukasa]

@haikuginger You absolutely may.

So as to doing the first or second option, I think the biggest issue is that while an implementation nominally MUST discard, it is potentially useful to be able to flag to the user that we have discarded, so they can log or do whatever is needed. The only tool hyper-h2 has to communicate this is events, so that's what we'd have to use.

[LilyFoote]

I just had a thought - would it be horribly difficult to allow a user of h2 to plug in custom handling for extension frames? Would that be a nicer solution than UnexpectedFrameReceived?

[Lukasa]

@Ian-Foote So this is something I have thought about in #57. The general answer is that yes, it would be good to allow users to have custom frame handling.

Actually doing it is a chunk of work, though, and doesn't obviate the need for this. If you want to pursue it though, it's well worth doing!

[alexwlchan]

I’d lean towards an UnexpectedFrameReceived event, in part because h2 doesn’t have extensibility framework. h2 might not know how to handle an unexpected value as part of vanilla HTTP/2, but the caller may know about that extension and be ready to handle it. Simply dropping the frame on the floor makes life difficult for them.

(Making it a non-event also introduces fun debugging scenarios where you can’t tell if your frame never arrived, or if h2 got confused about the frame and silently dropped it. Silent discards are the devil’s work.)

[mhils]

For @mitmproxy, having an UnexpectedFrameReceived event that provides the raw frame contents would be great. The RFC does not mention this explicitly, but if implementations (which should include proxies) MUST discard frames that have unknown or unsupported types, we at least need a way to communicate to users that we dropped frames.

[Lukasa]

Cool, so I think we have rough consensus around an event.

[haikuginger]

So, I think this is actually slightly bigger than I initially thought. Still very happy to do it, but I think there's actually work to consider in three places here.

First, in connection.py where we actually raise the exception. This is the simplest place - basically, it currently looks to determine whether the frame class is one that hyper-h2 has a defined behavior for; if not, it raises. It should be quite simple to modify this to return an event instead.

However, unless I'm misreading the code, we won't ever get to that point, because of this and this in framebuffer.py. If we get an UnknownFrameError from hyperframe, then we'll just save None as the frame - and it will be skipped over when we iterate across the FrameBuffer.

Right now, it appears that hyper-h2 implements behavior for each of the 11 types of frames that hyperframe implements; the upshot of this is that the frame data gets thrown away before hyper-h2 even gets around to parsing it in the context of the state machine- in other words, unless I'm mistaken, I don't think we'll ever raise the very nice UnsupportedFrameError we have right now in normal use. It's only reachable if hyperframe adds support for a frame type and h2 hasn't yet - which seems like a somewhat unlikely scenario, and not what we have in mind for extension frames.

What I'd like to do, once I run it past you fine folks for feedback, is to add a generic ExtensionFrame type to hyperframe - essentially, any frames that don't have a recognized identifier would be sent up as this frame type. From there, in h2, I'd just add a handler to create an UnrecognizedFrame event bearing the frame itself for use in downstream code.

@Lukasa, thoughts?

[Lukasa]

Hrm. Yeah, that's probably a good idea. That'll be a breaking change in hyperframe, and we may want to make "strict validation" a mode that hyperframe can use, but I think generally it seems like the right way to go.