Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

http3: Is there an equivalent of http.Server.ConnContext #3603

Closed
omz13 opened this issue Oct 20, 2022 · 17 comments · Fixed by #4230
Closed

http3: Is there an equivalent of http.Server.ConnContext #3603

omz13 opened this issue Oct 20, 2022 · 17 comments · Fixed by #4230
Milestone

Comments

@omz13
Copy link

omz13 commented Oct 20, 2022

I am trying to add http/3 support into an existing application.

For my existing application I hook into http.Server.ConnContext so I can insert my own Context. I was hoping that I could easily do something similar with http3.Server, but I can't see any equivalent mechanisms. Am I not seeing something obvious, or is this not implemented?

@marten-seemann
Copy link
Member

Can you explain what your use case is? I’m not sure I fully understand ConnContext.

@omz13
Copy link
Author

omz13 commented Oct 21, 2022

Sure. The use case is to be able to easily pass a user-defined per-connection context.Context to the http.Request that processes the request through http.Handle.

I see in func (s *Server) handleRequest that you are already wrapping the req with some package-specific context; what I am missing is the ability to add user-specific context somewhere into that context.

In net.http this is done by http.Server.ConnContext. Whenever a connection is established, this function is called if provided and this is where you can adjust the context.Context for that connection (typically adding to the context chain). When the connection calls handler.ServerHTTP(rw, req) the context is passed down so it ends up being available in the handler.

@marten-seemann
Copy link
Member

That makes sense. Thank you for the explanation!

I see in func (s *Server) handleRequest that you are already wrapping the req with some package-specific context; what I am missing is the ability to add user-specific context somewhere into that context.

I'm wondering how that would work. In handleRequest we already use the context returned from quic.Stream.Context. In Go, it's not possible to "combine" two contexts, so we'd have to decide which context we use as a base context. Or am I missing something here.

@omz13
Copy link
Author

omz13 commented Oct 21, 2022

It would make sense to go via quic.Stream.Context: a http3.Server.StreamContext(ctx context.Context, c quic.Connection) context.Context to be the equivalent of http.Server.ConnContext(ctx context.Context, c net.Conn) context.Context ?

@carljkempe
Copy link

carljkempe commented Oct 23, 2022

I too am looking to take advantage of something similar. In my case, I'm trying to hook-in and include ClientHello message info. 👋 Simply been trying to determine the client's JA3 fingerprint and pass it along the connection.

@marten-seemann
Copy link
Member

It would make sense to go via quic.Stream.Context: a http3.Server.StreamContext(ctx context.Context, c quic.Connection) context.Context to be the equivalent of http.Server.ConnContext(ctx context.Context, c net.Conn) context.Context ?

The stream context is not set by the user, it's set by the stream: https://github.com/lucas-clemente/quic-go/blob/af30cef57c5c20ebcb599200a96d89f7c33c3d50/send_stream.go#L66-L80

@omz13
Copy link
Author

omz13 commented Oct 23, 2022

The stream context is not set by the user, it's set by the stream

Yes, it is set by the stream, and what I mean is that is probably the place where a subsequent call to a user-supplied function (if supplied) would be where the user could add to that context.

@marten-seemann
Copy link
Member

That’s the QUIC layer though, not http3. It would be very hard to set a context that deep in the stack.

@mattrobenolt
Copy link
Contributor

I am also going to put a +1 in here. Our use case is using a per-connection context to bind a logger, connection-id, and store other metadata about the actual connection.

Without this, there's no way to understand or track multiple requests happening over the same connection, etc.

@marten-seemann
Copy link
Member

I'm not opposed to this, just trying to understand how this would be implemented. Any suggestions would be welcome, as I won't have the bandwidth to work on this myself. Happy to discuss design and review a PR though.

@mattrobenolt
Copy link
Contributor

I'd be happy to take a look, I'm very unfamiliar with any of the codebase here,so would need to dig a bit.

@omz13
Copy link
Author

