add a ConnContext to the quic.Config, remove it from the http3.Server

[marten-seemann]

As pointed out by @mattrobenolt in #4230 (comment), ConnContext should only be called once after the QUIC connection has been accepted.

Currently blocked on the http3 refactor for #3522.

[marten-seemann]

I'm actually not sure how we'd implement this. Currently, we use the stream context:

quic-go/http3/server.go

Lines 546 to 555 in da410a7

	ctx := str.Context()
	ctx = context.WithValue(ctx, ServerContextKey, s)
	ctx = context.WithValue(ctx, http.LocalAddrContextKey, conn.LocalAddr())
	ctx = context.WithValue(ctx, RemoteAddrContextKey, conn.RemoteAddr())
	if s.ConnContext != nil {
	ctx = s.ConnContext(ctx, conn)
	if ctx == nil {
	panic("http3: ConnContext returned nil")
	}
	}

This seems to be the right thing to do, because the request is inherently bound to the stream.

If we want to implement this change, we'd need a context that derives from both the stream and the connection, which is not possible.

Any ideas @mattrobenolt?

[mattrobenolt]

If we want to implement this change, we'd need a context that derives from both the stream and the connection, which is not possible.

Yeah, I'd suspect that's correct. You'd want one originating at the Connection itself, each Stream derives from that.

Not speaking about logistic for the implementation, but conceptually that's my expectation.

Connection -> Stream -> Request

Context is "forked" at each of those. Each Stream inherits from the Connection, and each Request inherits from the Stream. ConnContext is called when the Connection is established, and each Stream, thus each Request, inherits from that top level Connection.

Am I right in that each Stream is 1 HTTP3 request? Or can one Stream handle multiple HTTP3 requests?

[marten-seemann]

The problem here is that we're dealing with the HTTP/3 layer, but streams are opened on the QUIC layer.

Maybe that's the solution: We should add a ConnContext callback to the quic.Connection, not the http3.Server! This would allow us to get rid of the ConnectionTracingKey then. This would be really nice, it never felt like the right abstraction.

Unfortunately, this is a much bigger change, and we might want to implement #3862 first (but maybe not strictly necessary?).

[mattrobenolt]

I would agree, it would naturally be a better hook on the quic layer.

I'm curious how this will translate over to x/net/quic, but I have little skin in the game except for being a user. :)

[marten-seemann]

Unfortunately, this is a much bigger change, and we might want to implement #3862 first (but maybe not strictly necessary?).

Thinking about this more, I don't think this is even necessary. The API will be more useful, but it's pretty much orthogonal to this issue.

I'm curious how this will translate over to x/net/quic

Oh well, nothing we have to worry about here :)
We'll continue evolving the API here whenever we think it makes sense.

[rthellend]

A simple way to implement the original idea would be to use http3.connection:

• Call Server.ConnContext() in Server.handleConn() right before newConnection() is called.
• Set the returned value as hconn.requestBaseCtx

Then in Server.handleRequest():

        ctx := conn.requestBaseCtx
        ctx = context.WithValue(ctx, ServerContextKey, s)
        ctx = context.WithValue(ctx, http.LocalAddrContextKey, conn.LocalAddr())
        ctx = context.WithValue(ctx, RemoteAddrContextKey, conn.RemoteAddr())
        req = req.WithContext(ctx)

Arguably, the other values could also be set in handleConn.

[rthellend]

Nevermind. My suggestion won't work. The only way this can work is if the stream ctx is tied to the conn ctx.