Skip to content

reverse proxy: validate versions in http transport#7112

Merged
mholt merged 1 commit intomasterfrom
reverse-proxy-validate-http-version
Jul 9, 2025
Merged

reverse proxy: validate versions in http transport#7112
mholt merged 1 commit intomasterfrom
reverse-proxy-validate-http-version

Conversation

@WeidiDeng
Copy link
Copy Markdown
Member

Fix 7111

@francislavoie
Copy link
Copy Markdown
Member

Validation should probably happen in Validate, not provision. But also should happen in Caddyfile unmarshaling (earlier than both).

Copy link
Copy Markdown
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!

I somehow totally missed that.

I guess the incongruency with "h2c" is a bit annoying, since HTTP/2 over Cleartext is called "h2c" but all the others are just straight version numbers, not "h" whatever.

Thank you again for this patch :)


var (
allowedVersions = []string{"1.1", "2", "h2c", "3"}
allowedVersionsString = strings.Join(allowedVersions, ", ")
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Nit: This could just be inlined below with the error message, since it won't be happening THAT often, in fact quite rarely; but, I'm okay with it either way. So you don't have to change it.

@mholt
Copy link
Copy Markdown
Member

mholt commented Jul 8, 2025

@francislavoie

Validation should probably happen in Validate, not provision. But also should happen in Caddyfile unmarshaling (earlier than both).

You know I thought about this too before I saw your comment (which was just now, for some reason) -- and I agree... it would make sense. Although -- and I haven't checked yet -- but if there are places where we are manually calling Provision() on the module, we would also need to call Validate in those places.

(In hindsight, I think separating Provision/Validate in the v2 design was not especially helpful like I thought it would be.)

@mholt mholt merged commit 1209b5c into master Jul 9, 2025
26 checks passed
@mholt mholt deleted the reverse-proxy-validate-http-version branch July 9, 2025 20:13
qdongxu pushed a commit to qdongxu/caddy that referenced this pull request Jul 13, 2025
@francislavoie francislavoie added this to the v2.10.1 milestone Aug 22, 2025
mohammed90 pushed a commit to cedricziel/caddy that referenced this pull request Aug 29, 2025
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.

reverse proxy: http transport no error emitted when invalid http versions are provided

3 participants