Skip to content

Commit 06ff841

Browse files
committed
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 <[email protected]>
1 parent 51fbdf3 commit 06ff841

File tree

2 files changed

+108
-5
lines changed

2 files changed

+108
-5
lines changed

client/client_test.go

Lines changed: 99 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -104,6 +104,7 @@ var allTests = []func(t *testing.T, sb integration.Sandbox){
104104
testCallDiskUsage,
105105
testBuildMultiMount,
106106
testBuildHTTPSource,
107+
testBuildHTTPSourceEtagScope,
107108
testBuildPushAndValidate,
108109
testBuildExportWithUncompressed,
109110
testBuildExportScratch,
@@ -2769,6 +2770,104 @@ func testBuildHTTPSource(t *testing.T, sb integration.Sandbox) {
27692770
// TODO: check that second request was marked as cached
27702771
}
27712772

2773+
// docker/buildx#2803
2774+
func testBuildHTTPSourceEtagScope(t *testing.T, sb integration.Sandbox) {
2775+
integration.SkipOnPlatform(t, "windows")
2776+
c, err := New(sb.Context(), sb.Address())
2777+
require.NoError(t, err)
2778+
defer c.Close()
2779+
2780+
modTime := time.Now().Add(-24 * time.Hour) // avoid falso positive with current time
2781+
2782+
sharedEtag := identity.NewID()
2783+
resp := httpserver.Response{
2784+
Etag: sharedEtag,
2785+
Content: []byte("content1"),
2786+
LastModified: &modTime,
2787+
}
2788+
resp2 := httpserver.Response{
2789+
Etag: sharedEtag,
2790+
Content: []byte("another"),
2791+
LastModified: &modTime,
2792+
}
2793+
2794+
server := httpserver.NewTestServer(map[string]httpserver.Response{
2795+
"/one/foo": resp,
2796+
"/two/foo": resp2,
2797+
})
2798+
defer server.Close()
2799+
2800+
// first correct request
2801+
st := llb.HTTP(server.URL + "/one/foo")
2802+
2803+
def, err := st.Marshal(sb.Context())
2804+
require.NoError(t, err)
2805+
2806+
out1 := t.TempDir()
2807+
_, err = c.Solve(sb.Context(), def, SolveOpt{
2808+
Exports: []ExportEntry{
2809+
{
2810+
Type: ExporterLocal,
2811+
OutputDir: out1,
2812+
},
2813+
},
2814+
}, nil)
2815+
require.NoError(t, err)
2816+
2817+
require.Equal(t, 1, server.Stats("/one/foo").AllRequests)
2818+
require.Equal(t, 0, server.Stats("/one/foo").CachedRequests)
2819+
2820+
dt, err := os.ReadFile(filepath.Join(out1, "foo"))
2821+
require.NoError(t, err)
2822+
require.Equal(t, []byte("content1"), dt)
2823+
2824+
st = llb.HTTP(server.URL + "/two/foo")
2825+
2826+
def, err = st.Marshal(sb.Context())
2827+
require.NoError(t, err)
2828+
2829+
out2 := t.TempDir()
2830+
_, err = c.Solve(sb.Context(), def, SolveOpt{
2831+
Exports: []ExportEntry{
2832+
{
2833+
Type: ExporterLocal,
2834+
OutputDir: out2,
2835+
},
2836+
},
2837+
}, nil)
2838+
require.NoError(t, err)
2839+
2840+
require.Equal(t, 1, server.Stats("/two/foo").AllRequests)
2841+
require.Equal(t, 0, server.Stats("/two/foo").CachedRequests)
2842+
2843+
dt, err = os.ReadFile(filepath.Join(out2, "foo"))
2844+
require.NoError(t, err)
2845+
require.Equal(t, []byte("another"), dt)
2846+
2847+
out2 = t.TempDir()
2848+
_, err = c.Solve(sb.Context(), def, SolveOpt{
2849+
Exports: []ExportEntry{
2850+
{
2851+
Type: ExporterLocal,
2852+
OutputDir: out2,
2853+
},
2854+
},
2855+
}, nil)
2856+
require.NoError(t, err)
2857+
2858+
require.Equal(t, 2, server.Stats("/two/foo").AllRequests)
2859+
require.Equal(t, 1, server.Stats("/two/foo").CachedRequests)
2860+
2861+
allReqs := server.Stats("/two/foo").Requests
2862+
require.Equal(t, 2, len(allReqs))
2863+
require.Equal(t, http.MethodGet, allReqs[0].Method)
2864+
require.Equal(t, "gzip", allReqs[0].Header.Get("Accept-Encoding"))
2865+
require.Equal(t, http.MethodHead, allReqs[1].Method)
2866+
require.Equal(t, "gzip", allReqs[1].Header.Get("Accept-Encoding"))
2867+
2868+
require.NoError(t, os.RemoveAll(filepath.Join(out2, "foo")))
2869+
}
2870+
27722871
func testResolveAndHosts(t *testing.T, sb integration.Sandbox) {
27732872
requiresLinux(t)
27742873
c, err := New(sb.Context(), sb.Address())

source/http/source.go

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
package http
22

33
import (
4+
"bytes"
45
"context"
56
"crypto/sha256"
67
"encoding/json"
@@ -126,13 +127,16 @@ func (hs *httpSourceHandler) client(g session.Group) *http.Client {
126127
// this package.
127128
func (hs *httpSourceHandler) urlHash() (digest.Digest, error) {
128129
dt, err := json.Marshal(struct {
129-
Filename string
130+
Filename []byte
130131
Perm, UID, GID int
131132
}{
132-
Filename: getFileName(hs.src.URL, hs.src.Filename, nil),
133-
Perm: hs.src.Perm,
134-
UID: hs.src.UID,
135-
GID: hs.src.GID,
133+
Filename: bytes.Join([][]byte{
134+
[]byte(hs.src.URL),
135+
[]byte(hs.src.Filename),
136+
}, []byte{0}),
137+
Perm: hs.src.Perm,
138+
UID: hs.src.UID,
139+
GID: hs.src.GID,
136140
})
137141
if err != nil {
138142
return "", err

0 commit comments

Comments
 (0)