Skip to content

Commit

Permalink
http: fix etag cache scoping
Browse files Browse the repository at this point in the history
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>
  • Loading branch information
tonistiigi committed Nov 25, 2024
1 parent 51fbdf3 commit 06ff841
Show file tree
Hide file tree
Showing 2 changed files with 108 additions and 5 deletions.
99 changes: 99 additions & 0 deletions client/client_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -104,6 +104,7 @@ var allTests = []func(t *testing.T, sb integration.Sandbox){
testCallDiskUsage,
testBuildMultiMount,
testBuildHTTPSource,
testBuildHTTPSourceEtagScope,
testBuildPushAndValidate,
testBuildExportWithUncompressed,
testBuildExportScratch,
Expand Down Expand Up @@ -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())
Expand Down
14 changes: 9 additions & 5 deletions source/http/source.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package http

import (
"bytes"
"context"
"crypto/sha256"
"encoding/json"
Expand Down Expand Up @@ -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
Expand Down

0 comments on commit 06ff841

Please sign in to comment.