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

reverseproxy: Rewrite requests and responses for websocket over http2 #6567

Merged
merged 11 commits into from
Dec 6, 2024

Conversation

WeidiDeng
Copy link
Member

Supports reverse proxying h2 websockets to backends. Fix 5565.

Requires upstream support, but can be tested with xcaddy build reverse-proxy-h2-websocket --replace golang.org/x/net=github.com/WeidiDeng/net@websocket-http2

@mholt
Copy link
Member

mholt commented Sep 10, 2024

Very neat, nice work @WeidiDeng 💯

@bt90
Copy link
Contributor

bt90 commented Sep 29, 2024

I'm not 100% sure but a regular HTTP/2 GET request should contain the new setting parameter as outlined in RFC8441 Section 3?

[0-0] == Info: [HTTP/2] [0] ingress: read 45 bytes
[0-0] == Info: [HTTP/2] [0] <- FRAME[SETTINGS, len=36]
[0-0] == Info: [HTTP/2] [0] MAX_CONCURRENT_STREAMS: 250
[0-0] == Info: [HTTP/2] [0] ENABLE_PUSH: TRUE
[0-0] == Info: [HTTP/2] [0] notify MAX_CONCURRENT_STREAMS: 250
[0-0] == Info: [HTTP/2] [0] -> FRAME[SETTINGS, len=18]
[0-0] == Info: [HTTP/2] [0] -> FRAME[SETTINGS, ack=1]
[0-0] == Info: [HTTP/2] [0] -> FRAME[WINDOW_UPDATE, incr=1048510465]
[0-0] => Send SSL data, 5 bytes (0x5)
0000: 17 03 03 00 5a                                  ....Z
[0-0] => Send SSL data, 1 bytes (0x1)
0000: 17                                              .
[0-0] == Info: [HTTP/2] [0] egress: wrote 73 bytes
[0-0] == Info: [HTTP/2] cf_connect() -> 0, 1,
[0-0] == Info: using HTTP/2
[0-0] == Info: [HTTP/2] [1] OPENED stream for https://xxxxxxx.xxx/

Tested via curl:

curl --trace request.dump --trace-config http/2 https://example.com

@WeidiDeng
Copy link
Member Author

i'm not sure about curl debug dump. But I tested this with my Chrome and it works. The setting frame is send at the start of a new http2 connection, so it's sent regardless of the method client uses.

Even without my patch, golang http2 sends the following settings:

image

The output doesn't list all of them.

@bt90
Copy link
Contributor

bt90 commented Oct 5, 2024

Successfully tested in Firefox 👍

Bildschirmfoto vom 2024-10-05 11-38-26

@WeidiDeng WeidiDeng marked this pull request as ready for review November 26, 2024 01:06
@WeidiDeng
Copy link
Member Author

Upstream merged in 9a51899. Waiting for the next release.

Copy link
Member

@mholt mholt left a comment

Choose a reason for hiding this comment

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

Thanks for working on this Weidi! Cool feature. I am not sure I 100% fully understand every line of this, but after a scan I don't see any red flags that stick out to me.

@francislavoie francislavoie added upstream ⬆️ Relates to some dependency of this project go Pull requests that update Go code labels Nov 26, 2024
@WeidiDeng
Copy link
Member Author

golang/x/net v0.32.0 is out.

@bt90
Copy link
Contributor

bt90 commented Dec 5, 2024

Is this a simple version bump which could be done in this PR or are there any side effects?

@WeidiDeng
Copy link
Member Author

It's in another pr.

@francislavoie
Copy link
Member

So this is ready to merge @WeidiDeng ?

@francislavoie francislavoie changed the title reverse proxy: rewrite requests and responses for websocket over http2 reverseproxy: Rewrite requests and responses for websocket over http2 Dec 5, 2024
@WeidiDeng
Copy link
Member Author

Yes @francislavoie

Copy link
Member

@mholt mholt left a comment

Choose a reason for hiding this comment

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

Thanks Weidi. Let's give it a shot!

@mholt mholt merged commit 9c0c71e into master Dec 6, 2024
33 checks passed
@mholt mholt deleted the reverse-proxy-h2-websocket branch December 6, 2024 20:23
@JeDaYoshi
Copy link

JeDaYoshi commented Dec 11, 2024

I updated to latest master which included this commit, and it broke WebSocket reverse-proxying towards an HTTP/1.1 backend on Firefox for me.
The connection to wss://[...] was interrupted while the page was loading.

@bt90
Copy link
Contributor

bt90 commented Dec 11, 2024

See #6733

@WeidiDeng
Copy link
Member Author

@JeDaYoshi try xcaddy build encode-connect to see if it's fixed, or use the following request matchers

@not_h2_ws not {
    header :protocol *
    method CONNECT
    protocol http/2
}
encode @not_h2_ws zstd gzip

@JeDaYoshi
Copy link

JeDaYoshi commented Dec 11, 2024

I should check PRs/issues properly before commenting things.. My apologies.

@WeidiDeng encode-connect / #6738 seems to be working for me.

mholt pushed a commit that referenced this pull request Dec 20, 2024
…#6567)

* reverse proxy: rewrite requests and responses for websocket over http2

* delete protocol pseudo-header

* modify cloned requests

* set request variable to track if it's a h2 websocket

* use request bodu

* rewrite request body

* use WebSocket instead of Websocket in the headers

* use logger check for zap loggers

* fix lint
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
go Pull requests that update Go code upstream ⬆️ Relates to some dependency of this project
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Support WebSockets over HTTP/2 (RFC8441)
5 participants