diff --git a/internal/api/v1/forwardquery_test.go b/internal/api/v1/forwardquery_test.go new file mode 100644 index 0000000..be758e2 --- /dev/null +++ b/internal/api/v1/forwardquery_test.go @@ -0,0 +1,52 @@ +package v1 + +import ( + "net/url" + "strings" + "testing" + + "git.unkin.net/unkin/artifactapi/internal/provider" +) + +type plainProvider struct{ provider.Provider } + +type forwardingProvider struct{ plainProvider } + +func (forwardingProvider) ForwardedQuery(path string) []string { + if strings.HasSuffix(path, ".tar.gz") { + return nil + } + return []string{"limit", "offset"} +} + +func TestForwardQuery(t *testing.T) { + long := strings.Repeat("9", maxForwardedValueLen+1) + tests := []struct { + name string + prov provider.Provider + path string + query string + want string + }{ + {"non-forwarder keeps path", plainProvider{}, "a/versions/", "limit=10&offset=5", "a/versions/"}, + {"no query", forwardingProvider{}, "a/versions/", "", "a/versions/"}, + {"allowlisted params sorted", forwardingProvider{}, "a/versions/", "offset=5&limit=10", "a/versions/?limit=10&offset=5"}, + {"unknown params dropped", forwardingProvider{}, "a/versions/", "x=1&limit=10&cb=zz", "a/versions/?limit=10"}, + {"only unknown params", forwardingProvider{}, "a/versions/", "x=1", "a/versions/"}, + {"first value wins", forwardingProvider{}, "a/versions/", "limit=10&limit=20", "a/versions/?limit=10"}, + {"values re-encoded", forwardingProvider{}, "a/versions/", "limit=1%202", "a/versions/?limit=1+2"}, + {"overlong value dropped", forwardingProvider{}, "a/versions/", "limit=" + long + "&offset=5", "a/versions/?offset=5"}, + {"path without allowlist", forwardingProvider{}, "a/b-1.0.0.tar.gz", "limit=10", "a/b-1.0.0.tar.gz"}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + q, err := url.ParseQuery(tt.query) + if err != nil { + t.Fatal(err) + } + if got := forwardQuery(tt.prov, tt.path, q); got != tt.want { + t.Errorf("forwardQuery = %q, want %q", got, tt.want) + } + }) + } +} diff --git a/internal/api/v1/proxy.go b/internal/api/v1/proxy.go index 7429007..c64c139 100644 --- a/internal/api/v1/proxy.go +++ b/internal/api/v1/proxy.go @@ -7,6 +7,7 @@ import ( "io" "log/slog" "net/http" + "net/url" "github.com/go-chi/chi/v5" @@ -93,6 +94,8 @@ func (h *ProxyHandler) handleProxy(w http.ResponseWriter, r *http.Request) { } } + path = forwardQuery(prov, path, r.URL.Query()) + result, err := h.engine.Fetch(r.Context(), *remote, path, prov, r.Header) if err != nil { var proxyErr *proxy.ProxyError @@ -258,3 +261,24 @@ func scheme(r *http.Request) string { } return "http" } + +const maxForwardedValueLen = 32 + +// forwardQuery appends the provider's allowlisted query params to path in a +// canonical (sorted, single-valued) form, since the result is the cache key. +func forwardQuery(prov provider.Provider, path string, query url.Values) string { + f, ok := prov.(provider.QueryForwarder) + if !ok { + return path + } + kept := url.Values{} + for _, name := range f.ForwardedQuery(path) { + if v := query.Get(name); v != "" && len(v) <= maxForwardedValueLen { + kept.Set(name, v) + } + } + if len(kept) == 0 { + return path + } + return path + "?" + kept.Encode() +} diff --git a/internal/provider/provider.go b/internal/provider/provider.go index 4c005f3..37afb2d 100644 --- a/internal/provider/provider.go +++ b/internal/provider/provider.go @@ -65,6 +65,13 @@ type MetadataStore interface { InsertDebMetadata(ctx context.Context, meta *DebMetadata) error } +// QueryForwarder is a remote provider whose upstream answers depend on the +// query string (e.g. paginated JSON APIs). Only the named params for path are +// forwarded upstream and kept in the cache key; nil forwards nothing. +type QueryForwarder interface { + ForwardedQuery(path string) []string +} + // RemoteServer lets a remote provider fully answer a request itself instead of // going through the byte-proxy engine. It is the remote-side analog of // LocalIndexer: a metadata-only remote (e.g. github_rpm) uses it to synthesize