omz13 commented Nov 17, 2022

I have a POC in my local workspace. When I get five minutes I’ll tidy it up and make a PR.
One thing I did was to insert quic.ConnectionTracingKey into the context in server.handleRequest. I couldn’t see an easy way to get quic connID which might be more useful than tracing key.

@omz13
Copy link
Author

omz13 commented Nov 19, 2022

I submitted #3624 which mainly addresses this issue; @marten-seemann if you are happy with that, I can also submit my expose QUIC connection ID patch which (unless I have got things wrong) provides access to connection ID which can be retrieved and injected by the user-provided function to Server.ConnContext

@woorui
Copy link

woorui commented Jan 13, 2023

It would make sense to go via quic.Stream.Context: a http3.Server.StreamContext(ctx context.Context, c quic.Connection) context.Context to be the equivalent of http.Server.ConnContext(ctx context.Context, c net.Conn) context.Context ?

The stream context is not set by the user, it's set by the stream:

https://github.com/lucas-clemente/quic-go/blob/af30cef57c5c20ebcb599200a96d89f7c33c3d50/send_stream.go#L66-L80

Hi, I also face the same issues in QUIC layer.

I want bind some information to Stream after calling stream, err := Connection.AcceptStream(ctx),
and I can't use context.WithValue to Stream.Context(), Stream.Context().Value(key) never work.

I am using an external map to store the information for working around this.

@rthellend
Copy link
Contributor

It seems to me that all the use cases mentioned on this thread (except the AcceptStream case) can be satisfied with ConnContext where the caller can attach arbitrary data to the context with a func that does something like:

   ConnContext: func(ctx context.Context, c quic.Connection) context.Context {
       ctx = context.WithValue(ctx, ctxKey1, data1)
       ctx = context.WithValue(ctx, ctxKey2, data2)
       // ...
       return ctx
   }

Since this is done at the time the Server is created, any data structure or function can be passed to the handlers.

In my use case, I also need the connection itself, as explained in #4230 (comment).

From a pure API point of view, I would suggest using the same name and behavior of http.Server.ConnContext so that there are no surprises.

For the AcceptStream case, perhaps something like this would work:

func blah() {
        stream, err := Connection.AcceptStream(ctx)
        // ...
        streamWithNewCtx := NewMyStream(stream).WithContext(newCtx)
}

type MyStream struct {
        quic.Stream
        ctx context.Context
}

func NewMyStream(s quic.Stream) MyStream {
        return MyStream{
                Stream: s,
                ctx:    s.Context(),
        }
}

func (s MyStream) Context() context.Context {
        return s.ctx
}

func (s MyStream) WithContext(ctx context.Context) MyStream {
        return MyStream{
                Stream: s.Stream,
                ctx:    ctx,
        }
}

In any case, the AcceptStream case is somewhat orthogonal to the http3.Server case, IMO.

@marten-seemann marten-seemann changed the title Is there an equivalent of http.Server.ConnContext http3: Is there an equivalent of http.Server.ConnContext Jan 4, 2024
@marten-seemann
Copy link
Member

The context passed to AcceptStream is not used on the Stream.Context:

s.ctx, s.ctxCancel = context.WithCancelCause(context.Background())

I'm not opposed to discussing changing that, but that seems to be an orthogonal issue.

I'm leaning towards merging #4230, as it's a simple change, and seems to be exactly what Go itself does: https://github.com/golang/go/blob/6db1102605f227093ea95538f0fe9e46022ad7ea/src/net/http/server.go#L3275-L3281

@omz13, @carljkempe and @mattrobenolt Would you be able to take a look at that PR, if it resolves your problem as well?

@marten-seemann marten-seemann linked a pull request Jan 4, 2024 that will close this issue
@marten-seemann marten-seemann added this to the v0.41 milestone Jan 4, 2024
@mattrobenolt
Copy link
Contributor

Yup, I haven't tested the implementation, but if this works as advertised, this is what I'm looking for. :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants