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

Add property to force backend websocket connections to use HTTP/1.1 #263

Merged
merged 2 commits into from
Oct 21, 2021

Conversation

peterellisjones
Copy link
Contributor

@peterellisjones peterellisjones commented Oct 19, 2021

Based on #261

Gorouter recently added support for HTTP/2. However it turns out to be broken for HTTP/2 Websockets (RFC 8441): cloudfoundry/routing-release#230. It can't immediately be fixed because the bug arises from a problem in Go's net/http: golang/go#32763.

This PR implements a suggestion by @Gerg in #259: forward Websocket traffic to Gorouter over HTTP/1.1, even if HTTP2 is enabled, when the new property disable_backend_http2_websockets is set to true (default false)

@peterellisjones peterellisjones added the run-ci Allow this PR to be tested on Concourse label Oct 19, 2021
@peterellisjones peterellisjones force-pushed the send-websockets-upstream-using-http1.1 branch 4 times, most recently from 9b25197 to e65dc0e Compare October 19, 2021 10:46
@peterellisjones peterellisjones changed the title [WIP] Option to send websockets upstream using HTTP/1.1 Add websockets upstream using HTTP/1.1 Oct 19, 2021
@peterellisjones peterellisjones marked this pull request as ready for review October 19, 2021 10:48
@peterellisjones peterellisjones changed the title Add websockets upstream using HTTP/1.1 Add property to force backend websocket connections to use HTTP/1.1 Oct 19, 2021
@peterellisjones peterellisjones force-pushed the send-websockets-upstream-using-http1.1 branch from bb7a5e4 to 400a1e8 Compare October 19, 2021 12:18
Copy link
Member

@maxmoehl maxmoehl left a comment

Choose a reason for hiding this comment

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

LGTM

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
run-ci Allow this PR to be tested on Concourse
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants