From 8dcd194c39c38d78559ec80ffcbc526e05e597bf Mon Sep 17 00:00:00 2001 From: Simon Chrzanowski Date: Fri, 23 Oct 2026 13:22:49 +0200 Subject: [PATCH] Fix npm search routing and abbreviated packuments under cooldown `npm search` requests /-/v1/search?text=... . The path has no "/-/" after the leading slash is trimmed, so it fell through to the package metadata handler: upstream got /-/v1%2Fsearch without the query string, and the answer was cached under "-/v1/search", so every later search was served the first one's results. Search and /-/ping (used by `npm ping`, which lost its query string the same way) are now relayed to upstream uncached with their query strings, like the signing-keys endpoint. Packuments share one metadata cache row whatever format they were fetched in. Without cooldown the proxy asks for the abbreviated format, which has no "time" map. After cooldown was turned on, a still-fresh abbreviated packument kept being served from cache, and used as the stale copy when upstream failed, so cooldown filtered nothing and the tarball check let recent versions through. When full metadata is required (cooldown or npm_full_metadata on), packument lookups now pass a validator that rejects an abbreviated document (top-level "modified", no "time"), so it counts as a cache miss, is refetched in full and is never used as a stale fallback. Behaviour without cooldown is unchanged. fetchMetadataDocument takes the validate callback that fetchOrCacheMetadata already has; other callers pass nil. --- README.md | 1 + docs/configuration.md | 4 +- internal/handler/chef.go | 2 +- internal/handler/composer.go | 4 +- internal/handler/composer_hosted.go | 2 +- internal/handler/handler.go | 7 +- .../handler/metadata_not_modified_test.go | 6 +- internal/handler/npm.go | 69 ++++++++- internal/handler/npm_hosted.go | 2 +- internal/handler/npm_packument_accept_test.go | 132 ++++++++++++++++++ internal/handler/npm_test.go | 69 +++++++++ 11 files changed, 282 insertions(+), 16 deletions(-) diff --git a/README.md b/README.md index 0553e4a1..3433402b 100644 --- a/README.md +++ b/README.md @@ -1207,6 +1207,7 @@ Recently cached: | `GET /npm/*` | npm registry protocol | | `POST /npm/-/npm/v1/security/*` | npm/pnpm/Yarn audit endpoints, passed through to upstream | | `GET /npm/-/npm/v1/keys` | npm registry signing keys, passed through to upstream | +| `GET /npm/-/v1/search`, `GET /npm/-/ping` | `npm search` and `npm ping`, passed through to upstream uncached | | `GET /cargo/*` | Cargo sparse index protocol | | `GET /gem/*` | RubyGems protocol | | `GET /go/*` | Go module proxy protocol | diff --git a/docs/configuration.md b/docs/configuration.md index 4da6f634..5a6f292a 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -353,7 +353,7 @@ upstream: - Composer metadata of routed packages carries an empty `notification-url`, so Composer does not report their installs to the default upstream's `notify-batch` endpoint. - Use `upstream.auth` for the registry's credentials. The proxy has no authentication of its own, so every client that can reach it can download routed packages. - Search (`/composer/search.json`) and the package list (`/composer/packages/list.json`) still come from the default upstream only. -- npm audit and signing-key requests still go to `upstream.npm`. An audit request lists every package in the dependency tree, routed ones included. +- npm audit, signing-key, search and ping requests still go to `upstream.npm`. An audit request lists every package in the dependency tree, routed ones included. - Routes can only be set in the configuration file, not with environment variables. Helm HTTP repositories and additional OCI registries are configured as named maps: @@ -982,6 +982,8 @@ Resolution order: exact package override, then package pattern, then ecosystem o Currently supported for npm, PyPI, pub.dev, Composer, Cargo, NuGet, Conda, RubyGems, and Hex. These ecosystems include publish timestamps in their metadata. +With cooldown on, the proxy asks npm for full packuments, since the abbreviated ones it uses otherwise carry no publish times. Abbreviated packuments cached before cooldown was turned on are fetched again in full rather than served, including as a stale copy when the upstream is down. + Note: Hex cooldown requires disabling registry signature verification since the proxy re-encodes the protobuf payload without the original signature. Set `HEX_NO_VERIFY_REPO_ORIGIN=1` or configure your repo with `no_verify: true`. ## Version denylist diff --git a/internal/handler/chef.go b/internal/handler/chef.go index 764d8a36..cf2002d0 100644 --- a/internal/handler/chef.go +++ b/internal/handler/chef.go @@ -133,7 +133,7 @@ func (h *ChefHandler) serveDocument(w http.ResponseWriter, r *http.Request, docu return } - upstream := h.proxy.fetchMetadataDocument(r.Context(), chefEcosystem, cacheKey, upstreamURL, contentTypeJSON) + upstream := h.proxy.fetchMetadataDocument(r.Context(), chefEcosystem, cacheKey, upstreamURL, contentTypeJSON, nil) if err := upstream.err; err != nil { if errors.Is(err, ErrUpstreamNotFound) { http.Error(w, "not found", http.StatusNotFound) diff --git a/internal/handler/composer.go b/internal/handler/composer.go index 643b287f..223784cb 100644 --- a/internal/handler/composer.go +++ b/internal/handler/composer.go @@ -169,7 +169,7 @@ func (h *ComposerHandler) handlePackageMetadata(w http.ResponseWriter, r *http.R return } - upstream := h.proxy.fetchMetadataDocument(r.Context(), "composer", cacheKey, upstreamURL, contentTypeJSON) + upstream := h.proxy.fetchMetadataDocument(r.Context(), "composer", cacheKey, upstreamURL, contentTypeJSON, nil) if err := upstream.err; err != nil { if errors.Is(err, ErrUpstreamNotFound) { http.Error(w, "not found", http.StatusNotFound) @@ -753,7 +753,7 @@ func (h *ComposerHandler) cachedDist(ctx context.Context, files []composerMetada return composerDist{}, "", false } for _, f := range files { - doc := h.proxy.fetchMetadataDocument(ctx, "composer", f.cacheKey, f.url, contentTypeJSON) + doc := h.proxy.fetchMetadataDocument(ctx, "composer", f.cacheKey, f.url, contentTypeJSON, nil) if doc.err != nil { continue } diff --git a/internal/handler/composer_hosted.go b/internal/handler/composer_hosted.go index 689cf8f9..e94d11cc 100644 --- a/internal/handler/composer_hosted.go +++ b/internal/handler/composer_hosted.go @@ -78,7 +78,7 @@ func pushedComposerVersion(local []composerLocalEntry, version string) bool { // upstream does not know the package, the document holds only the pushed // versions. func (h *ComposerHandler) serveMetadataWithLocal(w http.ResponseWriter, r *http.Request, packageName, upstreamURL, cacheKey string, local []composerLocalEntry) { - upstream := h.proxy.fetchMetadataDocument(r.Context(), "composer", cacheKey, upstreamURL, contentTypeJSON) + upstream := h.proxy.fetchMetadataDocument(r.Context(), "composer", cacheKey, upstreamURL, contentTypeJSON, nil) var out []byte var err error switch { diff --git a/internal/handler/handler.go b/internal/handler/handler.go index 022f3d8b..8bcac24f 100644 --- a/internal/handler/handler.go +++ b/internal/handler/handler.go @@ -1335,9 +1335,10 @@ func (p *Proxy) fetchOrCacheMetadata(ctx context.Context, ecosystem, cacheKey, u // fetchMetadataDocument is FetchOrCacheMetadata for callers that rewrite the // document and need to know which one they got: the result carries its -// digest along with the body. -func (p *Proxy) fetchMetadataDocument(ctx context.Context, ecosystem, cacheKey, upstreamURL, accept string) metadataResult { - return p.lookupMetadata(ctx, ecosystem, cacheKey, upstreamURL, accept, "", nil) +// digest along with the body. validate, which may be nil, is as for +// fetchOrCacheMetadata. +func (p *Proxy) fetchMetadataDocument(ctx context.Context, ecosystem, cacheKey, upstreamURL, accept string, validate func([]byte) error) metadataResult { + return p.lookupMetadata(ctx, ecosystem, cacheKey, upstreamURL, accept, "", validate) } // lookupMetadata implements fetchOrCacheMetadata and fetchMetadataDocument. diff --git a/internal/handler/metadata_not_modified_test.go b/internal/handler/metadata_not_modified_test.go index a16c82ce..fd690dfd 100644 --- a/internal/handler/metadata_not_modified_test.go +++ b/internal/handler/metadata_not_modified_test.go @@ -36,7 +36,7 @@ func TestMetadataNotModifiedKeepsStoredDocument(t *testing.T) { proxy.HTTPClient = upstream.Client() ctx := context.Background() - first := proxy.fetchMetadataDocument(ctx, "test", "pkg", upstream.URL+"/pkg", contentTypeJSON) + first := proxy.fetchMetadataDocument(ctx, "test", "pkg", upstream.URL+"/pkg", contentTypeJSON, nil) if first.err != nil { t.Fatal(first.err) } @@ -46,7 +46,7 @@ func TestMetadataNotModifiedKeepsStoredDocument(t *testing.T) { } for range 2 { - res := proxy.fetchMetadataDocument(ctx, "test", "pkg", upstream.URL+"/pkg", contentTypeJSON) + res := proxy.fetchMetadataDocument(ctx, "test", "pkg", upstream.URL+"/pkg", contentTypeJSON, nil) if res.err != nil || string(res.body) != body || res.digest != first.digest { t.Fatalf("revalidated fetch = %q, digest %q, err %v; want %q, digest %q", res.body, res.digest, res.err, body, first.digest) } @@ -99,7 +99,7 @@ func TestMetadataNotModifiedStoresRowWithoutDigest(t *testing.T) { t.Fatal(err) } - res := proxy.fetchMetadataDocument(context.Background(), "test", "pkg", upstream.URL+"/pkg", contentTypeJSON) + res := proxy.fetchMetadataDocument(context.Background(), "test", "pkg", upstream.URL+"/pkg", contentTypeJSON, nil) if res.err != nil || string(res.body) != `{"v":1}` { t.Fatalf("fetch = %q, %v", res.body, res.err) } diff --git a/internal/handler/npm.go b/internal/handler/npm.go index a874c61c..0e2d8f26 100644 --- a/internal/handler/npm.go +++ b/internal/handler/npm.go @@ -32,6 +32,11 @@ const ( // npmKeysPath serves the registry signing keys `npm audit signatures` reads. npmKeysPath = "/-/npm/v1/keys" + // npmSearchPath is what `npm search` queries, with the terms in the query + // string. npmPingPath is what `npm ping` checks. + npmSearchPath = "/-/v1/search" + npmPingPath = "/-/ping" + // npmSecurityMaxBody caps the audit payload. Buffering it lets an oversized // body be refused before the upstream request starts. npmSecurityMaxBody = 16 << 20 @@ -88,10 +93,21 @@ func (h *NPMHandler) Routes() http.Handler { return } - if r.URL.Path == npmKeysPath { + switch r.URL.Path { + case npmKeysPath: h.proxy.ProxyUpstream(w, r, h.upstreamURL+npmKeysPath, []string{headerAccept, headerAcceptEncoding}) return + case npmSearchPath, npmPingPath: + // Every query has its own answer, so these are relayed uncached + // and keep their query string. + upstreamURL := h.upstreamURL + r.URL.Path + if r.URL.RawQuery != "" { + upstreamURL += "?" + r.URL.RawQuery + } + h.proxy.ProxyUpstream(w, r, upstreamURL, + []string{headerAccept, headerAcceptEncoding}) + return } path := strings.TrimPrefix(r.URL.Path, "/") @@ -224,7 +240,11 @@ func (h *NPMHandler) handlePackageMetadata(w http.ResponseWriter, r *http.Reques return } - upstream := h.proxy.fetchMetadataDocument(r.Context(), "npm", metadataCacheKey, upstreamURL, accept) + var validate func([]byte) error + if !versionLookup { + validate = h.packumentValidator() + } + upstream := h.proxy.fetchMetadataDocument(r.Context(), "npm", metadataCacheKey, upstreamURL, accept, validate) if err := upstream.err; err != nil { if errors.Is(err, ErrUpstreamNotFound) { if versionLookup { @@ -292,11 +312,52 @@ func (h *NPMHandler) packumentAccept() string { return npmAcceptDefault } +// errAbbreviatedPackument rejects an abbreviated packument where the full one +// is needed. +var errAbbreviatedPackument = errors.New("abbreviated packument where full metadata is required") + +// packumentValidator returns the check a packument must pass to be served or +// cached, or nil when any document will do. Packuments share one cache row +// whatever format they were fetched in, so one cached in abbreviated form +// before cooldown or full metadata was turned on would otherwise still be +// served, and used for the cooldown tarball check, while it stayed fresh or +// as a stale copy when upstream failed. It has no "time" map, so cooldown +// would let every version through. Rejecting it makes the lookup fetch the +// full document instead. +func (h *NPMHandler) packumentValidator() func([]byte) error { + if h.packumentAccept() != contentTypeJSON { + return nil + } + return requireFullPackument +} + +// requireFullPackument rejects an abbreviated packument: one with the +// top-level "modified" field the abbreviated format carries and no "time" +// map. A full document without a time map, as some registries serve, is +// let through as before, and so is anything that is not a JSON object, +// leaving it to the usual handling. +func requireFullPackument(body []byte) error { + var hasTime, hasModified bool + err := forEachJSONMember(body, func(m jsonMember) error { + switch key := m.key(body); { + case jsonKeyIs(key, "time"): + hasTime = true + case jsonKeyIs(key, "modified"): + hasModified = true + } + return nil + }) + if err == nil && hasModified && !hasTime { + return errAbbreviatedPackument + } + return nil +} + // handleFilteredNPMVersionMetadata serves /pkg/ and /pkg/ from the // filtered packument so cooldown and dist-tags match the root metadata response. func (h *NPMHandler) handleFilteredNPMVersionMetadata(w http.ResponseWriter, r *http.Request, packageName, specifier, accept, registryURL, cacheKey, metadataCacheKey string) { _, packumentPath := npmMetadataPath(packageName) - packument, _, err := h.proxy.FetchOrCacheMetadata(r.Context(), "npm", cacheKey, registryURL+"/"+packumentPath, accept) + packument, _, _, err := h.proxy.fetchOrCacheMetadata(r.Context(), "npm", cacheKey, registryURL+"/"+packumentPath, "", h.packumentValidator(), accept) if err != nil { if errors.Is(err, ErrUpstreamNotFound) { JSONError(w, http.StatusNotFound, ErrPackageNotFound.Error()) @@ -859,7 +920,7 @@ func (h *NPMHandler) handleDownload(w http.ResponseWriter, r *http.Request) { registryURL, cacheKey := h.sourceFor(packageName) metadata := sync.OnceValues(func() ([]byte, error) { upstreamURL := registryURL + "/" + url.PathEscape(packageName) - body, _, err := h.proxy.FetchOrCacheMetadata(r.Context(), "npm", cacheKey, upstreamURL, h.packumentAccept()) + body, _, _, err := h.proxy.fetchOrCacheMetadata(r.Context(), "npm", cacheKey, upstreamURL, "", h.packumentValidator(), h.packumentAccept()) return body, err }) if h.versionInCooldown(packageName, version, metadata) { diff --git a/internal/handler/npm_hosted.go b/internal/handler/npm_hosted.go index 7532cbdd..4c786e8d 100644 --- a/internal/handler/npm_hosted.go +++ b/internal/handler/npm_hosted.go @@ -98,7 +98,7 @@ func (h *NPMHandler) serveLocalMetadata(w http.ResponseWriter, r *http.Request, // When the upstream does not know the package, the packument holds only the // pushed versions. func (h *NPMHandler) serveMetadataWithLocal(w http.ResponseWriter, r *http.Request, packageName, upstreamURL, accept, metadataCacheKey string, local []npmLocalEntry) { - upstream := h.proxy.fetchMetadataDocument(r.Context(), "npm", metadataCacheKey, upstreamURL, accept) + upstream := h.proxy.fetchMetadataDocument(r.Context(), "npm", metadataCacheKey, upstreamURL, accept, h.packumentValidator()) var out []byte var err error switch { diff --git a/internal/handler/npm_packument_accept_test.go b/internal/handler/npm_packument_accept_test.go index 7158cefa..1a87fdda 100644 --- a/internal/handler/npm_packument_accept_test.go +++ b/internal/handler/npm_packument_accept_test.go @@ -1,15 +1,18 @@ package handler import ( + "encoding/json" "fmt" "io" "net/http" "net/http/httptest" "strings" "sync" + "sync/atomic" "testing" "time" + "github.com/git-pkgs/cooldown" "github.com/git-pkgs/registries/fetch" ) @@ -76,3 +79,132 @@ func TestNPMTarballKeepsCachedPackumentFormat(t *testing.T) { t.Errorf("cached packument Content-Type = %q, want %q", entry.ContentType.String, abbreviated) } } + +// abbreviatedThenFullRegistry serves a packument with a 2.0.0 published an +// hour ago and a 1.0.0 published a month ago. The abbreviated form, like +// registry.npmjs.org's, has no time map. fail makes requests for the full +// document fail. +func abbreviatedThenFullRegistry(t *testing.T, fail *atomic.Bool, fullRequests *atomic.Int64) *httptest.Server { + t.Helper() + const abbreviated = "application/vnd.npm.install-v1+json" + var upstream *httptest.Server + upstream = httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.URL.Path != "/demo" { + w.WriteHeader(http.StatusNotFound) + return + } + versions := fmt.Sprintf(`{"1.0.0":{"name":"demo","version":"1.0.0","dist":{"tarball":%q}},"2.0.0":{"name":"demo","version":"2.0.0","dist":{"tarball":%q}}}`, + upstream.URL+"/demo/-/demo-1.0.0.tgz", upstream.URL+"/demo/-/demo-2.0.0.tgz") + if strings.HasPrefix(r.Header.Get("Accept"), abbreviated) { + w.Header().Set(headerContentType, abbreviated) + _, _ = fmt.Fprintf(w, `{"name":"demo","modified":%q,"dist-tags":{"latest":"2.0.0"},"versions":%s}`, + time.Now().Format(time.RFC3339), versions) + return + } + fullRequests.Add(1) + if fail.Load() { + w.WriteHeader(http.StatusServiceUnavailable) + return + } + w.Header().Set(headerContentType, contentTypeJSON) + _, _ = fmt.Fprintf(w, `{"name":"demo","dist-tags":{"latest":"2.0.0"},"time":{"1.0.0":%q,"2.0.0":%q},"versions":%s}`, + time.Now().Add(-30*24*time.Hour).Format(time.RFC3339), time.Now().Add(-time.Hour).Format(time.RFC3339), versions) + })) + t.Cleanup(upstream.Close) + return upstream +} + +// seedAbbreviatedPackument caches the abbreviated packument for demo through +// the metadata route while cooldown is off, then turns cooldown on. +func seedAbbreviatedPackument(t *testing.T, upstream *httptest.Server) *NPMHandler { + t.Helper() + proxy, db, _, artifactFetcher := setupTestProxy(t) + proxy.CacheMetadata = true + proxy.MetadataTTL = time.Hour + proxy.HTTPClient = upstream.Client() + artifactFetcher.artifact = &fetch.Artifact{Body: io.NopCloser(strings.NewReader("package")), ContentType: "application/gzip"} + h := NewNPMHandler(proxy, "http://proxy.test", upstream.URL) + + if w := serveNPM(t, h, http.MethodGet, "/demo", nil, nil); w.Code != http.StatusOK { + t.Fatalf("seeding metadata: status %d: %s", w.Code, w.Body.String()) + } + entry, err := db.GetMetadataCache("npm", "demo") + if err != nil || entry == nil || entry.ContentType.String != "application/vnd.npm.install-v1+json" { + t.Fatalf("expected an abbreviated packument in the cache, got %+v (%v)", entry, err) + } + + proxy.Cooldown = &cooldown.Config{Default: "7d"} + return h +} + +// A packument cached in abbreviated form before cooldown was turned on has +// no publish times, so cooldown could not filter anything while it stayed +// fresh. With cooldown on it must be refetched in full. +func TestNPMCooldownRefetchesCachedAbbreviatedPackument(t *testing.T) { + var fail atomic.Bool + var fullRequests atomic.Int64 + upstream := abbreviatedThenFullRegistry(t, &fail, &fullRequests) + h := seedAbbreviatedPackument(t, upstream) + + w := serveNPM(t, h, http.MethodGet, "/demo", nil, nil) + if w.Code != http.StatusOK { + t.Fatalf("metadata: status %d: %s", w.Code, w.Body.String()) + } + var doc struct { + DistTags map[string]string `json:"dist-tags"` + Versions map[string]json.RawMessage `json:"versions"` + } + if err := json.Unmarshal(w.Body.Bytes(), &doc); err != nil { + t.Fatalf("decoding metadata: %v", err) + } + if _, ok := doc.Versions["2.0.0"]; ok { + t.Error("version 2.0.0, published an hour ago, was not filtered by cooldown") + } + if _, ok := doc.Versions["1.0.0"]; !ok { + t.Error("version 1.0.0 missing") + } + if doc.DistTags["latest"] != "1.0.0" { + t.Errorf("dist-tags.latest = %q, want 1.0.0", doc.DistTags["latest"]) + } + if fullRequests.Load() == 0 { + t.Error("full packument was never fetched from upstream") + } + + if w := serveNPM(t, h, http.MethodGet, "/demo/-/demo-2.0.0.tgz", nil, nil); w.Code != http.StatusNotFound { + t.Errorf("tarball 2.0.0: status %d, want %d (withheld by cooldown)", w.Code, http.StatusNotFound) + } +} + +// The tarball check alone must not trust the abbreviated packument either. +func TestNPMCooldownTarballRefetchesCachedAbbreviatedPackument(t *testing.T) { + var fail atomic.Bool + var fullRequests atomic.Int64 + upstream := abbreviatedThenFullRegistry(t, &fail, &fullRequests) + h := seedAbbreviatedPackument(t, upstream) + + if w := serveNPM(t, h, http.MethodGet, "/demo/-/demo-2.0.0.tgz", nil, nil); w.Code != http.StatusNotFound { + t.Errorf("tarball 2.0.0: status %d, want %d (withheld by cooldown)", w.Code, http.StatusNotFound) + } + if fullRequests.Load() == 0 { + t.Error("full packument was never fetched from upstream") + } +} + +// When upstream cannot supply the full packument, the abbreviated one must not +// stand in for it as a stale copy: it would let every version through. +func TestNPMCooldownSkipsAbbreviatedStaleFallback(t *testing.T) { + var fail atomic.Bool + var fullRequests atomic.Int64 + upstream := abbreviatedThenFullRegistry(t, &fail, &fullRequests) + h := seedAbbreviatedPackument(t, upstream) + h.proxy.MetadataTTL = time.Nanosecond // force a refetch + fail.Store(true) + + w := serveNPM(t, h, http.MethodGet, "/demo", nil, nil) + if w.Code != http.StatusBadGateway { + t.Errorf("metadata: status %d, want %d; body: %s", w.Code, http.StatusBadGateway, w.Body.String()) + } + if strings.Contains(w.Body.String(), "2.0.0") { + t.Errorf("served the abbreviated packument as a stale copy: %s", w.Body.String()) + } +} diff --git a/internal/handler/npm_test.go b/internal/handler/npm_test.go index c8a4eb3e..23e223fb 100644 --- a/internal/handler/npm_test.go +++ b/internal/handler/npm_test.go @@ -1328,3 +1328,72 @@ func TestNPMVersionMetadataDenied(t *testing.T) { t.Fatalf("status = %d, want 403; body: %s", w.Code, w.Body.String()) } } + +// `npm search` asks for /-/v1/search?text=.... The path used to fall through to +// the package dispatch, which dropped the query string, escaped the path into +// a package name and cached the answer, so every later search got the first +// one's results. +func TestNPMSearchRelaysQueryUncached(t *testing.T) { + var ( + requests int + gotPaths []string + ) + upstream := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + requests++ + gotPaths = append(gotPaths, r.URL.EscapedPath()) + if r.URL.EscapedPath() != "/-/v1/search" { + w.WriteHeader(http.StatusNotFound) + return + } + w.Header().Set("Content-Type", "application/json") + _, _ = fmt.Fprintf(w, `{"objects":[],"total":0,"query":%q}`, r.URL.RawQuery) + })) + defer upstream.Close() + + proxy, db, _, _ := setupTestProxy(t) + proxy.CacheMetadata = true + proxy.MetadataTTL = time.Hour + proxy.HTTPClient = upstream.Client() + h := NewNPMHandler(proxy, "http://proxy.test", upstream.URL) + + for _, query := range []string{"text=foo&size=20", "text=bar&size=20", "text=foo&size=20"} { + w := serveNPM(t, h, http.MethodGet, "/-/v1/search?"+query, nil, nil) + if w.Code != http.StatusOK { + t.Fatalf("search %q: status = %d; body: %s", query, w.Code, w.Body.String()) + } + if want := fmt.Sprintf(`{"objects":[],"total":0,"query":%q}`, query); w.Body.String() != want { + t.Errorf("search %q: body = %s, want %s", query, w.Body.String(), want) + } + } + if requests != 3 { + t.Errorf("upstream requests = %d (%q), want 3: searches must not be cached", requests, gotPaths) + } + for _, key := range []string{"-/v1/search", "-", "-/v1"} { + if entry, _ := db.GetMetadataCache("npm", key); entry != nil { + t.Errorf("search response cached under %q", key) + } + } +} + +// `npm ping` reads /-/ping, which used to be looked up as a package. +func TestNPMPingRelaysUpstream(t *testing.T) { + var gotPath, gotQuery string + upstream := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + gotPath, gotQuery = r.URL.EscapedPath(), r.URL.RawQuery + w.Header().Set("Content-Type", "application/json") + _, _ = io.WriteString(w, "{}") + })) + defer upstream.Close() + + proxy, _, _, _ := setupTestProxy(t) + proxy.HTTPClient = upstream.Client() + h := NewNPMHandler(proxy, "http://proxy.test", upstream.URL) + + w := serveNPM(t, h, http.MethodGet, "/-/ping?write=true", nil, nil) + if w.Code != http.StatusOK || w.Body.String() != "{}" { + t.Fatalf("ping = %d %q, want 200 {}", w.Code, w.Body.String()) + } + if gotPath != "/-/ping" || gotQuery != "write=true" { + t.Errorf("upstream request = %q?%q, want /-/ping?write=true", gotPath, gotQuery) + } +}