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