Skip to content

Test ECH + HTTP proxy - #9671

Merged
swankjesse merged 2 commits into
mainfrom
jwilson.0814.ech_proxy
Aug 14, 2026
Merged

Test ECH + HTTP proxy#9671
swankjesse merged 2 commits into
mainfrom
jwilson.0814.ech_proxy

Conversation

@swankjesse

Copy link
Copy Markdown
Collaborator

We had a bug where we incorrectly sent the proxy's ECH config list to the origin server. That wouldn't have worked.

We had a bug where we incorrectly sent the proxy's ECH config list
to the origin server. That wouldn't have worked.
when (proxy.type()) {
Proxy.Type.DIRECT -> serviceMetadata?.echConfigList
else -> null
},

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

This was a pretty nasty one. We sent the proxy’s ECH config list to the origin server

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Good catch.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

This would have manifested as a connectivity problem for clients that use an HTTP proxy, if the HTTP proxy itself has an ECH record. Possibly not likely in practice? Unsure.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Also, I love tests. That’s how we build complex things safely!

when (proxy.type()) {
Proxy.Type.DIRECT -> serviceMetadata?.echConfigList
else -> null
},

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Good catch.

@swankjesse
swankjesse merged commit 5410de9 into main Aug 14, 2026
24 of 25 checks passed
@swankjesse
swankjesse deleted the jwilson.0814.ech_proxy branch August 14, 2026 19:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

2 participants