From 10872e1722e86821df301d97da3934da8d8d2ee5 Mon Sep 17 00:00:00 2001 From: Tonis Tiigi Date: Wed, 20 May 2026 12:47:41 -0700 Subject: [PATCH] http: fix resolver cache nil race Avoid storing typed nil HTTP metadata in the resolver cache after failed metadata resolution. Also make resolver cache release functions consistently callable when no error is returned. Add a client integration test covering concurrent same-URL HTTP sources with and without checksum when the server returns 401. Signed-off-by: Tonis Tiigi --- client/client_test.go | 45 +++++++++++++++++++++++++++++++++++++++++ solver/resolvercache.go | 4 +++- source/http/source.go | 4 ++-- 3 files changed, 50 insertions(+), 3 deletions(-) diff --git a/client/client_test.go b/client/client_test.go index a4dc255e8..fde9dd058 100644 --- a/client/client_test.go +++ b/client/client_test.go @@ -116,6 +116,7 @@ var allTests = []func(t *testing.T, sb integration.Sandbox){ testCallDiskUsage, testBuildMultiMount, testBuildHTTPSource, + testBuildHTTPSourceUnauthorizedChecksumRace, testBuildHTTPSourceEtagScope, testBuildHTTPSourceAuthHeaderSecret, testBuildHTTPSourceHostTokenSecret, @@ -3785,6 +3786,50 @@ func testBuildHTTPSource(t *testing.T, sb integration.Sandbox) { // TODO: check that second request was marked as cached } +func testBuildHTTPSourceUnauthorizedChecksumRace(t *testing.T, sb integration.Sandbox) { + integration.SkipOnPlatform(t, "windows") + + c, err := New(sb.Context(), sb.Address()) + require.NoError(t, err) + defer c.Close() + + var requests atomic.Int32 + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if requests.Add(1) == 1 { + time.Sleep(200 * time.Millisecond) + } + w.WriteHeader(http.StatusUnauthorized) + })) + defer server.Close() + + url := server.URL + "/test.txt" + lower := llb.Scratch() + // Reproduce the race between same-URL HTTP sources with and without + // checksum sharing the resolver cache while the server returns 401. + withoutChecksum := lower.File(llb.Copy(llb.HTTP(url), "test.txt", "/a.txt")) + withChecksum := lower.File(llb.Copy(llb.HTTP(url, llb.Checksum(digest.FromBytes(nil))), "test.txt", "/b.txt")) + st := llb.Merge([]llb.State{ + llb.Diff(lower, withoutChecksum), + llb.Diff(lower, withChecksum), + }) + + def, err := st.Marshal(sb.Context()) + require.NoError(t, err) + + for range 10 { + _, err := c.Solve(sb.Context(), def, SolveOpt{ + Exports: []ExportEntry{ + { + Type: ExporterLocal, + OutputDir: t.TempDir(), + }, + }, + }, nil) + require.Error(t, err) + require.ErrorContains(t, err, "invalid response status 401") + } +} + // docker/buildx#2803 func testBuildHTTPSourceEtagScope(t *testing.T, sb integration.Sandbox) { c, err := New(sb.Context(), sb.Address()) diff --git a/solver/resolvercache.go b/solver/resolvercache.go index ef5d9cb87..da298b4d3 100644 --- a/solver/resolvercache.go +++ b/solver/resolvercache.go @@ -127,7 +127,9 @@ func (c *combinedCache) Lock(key any) (values []any, release func(any) error, er mu.Lock() valuesAll = append(valuesAll, vals...) - releasers = append(releasers, rel) + if rel != nil { + releasers = append(releasers, rel) + } mu.Unlock() }(rc) } diff --git a/source/http/source.go b/source/http/source.go index 1282626bc..5dff72b71 100644 --- a/source/http/source.go +++ b/source/http/source.go @@ -866,8 +866,8 @@ func (hs *httpSourceHandler) Snapshot(ctx context.Context, jobCtx solver.JobCont } for _, v := range vals { v2, ok := v.(*metadataWithRef) - if !ok { - return nil, errors.Errorf("invalid HTTP resolver cache value: %T", vals[0]) + if !ok || v2 == nil { + return nil, errors.Errorf("invalid HTTP resolver cache value: %T", v) } if hs.src.Checksum != "" && v2.Digest != hs.src.Checksum { continue