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

reverse_proxy with h2c http transport not working when client requests using http1.1 #4777

Closed
linaGirl opened this issue May 9, 2022 · 6 comments · Fixed by #4778
Closed
Assignees
Labels
feature ⚙️ New feature or request

Comments

@linaGirl
Copy link

linaGirl commented May 9, 2022

Caddy version: v2.5.1

I'm trying to send incoming requests to a h2c upstream server. This doesn't work if the client is using HTTP1.1 only. Caddy sends a HTTP1.1 request to the upstream server while it should use h2c. Did i miss something about how this is supposed to work or is the config not correct?

Caddyfile

localhost {
    reverse_proxy {
        transport http {
            versions h2c
        }
        to 127.0.0.1:8988
        # doesn't work either: reverse_proxy h2c://127.0.0.1:8988
    }
}
curl -v --http1.1 https://localhost/some/url

Logs

2022/05/09 13:39:55.692	DEBUG	tls.handshake	choosing certificate	{"identifier": "localhost", "num_choices": 1}
2022/05/09 13:39:55.692	DEBUG	tls.handshake	default certificate selection results	{"identifier": "localhost", "subjects": ["localhost"], "managed": true, "issuer_key": "local", "hash": "3b1a3eb05354d36a48042d511a80e9106bc7a9415660914f9c9476f85d8889e5"}
2022/05/09 13:39:55.692	DEBUG	tls.handshake	matched certificate in cache	{"subjects": ["localhost"], "managed": true, "expiration": "2022/05/10 01:07:01.000", "hash": "3b1a3eb05354d36a48042d511a80e9106bc7a9415660914f9c9476f85d8889e5"}
2022/05/09 13:39:55.693	DEBUG	http.handlers.reverse_proxy	selected upstream	{"dial": "127.0.0.1:8988", "total_upstreams": 1}
2022/05/09 13:39:55.694	DEBUG	http.handlers.reverse_proxy	upstream roundtrip	{"upstream": "127.0.0.1:8988", "duration": 0.001156052, "request": {"remote_ip": "127.0.0.1", "remote_port": "53180", "proto": "HTTP/1.1", "method": "GET", "host": "localhost", "uri": "/some/url", "headers": {"User-Agent": ["curl/7.68.0"], "Accept": ["*/*"], "X-Forwarded-For": ["127.0.0.1"], "X-Forwarded-Proto": ["https"], "X-Forwarded-Host": ["localhost"]}, "tls": {"resumed": false, "version": 772, "cipher_suite": 4865, "proto": "http/1.1", "server_name": "localhost"}}, "error": "net/http: HTTP/1.x transport connection broken: malformed HTTP response \"\\x00\\x00\\x00\\x04\\x00\\x00\\x00\\x00\\x00\\x00\\x00\\b\\a\\x00\\x00\\x00\\x00\\x00\\x00\\x00\\x00\\x00\\x00\\x00\\x00\\x02\""}
2022/05/09 13:39:55.694	ERROR	http.log.error.log0	net/http: HTTP/1.x transport connection broken: malformed HTTP response "\x00\x00\x00\x04\x00\x00\x00\x00\x00\x00\x00\b\a\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x02"	{"request": {"remote_ip": "127.0.0.1", "remote_port": "53180", "proto": "HTTP/1.1", "method": "GET", "host": "localhost", "uri": "/some/url", "headers": {"User-Agent": ["curl/7.68.0"], "Accept": ["*/*"]}, "tls": {"resumed": false, "version": 772, "cipher_suite": 4865, "proto": "http/1.1", "server_name": "localhost"}}, "duration": 0.001299073, "status": 502, "err_id": "5f3h3tzr5", "err_trace": "reverseproxy.statusError (reverseproxy.go:1196)"}
2022/05/09 13:39:55.694	ERROR	http.log.access.log0	handled request	{"request": {"remote_ip": "127.0.0.1", "remote_port": "53180", "proto": "HTTP/1.1", "method": "GET", "host": "localhost", "uri": "/some/url", "headers": {"Accept": ["*/*"], "User-Agent": ["curl/7.68.0"]}, "tls": {"resumed": false, "version": 772, "cipher_suite": 4865, "proto": "http/1.1", "server_name": "localhost"}}, "user_id": "", "duration": 0.001299073, "size": 0, "status": 502, "resp_headers": {"Server": ["Caddy"]}}

@mholt
Copy link
Member

mholt commented May 9, 2022

I think the incoming/original request protocol also has to be HTTP/2. I'm not sure we've had a request yet for it to support HTTP/1.1 downstream and upconvert to 2 going upstream. Can you try with an incoming HTTP/2 request and make sure it works? If so, I can prepare a branch I would like you to try that could solve it.

@mholt mholt added the bug 🐞 Something isn't working label May 9, 2022
@mholt mholt self-assigned this May 9, 2022
@linaGirl
Copy link
Author

linaGirl commented May 9, 2022

When requesting with http2 it works flawlessly. It would be extremely awesome if the conversion from http1.1 to http2 for upstream would work. Thanks so much!

@mholt mholt added feature ⚙️ New feature or request and removed bug 🐞 Something isn't working labels May 9, 2022
@mholt
Copy link
Member

mholt commented May 9, 2022

@linaGirl Thanks for checking. Can you please try the branch from PR #4778 and see how that works?

Keep in mind that the incoming request also needs to be cleartext HTTP.

@mholt
Copy link
Member

mholt commented May 9, 2022

@linaGirl Also, what is your use case? We think that's important when weighing decisions about features. We haven't heard of anyone needing to upconvert HTTP/1.1 to H2C before.

@linaGirl
Copy link
Author

linaGirl commented May 9, 2022

Thank you so much @mholt I will test PR #4778 tomorrow. TLS needs to be supported for the incoming requests though.

The use case is: there is a service mesh for a web app that communicates using h2c. There are some public h2c services that need to be accessed by browsers. There is thus the need to convert incoming http1.1 and h2 requests to h2c. This was done using envoy in the past. We decided to switch to caddy because of the very straightforward and easy to maintain config and the automatic certificate generation.

Since caddy provides the option to configure the http transport for the revers_proxy we were thinking that any incoming request protocol could be forwarded to whatever transport is configured.

but i can see that h2c may be a bit of an edgecase ...

@linaGirl
Copy link
Author

@mholt PR #4778 works like a charm and is deployed to production already. Works also when the incoming request has TLS enabled. Thank you so much!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature ⚙️ New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants