From 06ff84125d87dbd7850a230cadcb5a9c1f7eca35 Mon Sep 17 00:00:00 2001
From: Tonis Tiigi <tonistiigi@gmail.com>
Date: Mon, 25 Nov 2024 09:31:11 -0800
Subject: [PATCH] http: fix etag cache scoping

Currently the lookup from previous local etags was based on
filename. This leads to possibility where (misbehaving) server
may reuse the same eTag for different URLs. While using only
the filename might theoretically create more cache matches when
the same file is used via multiple URLs, I think was accidental
mistake and not intentional.

Signed-off-by: Tonis Tiigi <tonistiigi@gmail.com>
---
 client/client_test.go | 99 +++++++++++++++++++++++++++++++++++++++++++
 source/http/source.go | 14 +++---
 2 files changed, 108 insertions(+), 5 deletions(-)

diff --git a/client/client_test.go b/client/client_test.go
index f345cb2446cb..e7a5886f8eec 100644
--- a/client/client_test.go
+++ b/client/client_test.go
@@ -104,6 +104,7 @@ var allTests = []func(t *testing.T, sb integration.Sandbox){
 	testCallDiskUsage,
 	testBuildMultiMount,
 	testBuildHTTPSource,
+	testBuildHTTPSourceEtagScope,
 	testBuildPushAndValidate,
 	testBuildExportWithUncompressed,
 	testBuildExportScratch,
@@ -2769,6 +2770,104 @@ func testBuildHTTPSource(t *testing.T, sb integration.Sandbox) {
 	// TODO: check that second request was marked as cached
 }
 
+// docker/buildx#2803
+func testBuildHTTPSourceEtagScope(t *testing.T, sb integration.Sandbox) {
+	integration.SkipOnPlatform(t, "windows")
+	c, err := New(sb.Context(), sb.Address())
+	require.NoError(t, err)
+	defer c.Close()
+
+	modTime := time.Now().Add(-24 * time.Hour) // avoid falso positive with current time
+
+	sharedEtag := identity.NewID()
+	resp := httpserver.Response{
+		Etag:         sharedEtag,
+		Content:      []byte("content1"),
+		LastModified: &modTime,
+	}
+	resp2 := httpserver.Response{
+		Etag:         sharedEtag,
+		Content:      []byte("another"),
+		LastModified: &modTime,
+	}
+
+	server := httpserver.NewTestServer(map[string]httpserver.Response{
+		"/one/foo": resp,
+		"/two/foo": resp2,
+	})
+	defer server.Close()
+
+	// first correct request
+	st := llb.HTTP(server.URL + "/one/foo")
+
+	def, err := st.Marshal(sb.Context())
+	require.NoError(t, err)
+
+	out1 := t.TempDir()
+	_, err = c.Solve(sb.Context(), def, SolveOpt{
+		Exports: []ExportEntry{
+			{
+				Type:      ExporterLocal,
+				OutputDir: out1,
+			},
+		},
+	}, nil)
+	require.NoError(t, err)
+
+	require.Equal(t, 1, server.Stats("/one/foo").AllRequests)
+	require.Equal(t, 0, server.Stats("/one/foo").CachedRequests)
+
+	dt, err := os.ReadFile(filepath.Join(out1, "foo"))
+	require.NoError(t, err)
+	require.Equal(t, []byte("content1"), dt)
+
+	st = llb.HTTP(server.URL + "/two/foo")
+
+	def, err = st.Marshal(sb.Context())
+	require.NoError(t, err)
+
+	out2 := t.TempDir()
+	_, err = c.Solve(sb.Context(), def, SolveOpt{
+		Exports: []ExportEntry{
+			{
+				Type:      ExporterLocal,
+				OutputDir: out2,
+			},
+		},
+	}, nil)
+	require.NoError(t, err)
+
+	require.Equal(t, 1, server.Stats("/two/foo").AllRequests)
+	require.Equal(t, 0, server.Stats("/two/foo").CachedRequests)
+
+	dt, err = os.ReadFile(filepath.Join(out2, "foo"))
+	require.NoError(t, err)
+	require.Equal(t, []byte("another"), dt)
+
+	out2 = t.TempDir()
+	_, err = c.Solve(sb.Context(), def, SolveOpt{
+		Exports: []ExportEntry{
+			{
+				Type:      ExporterLocal,
+				OutputDir: out2,
+			},
+		},
+	}, nil)
+	require.NoError(t, err)
+
+	require.Equal(t, 2, server.Stats("/two/foo").AllRequests)
+	require.Equal(t, 1, server.Stats("/two/foo").CachedRequests)
+
+	allReqs := server.Stats("/two/foo").Requests
+	require.Equal(t, 2, len(allReqs))
+	require.Equal(t, http.MethodGet, allReqs[0].Method)
+	require.Equal(t, "gzip", allReqs[0].Header.Get("Accept-Encoding"))
+	require.Equal(t, http.MethodHead, allReqs[1].Method)
+	require.Equal(t, "gzip", allReqs[1].Header.Get("Accept-Encoding"))
+
+	require.NoError(t, os.RemoveAll(filepath.Join(out2, "foo")))
+}
+
 func testResolveAndHosts(t *testing.T, sb integration.Sandbox) {
 	requiresLinux(t)
 	c, err := New(sb.Context(), sb.Address())
diff --git a/source/http/source.go b/source/http/source.go
index a7d7ae49d749..8b4a417f4159 100644
--- a/source/http/source.go
+++ b/source/http/source.go
@@ -1,6 +1,7 @@
 package http
 
 import (
+	"bytes"
 	"context"
 	"crypto/sha256"
 	"encoding/json"
@@ -126,13 +127,16 @@ func (hs *httpSourceHandler) client(g session.Group) *http.Client {
 // this package.
 func (hs *httpSourceHandler) urlHash() (digest.Digest, error) {
 	dt, err := json.Marshal(struct {
-		Filename       string
+		Filename       []byte
 		Perm, UID, GID int
 	}{
-		Filename: getFileName(hs.src.URL, hs.src.Filename, nil),
-		Perm:     hs.src.Perm,
-		UID:      hs.src.UID,
-		GID:      hs.src.GID,
+		Filename: bytes.Join([][]byte{
+			[]byte(hs.src.URL),
+			[]byte(hs.src.Filename),
+		}, []byte{0}),
+		Perm: hs.src.Perm,
+		UID:  hs.src.UID,
+		GID:  hs.src.GID,
 	})
 	if err != nil {
 		return "", err