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

force http1 #1612

Merged
merged 2 commits into from
Jun 8, 2022
Merged

force http1 #1612

merged 2 commits into from
Jun 8, 2022

Conversation

fulmicoton
Copy link
Contributor

@fulmicoton fulmicoton commented Jun 7, 2022

Disables http2 to fix bug with gcloud

Google cloud storage allows http2, but hyper's http2 client
does not seem to support it properly, leading to an error.

This PR manually disables http2.

Closes #1584

Google cloud storage allows http2, but hyper's http2 client
does not seem to support it properly, leading to an error.

This PR manually disables http2.

Closes #1584
Copy link
Member

@saroh saroh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!
I can re raise the issue w/ rusoto and probably hyper-rustls or hyper (I haven't validated the presence of the host header tbh) WDYT ?
given hyper-tls is ok w/ http2
https://github.com/rusoto/rusoto/blob/master/rusoto/core/src/lib.rs#L22-L25
https://github.com/rusoto/rusoto/blob/master/rusoto/core/src/request.rs#L220-L260
https://datatracker.ietf.org/doc/html/rfc7540#section-8.1.2.3
hyperium/h2#548 (comment)

@fmassot
Copy link
Contributor

fmassot commented Jun 7, 2022

@saroh that would really nice to open an issue but it would be even better to check that it comes from some header.

@fulmicoton
Copy link
Contributor Author

fulmicoton commented Jun 8, 2022

@saroh yes please! that would be awesome to get that fixed in rusoto. (that's more a rusoto problem than an hyper problem I think. Maybe I misunderstand the problem, but one root is that http2 is kind of leaking and not honoring its promise to work as a drop-in more efficient replacement to http1 from the client's client side.

It expects an Authority header and no Host in request headers. I am not sure what should be the best way for rusoto to deal with the problem tbh... One thing to note is that by default, hyper clients will populate the Host header automatically if it is missing, so your fix might actually be less hacky than it seems.

@fulmicoton
Copy link
Contributor Author

Merging as a termporary fix, but it would be really awesome to get http2 to work eventually.

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 this pull request may close these issues.

Support Google cloud storage.
3 participants