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

should support HTTP/2 #28

Closed
weissi opened this issue May 8, 2019 · 9 comments
Closed

should support HTTP/2 #28

weissi opened this issue May 8, 2019 · 9 comments
Assignees
Labels
kind/enhancement Improvements to existing feature.

Comments

@weissi
Copy link
Contributor

weissi commented May 8, 2019

HTTP/2 support. Partial example can be found in swift-nio-examples/http2-client. This does HTTP/2 with 'prior knowledge', for this package we also need to support HTTP/2 upgrade

@weissi weissi added the kind/enhancement Improvements to existing feature. label May 8, 2019
@weissi
Copy link
Contributor Author

weissi commented May 8, 2019

apart from the obvious use-case -- to connect to HTTP/2 web servers -- this library could/should then also be used for swift-nio-apns.

@weissi
Copy link
Contributor Author

weissi commented Jul 5, 2019

@artemredkin / @ianpartridge / @tanner0101 should we pull this into the 1.0.0 milestone? I think it would be important to verify that the API actually works for HTTP/2. I don't foresee any issues but it'd be good to validate.
Obviously mostly useful together with a connection pool but ... @Lukasa what do you think?

@ianpartridge
Copy link
Contributor

I think we can ship 1.0 without HTTP/2. Adding connection pooling and HTTP/2 sounds like a great set of 2.0 features and we can make any necessary API changes then.

@Lukasa
Copy link
Collaborator

Lukasa commented Jul 8, 2019

I’m inclined to agree: it seems more important to enable users to start using the library than to throw HTTP/2 support into the client.

@artemredkin
Copy link
Collaborator

Same here

@artemredkin artemredkin added this to the 1.3.0 milestone Jun 13, 2020
@fabianfett
Copy link
Member

Hi,

I took some time and looked into this the last days... I haven't started programming or anything, but I think I should at least document the open questions/ideas I ran into:

I think we should go the lazy route (like most browsers except Safari) and first only support http/2 over TLS. The protocol negotiation should be fairly easy this way and we know before sending the first HTTP request, if we will talk HTTP1 or HTTP2 on this connection.

I think we should be able to achieve the simpler goal with the current architecture. Is this viable?

Further Questions

  • How many concurrent h/2 connections do we want to support to a host? Only one? What do we want to do with EventLoopPreferences?
  • What shall happen if an HTTP/2 connection already has the maximum number of concurrent requests running? SETTINGS_MAX_CONCURRENT_STREAMS (Especially important for something like sending push notifications)
  • Do we want to rename the HTTP1ConnectionProvider to HTTPConnectionProvider and save in the ConnectionsState whether this provider supports connections over HTTP/1.1 or HTTP/2? We should know after the first successful TLS connection.

If you have any other design thoughts in the back of your heads, I'd be happy to know them. 😉

@Lukasa
Copy link
Collaborator

Lukasa commented Sep 8, 2020

I think we should consider plaintext HTTP/2 (h2c) negotiation to be a non-goal. The IETF is considering removing h2c entirely from the next revision of the HTTP/2 RFC as it's essentially non-implemented. Plaintext HTTP/2 is supported, but only in the form of "prior knowledge" negotiation.

I think we should be able to achieve the simpler goal with the current architecture. Is this viable?

Yes.

How many concurrent h/2 connections do we want to support to a host? Only one? What do we want to do with EventLoopPreferences?

Only one. The RFC strongly encourages that we do so:

Clients SHOULD NOT open more than one HTTP/2 connection to a given host and port pair, where the host is derived from a URI, a selected alternative service, or a configured proxy.

What shall happen if an HTTP/2 connection already has the maximum number of concurrent requests running? SETTINGS_MAX_CONCURRENT_STREAMS (Especially important for something like sending push notifications)

Queue until a stream becomes available. swift-nio-http2 already has this ability.

Do we want to rename the HTTP1ConnectionProvider to HTTPConnectionProvider and save in the ConnectionsState whether this provider supports connections over HTTP/1.1 or HTTP/2? We should know after the first successful TLS connection.

TBD. Something somewhere needs to know the difference at some point, but whether it's encoded in the type system or not is a decision that needs to be made when we have a more detailed design.

@fabianfett fabianfett self-assigned this Jun 5, 2021
@fabianfett
Copy link
Member

Work on this has started...

@fabianfett
Copy link
Member

Fixed by #473

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/enhancement Improvements to existing feature.
Projects
None yet
Development

No branches or pull requests

5 participants