-
-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
proposal: attach remote address to context in HTTP3 request #4198
Comments
Hi @oncilla! Not opposed to your change, but I have two questions first:
|
Thanks for even considering this proposal. 1. Why can't you use the address from the http.Request.RemoteAddr?Our addresses cannot easily be represented in the http.Request.RemoteAddr because it is not simply an IP:port. Of course, that is very specific to our use case, but I think as soon as an address is not just IP:port it is not possible with the current mechanism. 2. Is there any precedent for using a context key?There are two similar cases that I would consider related and set somewhat of a precedence:
// LocalAddrContextKey is a context key. It can be used in
// HTTP handlers with Context.Value to access the local
// address the connection arrived on.
// The associated value will be of type net.Addr.
LocalAddrContextKey = &contextKey{"local-addr"} This already allows the http handler implementations to extract the local address from the request context.
type Peer struct {
// Addr is the peer address.
Addr [net](https://pkg.go.dev/net).[Addr](https://pkg.go.dev/net#Addr)
// LocalAddr is the local address.
LocalAddr [net](https://pkg.go.dev/net).[Addr](https://pkg.go.dev/net#Addr)
// AuthInfo is the authentication information of the transport.
// It is nil if there is no transport security being used.
AuthInfo [credentials](https://pkg.go.dev/google.golang.org/[email protected]/credentials).[AuthInfo](https://pkg.go.dev/google.golang.org/[email protected]/credentials#AuthInfo)
} gRPC handlers have full access to the remote address through the context. How does this work on HTTP/2?So far, we have only used HTTP/2 in combination with grpc-go, where this feature is available. I have not investigated how to do this with http2 of the stdlib in too much detail. From a brief skim, it looks like we would need to patch it too to achieve this. |
Thank you for your detailed explanation!
Does it need to though? The server just sets the string to whatever the Line 578 in ff6d575
If you're using a custom |
We are using a custom net.PacketConn, exactly. I'm thinking about this in the context of SCION. There, we have a SCION address, which looks something like "[1-ff00:0:110,2001:db8::1]:443". Now, we could encode the dataplane path in the string output. But that would negatively affect all the other places where the string representation is used. (E.g., in logging output). Probably we could use other techniques, like a singleton that keeps track of the real remote address. I realize that this is somewhat specific to our use case, but I had the suspicion that having the real remote address attached to the context would also be useful to other project. |
How would it be attached to the context though? I assume the way we'd implement this would be equivalent to what we do right now for the |
The diff in the initial post is all that is required for it to work (we have a patched version of quic-go that works). The remote address has all the required things and nothing needs to be encoded additionally. |
I'm confused now. If the remote |
In In |
Oh right, obviously! Sorry for missing the obvious! I'm wondering why Your proposal sounds reasonable to me. Would you mind sending a PR? Please make sure to add a comment explaining why you can't use the field on the request (i.e. string vs |
Add the remote address of the underlying packet connection to the HTTP request context. This is useful for applications that need access to the actual remote address (wrapped in a net.Addr) rather than just its string representation. Fixes quic-go#4198
Me too :)
Sweet. PR is out. Thanks for the swift feedback on this proposal 👍 |
* http3: add remote address to request context Add the remote address of the underlying packet connection to the HTTP request context. This is useful for applications that need access to the actual remote address (wrapped in a net.Addr) rather than just its string representation. Fixes #4198 * add an integration test to the self test suite. I was not sure how deep we want to go to assure the right value is set. For now, it asserts that a net.Addr is present in the context. Due to the dynamic nature of the requests, it is a bit harder to know exactly how the remote address will look like. IPv4 vs IPv6, random high port. I think it is fine to only assert that the value is present.
Hi there,
We are trying to move our control plane from gRPC to connect-go/HTTP3 (see scionproto/scion#4434).
We rely on a feature of gRPC that lets us access the peer address when handling requests.
This is currently not possible with HTTP3, because the remote address is only passed as a regular string on the HTTP request of the stdlib type.
By attaching the remote address to the HTTP request context, we would solve our use case. But I also believe that it is more generally useful for implementing server code.
Enabling this can be done with a very minimal patch. If you would be interested, I will create a PR to upstrem it:
The text was updated successfully, but these errors were encountered: