From be9b070035a9ea7c382114347eb07bd4e7eae52c Mon Sep 17 00:00:00 2001 From: Bretton Date: Mon, 28 Sep 2026 21:00:13 -0700 Subject: [PATCH] test(imageproxy): make the upstream-failure serving tests reach the fetch Since the handler canonicalizes CIDs and refuses one it cannot decode with 400 before any fetch, TestImageProxy_UndecodableUpstreamBytes ("textdata", "corruptedimage", "emptybody") and the roundtrip "unresolvable blob" case passed on that 400 without the upstream ever being asked. Both now use decodable canonical CIDs, and the undecodable-bytes test asserts that each request reached the upstream blob server before checking the status. Co-Authored-By: Claude Opus 5.5 (1M context) --- .../handlers/imageproxy/proxy_serving_test.go | 42 ++++++++++++++----- .../imageproxy/roundtrip_serving_test.go | 4 +- 2 files changed, 34 insertions(+), 12 deletions(-) diff --git a/internal/api/handlers/imageproxy/proxy_serving_test.go b/internal/api/handlers/imageproxy/proxy_serving_test.go index 7cc6580..f315acd 100644 --- a/internal/api/handlers/imageproxy/proxy_serving_test.go +++ b/internal/api/handlers/imageproxy/proxy_serving_test.go @@ -37,6 +37,7 @@ import ( "net/http" "net/http/httptest" "strings" + "sync" "testing" "time" @@ -360,28 +361,47 @@ func TestImageProxy_UndecodableUpstreamBytes(t *testing.T) { t.Parallel() did := "did:plc:" + testkit.UniqueID(t) - upstream := newBlobServer(t, map[string]func(http.ResponseWriter){ - "textdata": func(w http.ResponseWriter) { + + // Each case needs a decodable CID: the handler refuses one it cannot + // decode with a 400 before any fetch, which would satisfy the assertion + // below without the upstream bytes ever being read. + cases := []struct { + name string + cid string + write func(http.ResponseWriter) + }{ + {"textdata", "bafyreiebpu646nzrrs7utmoucb3c3pe3647so7duktwr4dpqlfkne2pgxa", func(w http.ResponseWriter) { w.Header().Set("Content-Type", "text/plain") w.WriteHeader(http.StatusOK) _, _ = w.Write([]byte("this is not an image")) - }, - "corruptedimage": func(w http.ResponseWriter) { + }}, + {"corruptedimage", "bafyreiemmgzsvld73r3qvp2pw5huuw35b6u4jw2jyonwujf2bmuv7wmc7q", func(w http.ResponseWriter) { w.Header().Set("Content-Type", "image/png") w.WriteHeader(http.StatusOK) // A PNG signature with nothing behind it. _, _ = w.Write([]byte{0x89, 0x50, 0x4E, 0x47, 0x00, 0x00}) - }, - "emptybody": func(w http.ResponseWriter) { + }}, + {"emptybody", "bafyreidunnc32hpxlcbmarbymtp3ag6ztshqosrppgucarhtleb7hxv3nq", func(w http.ResponseWriter) { w.WriteHeader(http.StatusOK) - }, - }) + }}, + } + var fetched sync.Map + blobs := make(map[string]func(http.ResponseWriter), len(cases)) + for _, blobCase := range cases { + blobs[blobCase.cid] = func(w http.ResponseWriter) { + fetched.Store(blobCase.cid, true) + blobCase.write(w) + } + } + upstream := newBlobServer(t, blobs) server := newProxyServer(t, &fixedPDSResolver{pdsURL: upstream.URL}, defaultFetchTimeout) - for _, cid := range []string{"textdata", "corruptedimage", "emptybody"} { - t.Run(cid, func(t *testing.T) { - resp, _ := fetch(t, proxyURL(server, "avatar", did, cid), nil) + for _, blobCase := range cases { + t.Run(blobCase.name, func(t *testing.T) { + resp, _ := fetch(t, proxyURL(server, "avatar", did, blobCase.cid), nil) + _, reached := fetched.Load(blobCase.cid) + require.True(t, reached, "the request must reach the upstream fetch (status %d)", resp.StatusCode) // The exact code differs by failure mode — a sniffable non-image is // a 400, a decoder blowing up mid-stream is a 500 — and pinning // each one would make this brittle about which layer noticed diff --git a/internal/api/handlers/imageproxy/roundtrip_serving_test.go b/internal/api/handlers/imageproxy/roundtrip_serving_test.go index 321f8b8..ae8191d 100644 --- a/internal/api/handlers/imageproxy/roundtrip_serving_test.go +++ b/internal/api/handlers/imageproxy/roundtrip_serving_test.go @@ -198,9 +198,11 @@ func TestImageProxy_EmittedURLsAreFetchable(t *testing.T) { // lifetime on success and sits behind a CDN, so a cacheable failure would // outlive the condition that caused it by a year. t.Run("an unresolvable blob returns an uncacheable error", func(t *testing.T) { + // A decodable CID the PDS does not hold, so the failure is the fetch; + // an undecodable one would be refused before it. missing := blobs.HydrateImageURL(blobs.GetImageURLConfig(), community.PDSURL, community.DID, - "bafkreiabcdefghijklmnopqrstuvwxyz234567abcdefghijklmnopq", "content_preview") + "bafkreial332smgjcus5atd7x52uof527vx4g4dg4yenw3inp3r7t4tod2m", "content_preview") require.NotEmpty(t, missing) resp, _ := fetch(t, missing, nil) -- 2.51.2