Support wildcard destinations with runtime header resolution#551
Support wildcard destinations with runtime header resolution#551
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| r = r.WithContext(contextWithResolvedDestination(r.Context(), resolvedDest)) | ||
| if integ.requiresDestinationHeader { | ||
| r.Header.Del("X-AT-Destination") |
There was a problem hiding this comment.
Removing destination header before auth prevents wildcard signing
The proxy now strips X-AT-Destination before executing the outgoing auth plugins, but the resolved upstream host is only stored in a private context key and is not exposed to those plugins. For integrations that both use wildcard destinations and require auth that depends on the concrete host (e.g. request signing), AddAuth will run with neither the header nor another API to discover the resolved host, so signatures will be calculated against the proxy’s own host and fail once the request is proxied. Consider delaying header removal until after outgoing auth or providing a public way for plugins to read the resolved destination.
Useful? React with 👍 / 👎.
|
https://github.com/codex review |
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| func joinProxyPath(target *url.URL, req *http.Request) (string, string) { | ||
| tgtPath := target.EscapedPath() | ||
| if tgtPath == "" { | ||
| tgtPath = target.Path | ||
| } | ||
| reqPath := req.URL.EscapedPath() | ||
| if reqPath == "" { | ||
| reqPath = req.URL.Path | ||
| } | ||
| path := singleJoiningSlash(tgtPath, reqPath) | ||
| rawPath := path | ||
| if req.URL.RawPath != "" || target.RawPath != "" { | ||
| rawPath = singleJoiningSlash(target.RawPath, req.URL.RawPath) | ||
| } |
There was a problem hiding this comment.
Preserve base path when joining escaped URLs
The new wildcard path rewrites build the upstream path in joinProxyPath, but when the incoming request contains percent‑encoded segments (req.URL.RawPath is non‑empty) and the configured destination path has no RawPath, the code sets rawPath = singleJoiningSlash(target.RawPath, req.URL.RawPath). Because target.RawPath is empty in that case, the returned raw path omits the base path and ends up as just the original request path while req.URL.Path contains the base prefix. When the reverse proxy sends the request it prefers RawPath, so a request like /foo%2Fbar to a destination https://*.example.com/base is forwarded as /foo%2Fbar instead of /base/foo%2Fbar. Prior behaviour using httputil’s join logic kept the encoded and unencoded paths consistent. This breaks wildcard integrations whenever callers use escaped path components. Consider replicating httputil’s joinURLPath logic so RawPath includes the base path too.
Useful? React with 👍 / 👎.
|
@codex review |
|
Codex Review: Didn't find any major issues. Nice work! ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
Summary
X-AT-DestinationheaderTesting
go test ./...Codex Task