-
Notifications
You must be signed in to change notification settings - Fork 24
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
Add incoming and outgoing handlers for HTTP/2 #129
base: main
Are you sure you want to change the base?
Conversation
This PR adds the interfaces `http2-incoming-handler` and `http2-outgoing-handler` and the world `http2-proxy`, which exports and imports those interfaces, respectively. Additionally, it adds the variant `types.http-version` and a method `http-version()-> http-version` to both `types.incoming-request` and `types.incoming-response`. The motivation for these additions is that there are use cases that require specifically HTTP/2 to be used for both incoming and outgoing connections, so wasi-http should provide a way to enforce this requirement. Most notably, gRPC is specified as layered on top of HTTP/2, not HTTP more generally, so attempting to transport it over HTTP/1.1 or HTTP/3 would fail. An alternative API design would be to add a method each to `types.{ outgoing-request, outgoing-response }` to opt in to HTTP/2. I think the design proposed here is better in two regards: 1. It makes static analysis of an application's intent much easier: instead of having to check for use of a `set-http-version` method on outgoing requests/responses and then making assumptions about which version that might opt in to, the normal validations for targeted worlds is sufficient. 2. It matches what other APIs are doing and should make it easier to implement them in terms of wasi-http. As an example of the latter point, Go's standard library uses the same approach: Go's [`net/http` package](https://pkg.go.dev/net/[email protected]#hdr-HTTP_2) transparently supports multiple HTTP versions, and explicitly opting in to HTTP/2 is possible using the [`x/net/http2` package](https://pkg.go.dev/golang.org/x/net/http2). Similarly, the Rust ecosystem's Hyper crate supports [automatically choosing](https://docs.rs/hyper/latest/hyper/server/conn/index.html) an HTTP version, or using `http1` or `http2` modules specifically. In both cases, the other types in the HTTP APIs remain the same, as in this proposal. Signed-off-by: Till Schneidereit <[email protected]>
I'd be very surprised if nobody had any good arguments for any tweaks to the API at all, so I'll hold off on updating the ABI files until the API is more final |
HTTP11, | ||
HTTP2, | ||
HTTP3 | ||
other(string) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Adding HTTP4 to the enum would be API breaking change.
There is similar discussion about TSL version, with regards to future compatibility.
WebAssembly/wasi-sockets#104 (comment)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, I saw that. My intuition is that HTTP versions are introduced rarely enough that we'll have solved this by the time HTTP/4 comes around, but I'm happy to change it if people feel that that's not right.
C# could opt in into HTTP version dynamically via HttpRequestMessage.Version Does this proposal mean we would have to import both interfaces for all dotnet apps ? |
That's a very good question. I don't know enough about .Net's configuration options, but maybe it'd be possible to make the inclusion optional by configuring things such that the requisite package isn't linked in? |
And I assume you're also not able to prove that an app doesn't use HTTP/2? Because That seems strictly more important, as every host that can support the |
The value of the The new interface/methods is breaking change, because we will need new version of the host. But I think new-interface-per-feature approach is fragmenting the otherwise nice API surface. Because we are going to break API compat again in p3 for promises, we could reconcile it back to one interface then. |
I realized that it's more complex. There is also HttpRequestMessage.VersionPolicy which defines what Do I understand correctly that for Essentially the new interface partially expressing the policy ? Could I use the new interface to talk to HTTP11 only server ? |
My thinking is that explicitly separating the interface is actively desirable in some, though unfortunately not all cases. Specifically for APIs that express the same kind of separation, as Go's standard library and Hyper do, the separation allows us to provide implementations of these APIs that only pull in It's unfortunate that this doesn't work for all languages, including C#, but I don't think that's a reason to go with a design that doesn't make it work for any languages.
As described above, I think there is value in having multiple interfaces. If we get to a consensus that this assumption is wrong, then I think we should indeed go with something more like Joel's proposal and not have the API proposed here to begin with. |
That is more complex indeed :(
I think the best fit for the currently existing wasi-http interface is dotnet's I think we could add support for both
Basically, one would express a requirement for a minimum http version by choosing the right interface (and I'd be happy to add This would allow components to continue targeting environments that can't provide HTTP/2 semantics (such as
I think that's correct, with slight differences as described above.
With the suggested changes above, it would be possible to use the non-http2 interface this way. |
Heh, yeah I have to say I was also a little surprised to see this PR. I feel like this raises a question about what the scope should be for What this PR seems to be adding is a version of the But I realize there are also practical limitations in the component model. And so if this PR is intended to be a stepping stone to eventually consolidate on that more general interface, I definitely see the value in that. I don't want perfect to be the enemy of good - but I also want to make sure that we don't end up with various incompatible, protocol-specific, diverging interfaces. So I guess I'm also curious about what we want the eventual end-state to be for edit: Intuitively, I feel like asking a question like: "Does this HTTP server make exclusive use of HTTP/2?" feels like an instance of a more general "feature detection" feature or pattern. When targeting the web, I can see us asking similar questions. For example: "Does this HTTP client persist cookies?" is another thing people will probably want to ask. Idk, maybe there's something along those lines we might be able to improve instead? |
/// The implementor of this function must write a response to the | ||
/// `response-outparam` before returning, or else the caller will respond | ||
/// with an error on its behalf. | ||
@since(version = 0.2.2) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This PR implements an interface which has not yet gone through a WASI SG vote, and until it has it can't be included as part of a WASI release. Can you please change the @since
gates to @unstable
gates instead?:
@since(version = 0.2.2) | |
@unstable(feature = "http-http2") |
Once this PR lands, please also file a PR on the WASI proposal tracker so we can track this extension as part of the WASI phase process. Full details on the process for adding interfaces to existing WASI proposals can be found here. Thanks!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh of course yes: I'll make sure to change this before this lands—if it ever does. Thank you!
I agree with @yoshuawuyts that we should keep wasi-http a generic and protocol-agnostic interface, as stated in the Goals section of readme "The proposal intends to abstract over HTTP version and transport protocol choices (such as HTTP/1.1, HTTP/2 or HTTP/3)" With that said, I do see values to a static representation of http/2 for gRPC. What if we make a separate "This package is low-level and intended to be used directly by very few people. Most users will use it indirectly through the automatic use by the net/http package (from Go 1.6 and later)" Essentially, I am proposing to move http/2 to a separate proposal, outside of wasi-http. Will wasi-http supports http/2? Yes. but it's scope should remain to be the semantics of http. |
@yoshuawuyts and @Mossaka thank you for much for the thoughtful feedback here! I'll lay out my reasoning and how I see various constraints and design goals in a bit more detail below, but before I do: I think @Mossaka's proposal is right, and that this really should be a separate Now for the constraints and design goals. Assumed constraints
Assumed design goals
My conclusions from these constraints and goalsBased on this, I think we should have a strong signal to use exactly For the former, I had shied away from making an entire new proposal because it seemed like that proposal would be too small to be worth it. But I now realize that @Mossaka is right and that yes, it makes sense for this to be For the latter, I think it does make sense to add support for expressing preferred protocol choices and being able to check them dynamically as part of Crucially though, I think an application making use of
@yoshuawuyts on this particular, as described above I think we're dealing with a much more fundamental feature here. The cookies example you give seems useful to query, but won't make a binary difference on whether the entire way the component is communicating is implementable in the host environment or not.
I hope based on the above it's clear that I don't think what I'm proposing is a stepping stone. Nor do I think it's working around any limitations of the component model. I don't love the proposed solution here in the least, and would love for someone to come up with a better one, but given the constraints above I can't see how that would involve a single |
It seems pretty useful to have a single portable interface that a component can import to do gRPC that is implemented in terms of either H2 outside the browser or gRPC-Web inside the browser (or, before too long gRPC-over-HTTP/3, and also one day, when browsers eventually support trailers, gRPC-in-browser). Instead of creating a whole new Other reasons in favor of a
That being said, I'm not sure what exactly |
I agree that that seems useful, in the abstract. I'm less sure it's useful in the real world, though: I'm fairly certain that introducing We're already asking language ecosystems to integrate
Is it? What I'm thinking of boils down to moving the API proposed here into it's own
Do we have any evidence that this portability is something people would actually want? My understanding is that people explicitly decide on whether to use gRPC or gRPC-Web, and would not want the host to make any implicit decisions for them. And to the extent that servers and clients support both protocol versions, they already have implementations of both, which we can readily support if we provide
Can you say more about what would force us to add "all these HTTP/2 details"? I'm not sure why providing a way to explicitly choose HTTP/2 as the protocol version would somehow require us to also provide a way to tweak all possible knobs that can be tweaked for HTTP/2.
Are there more examples of subtle details than this one? And do we have any evidence that this is a practical concern? At a cursory glance at the canonical gRPC implementations for Rust and Go I couldn't find where those would have code to ensure that HTTP/2 is configured exactly right, leading me to the hunch that maybe this requirement is met by default in practice? And if that's the case, could we address this issue by adding a requirement to keep to this particular headers/trailers framing?
I think it's totally fine to add
Which gets me back to my proposal of doing pretty much exactly this with |
I agree with Till that We've already demonstrated that |
Could we just add I think that specific |
If we have a whole interface focused on
I was basing this on your own comment above, where you mention applications probing for HTTP/2 and falling back to gRPC-Web. Googling around shows a number of places doing this. That suggests that there is an abstraction presented to the developer that allows this sort of transparent fallback. Instead of having N libraries/frameworks/languages implement this N times, it seems valuable to move this virtualization on the other side of the interface. But I agree this isn't a hard requirement.
It was an actual observed bug that required changing a real HTTP server's implementation based on a "this is for gRPC" flag set on the backend to disable an optimization that otherwise seems allowed by HTTP semantics. It's possible most implementations don't do this optimization, however, so maybe it's not enough of a problem in practice though. Overall, I do appreciate the argument that the most expedient "cut point" is in the standard library HTTP implementation, where the context that we're doing gRPC has been lost. But then it seems like the next-best option is to do the minimal addition to |
I don't know how to say this without it sounding facetious: I'd be happy to say "yes" to more things in this package as long as someone else steps up to do the work. In lieu of that, I find it pretty easy to say "no" 🤷🏻
Ah, that's a good data point. It seems like we can address it by specifying that hosts must not do those optimizations? And if people care enough, we could work with them to support making them configurable?
As I said, I do agree that we should have a dynamic way to configure this, so I'm happy to focus on this for I'd like the static analyzability that |
It remains to define if the host would prefer max or min version in given range. Or we are better off with another field/enum I also don' know if this is all that's missing for grpc. Today we also mentioned TLS version in some meeting. Should that be also something that we want to set min/max range ? I don't have specific use-case for this at the moment. |
Are there situations in which an application will only work with a specific TLS version used, but that specific version won't be negotiated by the normal protocol negotiation process? If so, I agree that a similar situation exists, but it's not something I've heard of. |
I imagine that particular application could have stronger security expectation than the host it's running in. Requiring higher TLS version and breaking rather than compromising the communication. There is lot of complexity in it's governance. Those decisions are split differently between OS (or WASI host) and the application. I'm not an expert in that. I guess we don't need to solve it now. I just wanted to show that there are other dimensions/features than HTTP version. |
Oh, that is indeed a compelling scenario, thank you! Maybe that points to the fact that if we do want to have good static analyzability for these kinds of things, we might want to have a WIT-level mechanism for composing interfaces in fine-grained ways. That all would only be useful if it can be made use of in toolchains, of course. I.e., if it'd be possible to create good developer experiences that result in components that have the interface they really do need, instead of one that they might need in order to support all the configurability the toolchain wants to provide. Anyway, all the more reason to focus on the dynamic configuration part for now. |
This PR adds the interfaces
http2-incoming-handler
andhttp2-outgoing-handler
and the worldhttp2-proxy
, which exports and imports those interfaces, respectively.Additionally, it adds the variant
types.http-version
and a methodhttp-version()-> http-version
to bothtypes.incoming-request
andtypes.incoming-response
.The motivation for these additions is that there are use cases that require specifically HTTP/2 to be used for both incoming and outgoing connections, so wasi-http should provide a way to enforce this requirement.
Most notably, gRPC is specified as layered on top of HTTP/2, not HTTP more generally, so attempting to transport it over HTTP/1.1 or HTTP/3 would fail.
An alternative API design would be to add a method each to
types.{ outgoing-request, outgoing-response }
to opt in to HTTP/2. I think the design proposed here is better in two regards:set-http-version
method on outgoing requests/responses and then making assumptions about which version that might opt in to, the normal validations for targeted worlds is sufficient.As an example of the latter point, Go's standard library uses the same approach: Go's
net/http
package transparently supports multiple HTTP versions, and explicitly opting in to HTTP/2 is possible using thex/net/http2
package.Similarly, the Rust ecosystem's Hyper crate supports automatically choosing an HTTP version, or using
http1
orhttp2
modules specifically.In both cases, the other types in the HTTP APIs remain the same, as in this proposal.