-
Notifications
You must be signed in to change notification settings - Fork 46
Add H2C support behind H2C_ENABLED feature flag #89
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
Conversation
|
I have this disabled by default, but I wonder if it's actually safe by default to leave enabled. I know #72 shared the blog post about h2c smuggling that can happen as a result of an h2c upgrade, but now that a protocol upgrade is deprecated, not implemented (and has no plans to be added), maybe it's safe to leave this enabled by default? Example: In the output you can see there is no upgrade since http2 is assumed. Verbose Output* Host localhost:4000 was resolved.
* IPv6: ::1
* IPv4: 127.0.0.1
% Total % Received % Xferd Average Speed Time Time Time Current
Dload Upload Total Spent Left Speed
0 0 0 0 0 0 0 0 --:--:-- --:--:-- --:--:-- 0* Trying [::1]:4000...
* Connected to localhost (::1) port 4000
* [HTTP/2] [1] OPENED stream for http://localhost:4000/sessions/new
* [HTTP/2] [1] [:method: GET]
* [HTTP/2] [1] [:scheme: http]
* [HTTP/2] [1] [:authority: localhost:4000]
* [HTTP/2] [1] [:path: /sessions/new]
* [HTTP/2] [1] [user-agent: curl/8.7.1]
* [HTTP/2] [1] [accept: */*]
> GET /sessions/new HTTP/2
> Host: localhost:4000
> User-Agent: curl/8.7.1
> Accept: */*
>
* Request completely sent off
< HTTP/2 404
< content-type: text/html; charset=UTF-8
< server-timing: render_partial.action_view;dur=0.67, render_collection.action_view;dur=4.60, render_template.action_view;dur=10.99, render_layout.action_view;dur=8.97
< vary: Accept-Encoding
< x-cache: miss
< x-request-id: 4841e945-8c91-4a10-a3a8-f7fbba54f359
< x-runtime: 0.054539
< x-web-console-mount-point: /__web_console
< x-web-console-session-id: 52411745e2db02badf1bae152343add9
< content-length: 116338
< date: Wed, 03 Sep 2025 19:59:42 GMT
<
{ [4096 bytes data]
HTTP/2 404
content-type: text/html; charset=UTF-8
server-timing: render_partial.action_view;dur=0.67, render_collection.action_view;dur=4.60, render_template.action_view;dur=10.99, render_layout.action_view;dur=8.97
vary: Accept-Encoding
x-cache: miss
x-request-id: 4841e945-8c91-4a10-a3a8-f7fbba54f359
x-runtime: 0.054539
x-web-console-mount-point: /__web_console
x-web-console-session-id: 52411745e2db02badf1bae152343add9
content-length: 116338
date: Wed, 03 Sep 2025 19:59:42 GMT
Unless there is a confident "yes" to enabling by default, I would prefer to have this released soon as I could really use it right away. No issues with leaving this disabled by default while we learn more. |
|
Sorry for a 3rd comment. Forgot to ping @kevinmcconnell for review. |
This adds support for http/2 cleartext (h2c) where prior knowledge is known about the server so an upgrade request (where potential memory consumption issues are not possible). To quote [this issues](golang/go#72039): > Go 1.24's net/http package includes support for HTTP/2 with prior knowledge. It does not include support for the deprecated protocol upgrade mechanism, and we have no plans to add such support. Good news! This change is useful for those who run rails apps behind Cloud Run (or AWS) where some limits may apply to apps (like max file size) unless you run http/2 and enable h2c in the cloud provider settings. Snippet taken from: basecamp#72 Applied feedback from: basecamp#3
| "cache", cache, | ||
| "query", r.URL.RawQuery) | ||
| "query", r.URL.RawQuery, | ||
| "proto", r.Proto) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've been testing this in cloud run to make sure everything works as expected and one item missing for me was seeing the protocol used for the request.
With this I was able to test the following:
- By default, cloud run will use http/1.1.
- With cloud run's
Use HTTP/2 end-to-endenabled, it will use http/2 and fail without h2c enabled in thruster. - With
H2C_ENABLED=true DEBUG=trueI see the "Enabling h2c" log, cloud run use http/2, and requests are proxied successfully.
Nice!
|
Thanks @film42 for doing this. I was also though to add this, but you already did greate job. Lets add tests for new config variable in https://github.com/basecamp/thruster/blob/main/internal/config_test.go and add tests for h2c in https://github.com/basecamp/thruster/blob/main/internal/handler_test.go (this example generated by LLM, so may not work) // TestHandler_H2C tests the handler with an H2C (HTTP/2 Cleartext) server.
// This ensures that the application handler is compatible with an H2C connection.
func TestHandler_H2C(t *testing.T) {
// 1. Create the application handler we want to test.
appHandler := NewHandler()
// 2. Wrap the application handler with the h2c handler.
h2cHandler := h2c.NewHandler(appHandler, &http2.Server{})
// 3. Start a test server with the h2c handler.
// httptest.NewServer handles creating a listener on a random free port
// and shutting down the server cleanly after the test.
server := httptest.NewServer(h2cHandler)
defer server.Close()
// 4. Create a special HTTP/2 client that can perform h2c (unencrypted HTTP/2).
// We configure its transport to dial our test server directly without TLS.
client := http.Client{
Transport: &http2.Transport{
// Allow non-encrypted HTTP/2 connections.
AllowHTTP: true,
// The DialTLS function is used to establish the connection.
// We override it to dial our test server's plain TCP listener.
DialTLS: func(network, addr string, cfg *tls.Config) (net.Conn, error) {
return net.Dial(network, server.Listener.Addr().String())
},
},
}
// 5. Make a request to the /up endpoint using the h2c client.
resp, err := client.Get(server.URL + "/up")
if !assert.NoError(t, err, "request should not fail") {
t.FailNow()
}
defer resp.Body.Close()
// 6. Assert that the response was successful and delivered over HTTP/2.
assert.Equal(t, http.StatusOK, resp.StatusCode, "should return status OK")
assert.Equal(t, "HTTP/2.0", resp.Proto, "protocol should be HTTP/2.0")
// 7. Assert the response body is correct.
body, err := io.ReadAll(resp.Body)
assert.NoError(t, err, "reading body should not fail")
assert.Equal(t, "OK", string(body), "body should be OK")
}If you need help with tests - just write here, I will try to help P.S. I also using GCP Cloud Run and want to test it here too |
In order to do this with public functions, we'll need to refactor the way server is initialized. This isn't a hard thing to do, but I first want feedback from the maintainers to see if there is appetite to do so.
|
Thanks @film42 . Ping @kevinmcconnell . Please check this stuff, when you will have time. If it will be merged, it also will resolve #72 and #3 I will be first in queue to test this on production after thruster new version release. Thanks |
|
Thanks @le0pard . Since this is added at the server
Btw, I have two apps running in prod with test builds and they are both working great. |
|
Update here. I ended up disabling h2c for the time being because when action cable tries to connect, I see a 503 in google cloud. More to unpack here but this is a pretty big edge case that needs addressing. |
|
Yep, that is why it is better disabled by default. So who needs - activate it |
|
Agreed. I have one app that doesn't use action cable and I will leave this enabled for that one. Action cable is the only "gotcha" I've come across so far. |
|
Ping @kevinmcconnell , this should resolve also #3 🙇 |
|
https://github.com/le0pard/thruster_plus - resolved for now by fork (
|

This adds support for http/2 cleartext (h2c) where prior knowledge is known about the server so an upgrade request (where potential memory consumption issues are not possible).
To quote this issues:
Good news!
This change is useful for those who run rails apps behind Cloud Run (or AWS) where some limits may apply to apps (like max file size) unless you run http/2 and enable h2c in the cloud provider settings.
Snippet taken from: #72
Applied feedback from: #3