Files
Brad Fitzpatrick a6cffd0a6d drive/driveimpl/compositedav: don't run ttlcache cleanup goroutines
StatCache started each ttlcache with "go cache.Start()" and stopped
them in StatCache.stop. But ttlcache caches begin in the stopped
state, so if stop ran before the Start goroutine was scheduled,
Stop was a no-op and Start then ran forever, leaking a goroutine.
That made tests using tstest.ResourceCheck flaky:

    goroutine github.com/jellydator/ttlcache/v3.(*Cache[...]).Start

Rather than synchronize with Start, drop the background goroutine
entirely. Cache.Get already ignores expired items, so the goroutine
only existed to free memory. Instead, call DeleteExpired on each
write, which is cheap: it pops expired entries from the front of the
expiration heap and stops at the first unexpired one. With no
goroutines, StatCache.stop is no longer needed, so remove it.

Fixes #21824
Updates tailscale/corp#49459

Change-Id: Ib438fcd45296be79528613c5f5687725c38a137d
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>
2026-10-23 20:41:40 -04:00

259 lines
7.3 KiB
Go

// Copyright (c) Tailscale Inc & contributors
// SPDX-License-Identifier: BSD-3-Clause
package compositedav
import (
"fmt"
"log"
"net/http"
"net/url"
"path"
"strings"
"testing"
"time"
"github.com/google/go-cmp/cmp"
"tailscale.com/tstest"
)
var parentPath = "/parent with spaces"
var childPath = "/parent with spaces/child.txt"
var parentResponse = `<D:response>
<D:href>/parent%20with%20spaces/</D:href>
<D:propstat>
<D:prop>
<D:getlastmodified>Mon, 29 Apr 2024 19:52:23 GMT</D:getlastmodified>
<D:creationdate>Fri, 19 Apr 2024 04:13:34 GMT</D:creationdate>
<D:resourcetype>
<D:collection xmlns:D="DAV:" />
</D:resourcetype>
</D:prop>
<D:status>HTTP/1.1 200 OK</D:status>
</D:propstat>
</D:response>`
var childResponse = `
<D:response>
<D:href>/parent%20with%20spaces/child.txt</D:href>
<D:propstat>
<D:prop>
<D:getlastmodified>Mon, 29 Apr 2024 19:52:23 GMT</D:getlastmodified>
<D:creationdate>Fri, 19 Apr 2024 04:13:34 GMT</D:creationdate>
<D:resourcetype>
<D:collection xmlns:D="DAV:" />
</D:resourcetype>
</D:prop>
<D:status>HTTP/1.1 200 OK</D:status>
</D:propstat>
</D:response>`
var fullParent = []byte(
strings.ReplaceAll(
fmt.Sprintf(`<?xml version="1.0" encoding="UTF-8"?><D:multistatus xmlns:D="DAV:">%s%s</D:multistatus>`, parentResponse, childResponse),
"\n", ""))
var partialParent = []byte(
strings.ReplaceAll(
fmt.Sprintf(`<?xml version="1.0" encoding="UTF-8"?><D:multistatus xmlns:D="DAV:">%s</D:multistatus>`, parentResponse),
"\n", ""))
var fullChild = []byte(
strings.ReplaceAll(
fmt.Sprintf(`<?xml version="1.0" encoding="UTF-8"?><D:multistatus xmlns:D="DAV:">%s</D:multistatus>`, childResponse),
"\n", ""))
func TestStatCacheNoTimeout(t *testing.T) {
// Make sure we don't leak goroutines
tstest.ResourceCheck(t)
c := &StatCache{TTL: 5 * time.Second}
// check get before set
fetched := c.get(childPath, 0)
if fetched != nil {
t.Errorf("got %v, want nil", fetched)
}
// set new stat
ce := newCacheEntry(http.StatusMultiStatus, fullChild)
c.set(childPath, 0, ce)
fetched = c.get(childPath, 0)
if diff := cmp.Diff(fetched, ce); diff != "" {
t.Errorf("should have gotten cached value; (-got+want):%v", diff)
}
// fetch stat again, should still be cached
fetched = c.get(childPath, 0)
if diff := cmp.Diff(fetched, ce); diff != "" {
t.Errorf("should still have gotten cached value; (-got+want):%v", diff)
}
}
func TestStatCacheTimeout(t *testing.T) {
// Make sure we don't leak goroutines
tstest.ResourceCheck(t)
c := &StatCache{TTL: 250 * time.Millisecond}
// set new stat
ce := newCacheEntry(http.StatusMultiStatus, fullChild)
c.set(childPath, 0, ce)
fetched := c.get(childPath, 0)
if diff := cmp.Diff(fetched, ce); diff != "" {
t.Errorf("should have gotten cached value; (-got+want):%v", diff)
}
// wait for cache to expire and refetch stat, should be empty now
time.Sleep(c.TTL * 2)
fetched = c.get(childPath, 0)
if fetched != nil {
t.Errorf("cached value should have expired")
}
c.set(childPath, 0, ce)
// invalidate the cache and make sure nothing is returned
c.invalidate()
fetched = c.get(childPath, 0)
if fetched != nil {
t.Errorf("invalidate should have cleared cached value")
}
}
func TestParentChildRelationship(t *testing.T) {
// Make sure we don't leak goroutines
tstest.ResourceCheck(t)
c := &StatCache{TTL: 24 * time.Hour} // don't expire
missingParentPath := "/missingparent"
unparseableParentPath := "/unparseable"
c.set(parentPath, 1, newCacheEntry(http.StatusMultiStatus, fullParent))
c.set(missingParentPath, 1, newCacheEntry(http.StatusNotFound, nil))
c.set(unparseableParentPath, 1, newCacheEntry(http.StatusMultiStatus, []byte("<this will not parse")))
tests := []struct {
path string
depth int
want *cacheEntry
}{
{
path: parentPath,
depth: 1,
want: newCacheEntry(http.StatusMultiStatus, fullParent),
},
{
path: parentPath,
depth: 0,
want: newCacheEntry(http.StatusMultiStatus, partialParent),
},
{
path: childPath,
depth: 0,
want: newCacheEntry(http.StatusMultiStatus, fullChild),
},
{
path: path.Join(parentPath, "nonexistent.txt"),
depth: 0,
want: notFound,
},
{
path: missingParentPath,
depth: 1,
want: notFound,
},
{
path: missingParentPath,
depth: 0,
want: notFound,
},
{
path: path.Join(missingParentPath, "filename.txt"),
depth: 0,
want: notFound,
},
{
path: unparseableParentPath,
depth: 1,
want: nil,
},
{
path: unparseableParentPath,
depth: 0,
want: nil,
},
{
path: path.Join(unparseableParentPath, "filename.txt"),
depth: 0,
want: nil,
},
{
path: "/unknown",
depth: 1,
want: nil,
},
}
for _, test := range tests {
t.Run(fmt.Sprintf("%d%s", test.depth, test.path), func(t *testing.T) {
got := c.get(test.path, test.depth)
if diff := cmp.Diff(got, test.want); diff != "" {
t.Errorf("unexpected cached value; (-got+want):%v", diff)
log.Printf("want\n%s", test.want.Raw)
log.Printf("got\n%s", got.Raw)
}
})
}
}
// TestUnicodeNormalizationInsensitivity verifies that cache lookups treat
// canonically equivalent names as the same key. A parent directory listing
// caches children under the names from the server's hrefs (often NFC), while
// macOS WebDAV clients request the same files using NFD names. Without
// normalization, the depth 0 lookup would miss and get would wrongly infer
// notFound from the cached parent.
func TestUnicodeNormalizationInsensitivity(t *testing.T) {
// Make sure we don't leak goroutines
tstest.ResourceCheck(t)
c := &StatCache{TTL: 24 * time.Hour} // don't expire
const (
nfcParent = "\u30ae\u30bf\u30fc" // ギター in NFC: ギ is the single code point U+30AE
nfdParent = "\u30ad\u3099\u30bf\u30fc" // same name in NFD: キ U+30AD plus combining voiced mark U+3099
nfcChild = "\u30c6\u30ba\u30c8.wav" // テズト.wav in NFC: ズ is the single code point U+30BA
nfdChild = "\u30c6\u30b9\u3099\u30c8.wav" // same name in NFD: ス U+30B9 plus combining voiced mark U+3099
)
nfcParentPath := "/" + nfcParent
nfdParentPath := "/" + nfdParent
nfcChildPath := nfcParentPath + "/" + nfcChild
nfdChildPath := nfdParentPath + "/" + nfdChild
// The hrefs in a real PROPFIND response contain the percent-encoded UTF-8
// bytes of the names as they appear on the server's disk, here NFC.
unicodeParentResponse := strings.ReplaceAll(parentResponse, "/parent%20with%20spaces/", "/"+url.PathEscape(nfcParent)+"/")
unicodeChildResponse := strings.ReplaceAll(childResponse, "/parent%20with%20spaces/child.txt", "/"+url.PathEscape(nfcParent)+"/"+url.PathEscape(nfcChild))
unicodeFullParent := []byte(
strings.ReplaceAll(
fmt.Sprintf(`<?xml version="1.0" encoding="UTF-8"?><D:multistatus xmlns:D="DAV:">%s%s</D:multistatus>`, unicodeParentResponse, unicodeChildResponse),
"\n", ""))
unicodeFullChild := []byte(
strings.ReplaceAll(
fmt.Sprintf(`<?xml version="1.0" encoding="UTF-8"?><D:multistatus xmlns:D="DAV:">%s</D:multistatus>`, unicodeChildResponse),
"\n", ""))
c.set(nfcParentPath, 1, newCacheEntry(http.StatusMultiStatus, unicodeFullParent))
want := newCacheEntry(http.StatusMultiStatus, unicodeFullChild)
for _, childPath := range []string{nfcChildPath, nfdChildPath} {
got := c.get(childPath, 0)
if diff := cmp.Diff(got, want); diff != "" {
t.Errorf("get(%q): unexpected cached value; (-got+want):%v", childPath, diff)
}
}
}