Skip to content

Comments

Force HTTP 1.1 to avoid websocket incompatibility#1364

Merged
ahl merged 1 commit intomainfrom
force-http-1.1
Feb 20, 2026
Merged

Force HTTP 1.1 to avoid websocket incompatibility#1364
ahl merged 1 commit intomainfrom
force-http-1.1

Conversation

@ahl
Copy link
Collaborator

@ahl ahl commented Feb 20, 2026

See #1352. I don't want to claim that it fixes the issue, because I don't fully understand it yet; but in all likelihood this will suffice.

@ahl ahl requested review from hawkw and lifning February 20, 2026 01:43
@lifning
Copy link
Contributor

lifning commented Feb 20, 2026

as per the comment, we certainly don't get much out of HTTP/2 in this case, so this makes sense as a workaround.

for whatever this is worth, from websocket's perspective, RFC 6455's only mentions of HTTP 1.1 are accompanied by the phrase "at least" (or in examples, along with "non-normative"), so it's strange that something is throwing this back at us being that 2 > 1.1...

but also, the reqwest method is http1_only, whose documented purpose is "Only use HTTP/1.", whose omission of one extra significant figure makes a pedantic part of me slightly nervous from an API-contract perspective.

but as long reqwest is implementing that with 1.1 (and it is), then this should suffice for our immediate purposes

@david-crespo
Copy link
Contributor

I wonder if we could run into this on web. We’ve had issues before, I think with image upload speed, that were due to the browser using HTTP/2.

@ahl
Copy link
Collaborator Author

ahl commented Feb 20, 2026

Here's what I found here:

Warning: HTTP/2 explicitly disallows the use of this mechanism and header; it is specific to HTTP/1.1.

@ahl
Copy link
Collaborator Author

ahl commented Feb 20, 2026

I wonder if we could run into this on web. We’ve had issues before, I think with image upload speed, that were due to the browser using HTTP/2.

Seems possible. I'm still not clear yet on what exactly changed that triggered this behavior change. It might be as simple as reqwest getting more pedantic about the types of headers it's willing to send, or it might be that something in the hyper/reqwest stack stripped out the header or modified how protocol negotiation happens. I tried to reproduce this locally and wasn't able to--it might be that there's some proxy in that that's stripping out the headers for HTTP/2. Anyhow: more mysteries to investigate.

@ahl ahl merged commit df67310 into main Feb 20, 2026
17 checks passed
@ahl ahl deleted the force-http-1.1 branch February 20, 2026 05:12
@sunshowers
Copy link

As part of oxidecomputer/omicron#9824, did some investigation here (thanks Claude).

It looks like the root cause is seanmonstar/reqwest#2902 / seanmonstar/reqwest#2907, which turns on ALPN. TLS with ALPN is required for HTTP/2 protocol negotiation in reqwest (see connect.rs which only calls negotiated_h2 in paths with TLS + ALPN).

There is RFC 8441 which allows WebSockets upgrades to occur with HTTP/2, but that isn't implemented in reqwest.

For our internal APIs, the options are:

  1. Create either every client or just WebSockets-related clients with http1_only (the approach followed in this PR).
  2. Disable HTTP/2 on the server.
  3. Ship all clients with the reqwest native-tls-vendored-no-alpn feature, which will effectively cause HTTP/2 to be disabled.
  4. Don't worry about it since we don't use TLS for our internal APIs today.

Out of these, for our internal APIs, 4 is the most appealing to me. Maybe 2 as a second choice.

@ahl
Copy link
Collaborator Author

ahl commented Feb 24, 2026

Agreed: 4 until slightly too late and then 2.

@davepacheco
Copy link

@sunshowers does that analysis explain why @ahl couldn't reproduce locally?

@ahl
Copy link
Collaborator Author

ahl commented Feb 24, 2026

@sunshowers does that analysis explain why @ahl couldn't reproduce locally?

We think it may be because I was not using SSL locally

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.

5 participants