feat: add HTTP proxy strategy for caching absolute-form proxy requests#176
Conversation
joshfriend
requested review from
jrobotham-square
and removed request for
a team
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c025b19cea
ℹ️ 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".
| logger := logging.FromContext(ctx) | ||
| p := &HTTPProxy{logger: logger} | ||
|
|
||
| mux.Handle("/", p.newHandler(c)) |
There was a problem hiding this comment.
Registering the proxy as "/" lets more-specific handlers win first, and I verified this repo already registers GET /api/v1/... (internal/strategy/apiv1.go) plus /admin/... routes (cmd/cachewd/main.go). For absolute-form proxy traffic, ServeMux routing is still path-based, so a request like GET http://upstream/api/v1/object/ns/key is routed to cachew’s local API handler instead of being proxied upstream. That breaks proxied downloads for overlapping paths and can hit local control routes unexpectedly; absolute-form detection needs to happen before normal ServeMux path dispatch (or on a separate listener/host pattern).
Useful? React with 👍 / 👎.
| if target == nil { | ||
| return r, nil | ||
| } | ||
| return http.NewRequestWithContext(r.Context(), http.MethodGet, target.String(), nil) |
There was a problem hiding this comment.
The transform unconditionally rewrites upstream calls to GET, but this handler is mounted on "/" for all methods. Any absolute-form HEAD, POST, PUT, etc. request is therefore silently converted into a GET, which returns incorrect semantics and can cause clients to misbehave (for example, HEAD probes downloading content or POST never reaching upstream). Either constrain this strategy to GET/HEAD at routing time or forward method/body/headers as-is.
Useful? React with 👍 / 👎.
Adds a new `proxy {}` strategy that acts as a caching HTTP proxy for
clients that route traffic via --proxy_host / --proxy_port flags (e.g.
Android sdkmanager). When a client sends an absolute-form request
(GET http://host/path HTTP/1.1), the strategy upgrades the upstream
fetch to HTTPS, caches the response, and serves subsequent requests
from cache.
Registers a "/" catch-all handler that detects proxy-form requests by
inspecting r.RequestURI. Non-proxy requests (relative paths) fall
through to a 404. The strategy is namespaced in the cache under "proxy"
and is separate from the hermit strategy, which handles hermit package
manager downloads specifically.
joshfriend
force-pushed
the
feat/http-proxy-strategy
branch
from
c025b19 to
a6c74eb
Compare
#176) Adds a new `proxy {}` strategy that acts as a caching HTTP proxy for clients that route downloads through a standard HTTP proxy via `--proxy_host` / `--proxy_port` flags. A common example is Android `sdkmanager`, which supports `--no_https --proxy=http --proxy_host=<host> --proxy_port=<port>` to route all package and repository manifest fetches through a proxy. When a client sends an absolute-form proxy request (`GET http://host/path HTTP/1.1`), the strategy upgrades the upstream fetch to HTTPS, caches the response, and serves subsequent requests from the cache. Non-proxy requests (relative paths) are passed through as 404s so the handler does not interfere with other routes. The strategy is registered under the `proxy` namespace in the cache, keeping it separate from the `hermit` strategy which handles hermit package manager downloads. It registers a `"/"` catch-all on the mux and detects proxy-form requests by inspecting `r.RequestURI`.
2 participants
Adds a new
proxy {}strategy that acts as a caching HTTP proxy for clients that route downloads through a standard HTTP proxy via--proxy_host/--proxy_portflags. A common example is Androidsdkmanager, which supports--no_https --proxy=http --proxy_host=<host> --proxy_port=<port>to route all package and repository manifest fetches through a proxy.When a client sends an absolute-form proxy request (
GET http://host/path HTTP/1.1), the strategy upgrades the upstream fetch to HTTPS, caches the response, and serves subsequent requests from the cache. Non-proxy requests (relative paths) are passed through as 404s so the handler does not interfere with other routes.The strategy is registered under the
proxynamespace in the cache, keeping it separate from thehermitstrategy which handles hermit package manager downloads. It registers a"/"catch-all on the mux and detects proxy-form requests by inspectingr.RequestURI.