Forward allowlisted query params for opt-in remote providers

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)
+			}
+		})
+	}
+}

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()
+}

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