amadan

carlos / platform Public

edge: the static tag covers the resolved type, and nobody queues for one

Astra round 3 (accept with fixes):

- Hashing the extension did not cover two boxes whose MIME tables type
  one file differently. The cache now resolves the Content-Type as
  net/http does, hashes that in, and hands it back so the type the tag
  covers is the type sent.
- Four requests for four large cold assets could hold every hasher and
  make the first load of any other static site wait. Files up to 1 MiB
  hash inline, and a request that has waited 250ms for a hasher or for
  another request's hash is served untagged.
- The hashed-once test did not force its requests to overlap, and the
  replaced-file test changed size as well as identity. The first now
  holds the hash open until every other caller is waiting; the second
  replaces the file with the same length and mtime.
Agent notes
Mutants checked: no coalescing, no os.SameFile, type not hashed, small files queueing. Each fails exactly the test written for it. 20 runs under -race are clean.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
claude 2b6aad20b0dbecf4d7f2d4efe5c7a73ec1d8e2bc parent fcaf736
2 files changed, +257 −79
  • internal/edge/static.go +105 −43
  • internal/edge/static_cache_test.go +152 −36

CI passed — run details

diff --git a/internal/edge/static.go b/internal/edge/static.go
index e816af52b..0cf6cf776 100644
--- a/internal/edge/static.go
+++ b/internal/edge/static.go
@@ -22,6 +22,7 @@ import (
"io"
"io/fs"
"log"
+ "mime"
"net/http"
"os"
"path"
@@ -442,9 +443,12 @@ func (s *staticSite) serveTree(dir string, w http.ResponseWriter, r *http.Reques
// against, and it is the file whose bytes are about to be sent.
if ofi, err := f.Stat(); err == nil && !ofi.IsDir() {
fi = ofi
- if tag, err := staticTags.tag(r.Context(), fp, f, fi); err == nil {
+ if tag, ctype, err := staticTags.tag(r.Context(), fp, f, fi); err == nil {
+ // The type the tag was computed over is the type sent: set here,
+ // ServeContent leaves it alone.
w.Header().Set("ETag", tag)
- } else if r.Context().Err() == nil {
+ w.Header().Set("Content-Type", ctype)
+ } else if !errors.Is(err, errTagBusy) && r.Context().Err() == nil {
// No tag is the honest answer: the response then carries no
// validator and no-cache makes the browser refetch it whole.
log.Printf("edge: static %s/%s: tag %s: %v", s.app, s.channel, p, err)
@@ -457,7 +461,7 @@ func (s *staticSite) serveTree(dir string, w http.ResponseWriter, r *http.Reques
// tagCache computes and remembers the entity tag of each extracted file: a
// hash of its bytes and of the one thing besides them that decides what the
-// client receives, the file name's extension.
+// client receives, its Content-Type.
//
// The content, and nothing that merely names it. A version name is reused —
// a pruned version re-shipped under its old name is a new release
@@ -468,12 +472,14 @@ func (s *staticSite) serveTree(dir string, w http.ResponseWriter, r *http.Reques
// release that changed one page does not make every browser refetch every
// asset.
//
-// The extension is hashed in because the same bytes can be two
-// representations. net/http types a file by its extension and sniffs only
-// when there is none, so /about served from an extensionless file is
-// text/plain and the same bytes at about/index.html are text/html. A 304
-// carries no Content-Type; bytes alone would leave the browser rendering the
-// old one.
+// The type is hashed in because the same bytes can be two representations.
+// /about served from an extensionless file is sniffed as text/plain; the same
+// bytes at about/index.html are text/html; and two boxes whose MIME tables
+// differ type one .md file differently. A 304 carries no Content-Type, so
+// bytes alone would leave the browser rendering the old one. The type is
+// resolved here exactly as net/http resolves it — the extension's mapping,
+// else a sniff of the first 512 bytes — and handed back with the tag, so the
+// type the tag covers is the type the response carries.
//
// WEAK, although such a hash is as strong as a validator gets. The edge gzips
// compressible responses above this handler and weakens a strong tag when it
@@ -486,16 +492,23 @@ func (s *staticSite) serveTree(dir string, w http.ResponseWriter, r *http.Reques
// One cache for the process rather than one per handler: the edge rebuilds
// every static handler on each route-table reload, and a per-handler memo
// would re-hash every site each time a route anywhere on the box changed.
-// Three properties make a shared, long-lived memo safe:
+// What makes a shared, long-lived memo safe:
//
// - An entry is checked against the file it was computed FROM, not the path
// it was computed at (os.SameFile, size, mtime). A tree replaced on disk
// under a live handler — cache maintenance, then a re-shipped version of
// the same name extracted again — is new files, and hashes again.
// - A cold file is hashed once however many requests want it: the rest
-// wait for the one in flight. And at most tagHashers files hash at a
-// time, so a sweep of a site's large assets cannot occupy every core and
-// the disk at once. A request that gives up waiting is served untagged.
+// wait for the one in flight.
+// - A file over tagInline hashes under a semaphore, tagHashers at a time,
+// so a sweep of a site's large assets cannot take every core and the disk
+// at once. Small files never queue behind them: their work is bounded by
+// their size and they hash inline.
+// - Nobody waits long. A tag is an optimization, and a request that has
+// waited `wait` for a hasher or for someone else's hash is served without
+// one (errTagBusy): correct, uncached, and tried again on the next
+// request. Only the request doing a hash sees it through, because every
+// later request needs the result.
// - It is bounded: past tagCacheMax entries it is emptied and refills on
// demand, which costs hashing and never correctness.
type tagCache struct {
@@ -503,87 +516,136 @@ type tagCache struct {
entries map[string]tagEntry
flights map[string]chan struct{} // closed when the hash for that path is done
sem chan struct{}
- hashed atomic.Int64 // files hashed since start; the tests' evidence of coalescing
+ wait time.Duration
+
+ // Evidence and a seam for the tests: files hashed, callers currently
+ // waiting on another's hash, and a hook run inside a hash (nil in
+ // production) so a test can hold one open.
+ hashed atomic.Int64
+ waiting atomic.Int64
+ hold func()
}
type tagEntry struct {
- fi os.FileInfo
- tag string
+ fi os.FileInfo
+ tag, ctype string
}
const (
tagHashers = 4
+ tagInline = 1 << 20 // files up to this size hash without a hasher slot
+ tagWait = 250 * time.Millisecond
tagCacheMax = 1 << 16
)
-var staticTags = &tagCache{
- entries: map[string]tagEntry{},
- flights: map[string]chan struct{}{},
- sem: make(chan struct{}, tagHashers),
+// errTagBusy: the tag could not be had within the wait budget. The caller
+// serves the file untagged.
+var errTagBusy = errors.New("static tag: hashers busy")
+
+func newTagCache() *tagCache {
+ return &tagCache{
+ entries: map[string]tagEntry{},
+ flights: map[string]chan struct{}{},
+ sem: make(chan struct{}, tagHashers),
+ wait: tagWait,
+ }
}
+var staticTags = newTagCache()
+
// current reports whether e still describes the file fi is.
func (e tagEntry) current(fi os.FileInfo) bool {
return e.fi != nil && os.SameFile(e.fi, fi) && e.fi.Size() == fi.Size() && e.fi.ModTime().Equal(fi.ModTime())
}
-// tag returns the entity tag for the open file f at fp, hashing it if no
-// current entry exists. f is left positioned at its start. ctx bounds the
-// WAITING — on another request's hash, or on a free hasher — never a hash in
-// progress, whose result every later request needs anyway.
-func (c *tagCache) tag(ctx context.Context, fp string, f *os.File, fi os.FileInfo) (string, error) {
+// tag returns the entity tag and Content-Type for the open file f at fp,
+// hashing it if no current entry exists. f is left positioned at its start.
+// ctx and the wait budget bound the WAITING — on another request's hash, or
+// on a free hasher — never a hash in progress.
+func (c *tagCache) tag(ctx context.Context, fp string, f *os.File, fi os.FileInfo) (tag, ctype string, err error) {
+ var budget <-chan time.Time
for {
c.mu.Lock()
if e, ok := c.entries[fp]; ok && e.current(fi) {
c.mu.Unlock()
- return e.tag, nil
+ return e.tag, e.ctype, nil
+ }
+ if budget == nil {
+ t := time.NewTimer(c.wait)
+ defer t.Stop()
+ budget = t.C
}
- if wait, ok := c.flights[fp]; ok {
+ if flight, ok := c.flights[fp]; ok {
c.mu.Unlock()
+ c.waiting.Add(1)
select {
- case <-wait:
- continue // look again: usually a hit, a miss if the file changed
+ case <-flight:
+ c.waiting.Add(-1)
+ continue // look again: usually a hit, a miss if the file differs
case <-ctx.Done():
- return "", ctx.Err()
+ c.waiting.Add(-1)
+ return "", "", ctx.Err()
+ case <-budget:
+ c.waiting.Add(-1)
+ return "", "", errTagBusy
}
}
done := make(chan struct{})
c.flights[fp] = done
c.mu.Unlock()
- tag, err := c.hash(ctx, f, fi)
+ tag, ctype, err = c.hash(ctx, budget, f, fi)
c.mu.Lock()
if err == nil {
if len(c.entries) >= tagCacheMax {
c.entries = map[string]tagEntry{}
}
- c.entries[fp] = tagEntry{fi: fi, tag: tag}
+ c.entries[fp] = tagEntry{fi: fi, tag: tag, ctype: ctype}
}
delete(c.flights, fp)
c.mu.Unlock()
close(done)
- return tag, err
+ return tag, ctype, err
}
}
-func (c *tagCache) hash(ctx context.Context, f *os.File, fi os.FileInfo) (string, error) {
- select {
- case c.sem <- struct{}{}:
- defer func() { <-c.sem }()
- case <-ctx.Done():
- return "", ctx.Err()
+func (c *tagCache) hash(ctx context.Context, budget <-chan time.Time, f *os.File, fi os.FileInfo) (string, string, error) {
+ if fi.Size() > tagInline {
+ select {
+ case c.sem <- struct{}{}:
+ defer func() { <-c.sem }()
+ case <-ctx.Done():
+ return "", "", ctx.Err()
+ case <-budget:
+ return "", "", errTagBusy
+ }
}
c.hashed.Add(1)
+ if c.hold != nil {
+ c.hold()
+ }
+ // net/http's own rule (serveContent): the extension's type, else a sniff
+ // of the first 512 bytes.
+ var head [512]byte
+ n, err := io.ReadFull(f, head[:])
+ if err != nil && err != io.EOF && err != io.ErrUnexpectedEOF {
+ return "", "", err
+ }
+ ctype := mime.TypeByExtension(filepath.Ext(fi.Name()))
+ if ctype == "" {
+ ctype = http.DetectContentType(head[:n])
+ }
h := sha256.New()
- io.WriteString(h, strings.ToLower(filepath.Ext(fi.Name())))
+ io.WriteString(h, ctype)
h.Write([]byte{0})
+ h.Write(head[:n])
if _, err := io.Copy(h, f); err != nil {
- return "", err
+ return "", "", err
}
if _, err := f.Seek(0, io.SeekStart); err != nil {
- return "", err
+ return "", "", err
}
- return `W/"` + hex.EncodeToString(h.Sum(nil)[:16]) + `"`, nil
+ return `W/"` + hex.EncodeToString(h.Sum(nil)[:16]) + `"`, ctype, nil
}
// containsDotDot reports whether a URL path has a ".." element — net/http's
diff --git a/internal/edge/static_cache_test.go b/internal/edge/static_cache_test.go
index 0aa64924e..8a51154a6 100644
--- a/internal/edge/static_cache_test.go
+++ b/internal/edge/static_cache_test.go
@@ -2,7 +2,10 @@ package edge
import (
"context"
+ "errors"
+ "fmt"
"io"
+ "mime"
"net/http"
"net/http/httptest"
"os"
@@ -10,6 +13,7 @@ import (
"strings"
"sync"
"testing"
+ "time"
"github.com/carlosframework/platform/internal/registry"
"github.com/carlosframework/platform/internal/release"
@@ -330,48 +334,152 @@ func TestStaticTagCoversTheContentType(t *testing.T) {
}
}
+// openTagged writes body at dir/name and opens it the way serveTree does.
+func openTagged(t *testing.T, dir, name, body string) (string, *os.File, os.FileInfo) {
+ t.Helper()
+ fp := filepath.Join(dir, name)
+ if err := os.WriteFile(fp, []byte(body), 0o644); err != nil {
+ t.Fatal(err)
+ }
+ f, err := os.Open(fp)
+ if err != nil {
+ t.Fatal(err)
+ }
+ t.Cleanup(func() { f.Close() })
+ fi, err := f.Stat()
+ if err != nil {
+ t.Fatal(err)
+ }
+ return fp, f, fi
+}
+
// A cold file is hashed ONCE however many requests arrive for it together.
// Without that, a burst of requests against one large uncached asset — HEADs
// and one-byte ranges included — would each read and hash the whole file
// before sending a byte.
-func TestStaticColdFileIsHashedOnceUnderABurst(t *testing.T) {
- big := strings.Repeat("0123456789abcdef", 64*1024) // 1 MiB
- srv := siteServer(t, "site", "v1", map[string]string{"index.html": "home", "big.bin": big})
- before := staticTags.hashed.Load()
-
+//
+// The overlap is forced, not hoped for: the first hash is held until every
+// other caller is known to be waiting on it.
+func TestTagCacheHashesAColdFileOnceForOverlappingRequests(t *testing.T) {
+ c := newTagCache()
+ c.wait = time.Minute
+ held, release := make(chan struct{}), make(chan struct{})
+ var heldOnce, releaseOnce sync.Once
+ c.hold = func() { heldOnce.Do(func() { close(held) }); <-release }
+ letGo := func() { releaseOnce.Do(func() { close(release) }) }
+ defer letGo() // a failure below must not strand the callers
+ dir := t.TempDir()
+ if err := os.WriteFile(filepath.Join(dir, "big.bin"), []byte(strings.Repeat("x", 4096)), 0o644); err != nil {
+ t.Fatal(err)
+ }
+ const callers = 12
+ tags := make([]string, callers)
+ errs := make([]error, callers)
var wg sync.WaitGroup
- tags := make([]string, 24)
- for i := range tags {
+ call := func(i int) {
+ defer wg.Done()
+ fp := filepath.Join(dir, "big.bin")
+ f, err := os.Open(fp)
+ if err != nil {
+ errs[i] = err
+ return
+ }
+ defer f.Close()
+ fi, err := f.Stat()
+ if err != nil {
+ errs[i] = err
+ return
+ }
+ tags[i], _, errs[i] = c.tag(context.Background(), fp, f, fi)
+ }
+ wg.Add(1)
+ go call(0)
+ <-held // the first caller is inside its hash
+ for i := 1; i < callers; i++ {
wg.Add(1)
- go func(i int) {
- defer wg.Done()
- method := "GET"
- if i%2 == 1 {
- method = "HEAD"
- }
- req, err := http.NewRequest(method, srv.URL+"/big.bin", nil)
- if err != nil {
- return
- }
- req.Host = "site.example.com"
- req.Header.Set("Range", "bytes=0-0")
- resp, err := http.DefaultClient.Do(req)
- if err != nil {
- return
- }
- io.Copy(io.Discard, resp.Body)
- resp.Body.Close()
- tags[i] = resp.Header.Get("ETag")
- }(i)
+ go call(i)
}
+ deadline := time.Now().Add(10 * time.Second)
+ for c.waiting.Load() != callers-1 {
+ if time.Now().After(deadline) {
+ t.Fatalf("%d of %d callers are waiting on the hash in flight", c.waiting.Load(), callers-1)
+ }
+ time.Sleep(time.Millisecond)
+ }
+ letGo()
wg.Wait()
- for i, tag := range tags {
- if tag == "" || tag != tags[0] {
- t.Fatalf("request %d ETag = %q, want every request to carry %q", i, tag, tags[0])
+ for i := range tags {
+ if errs[i] != nil || tags[i] == "" || tags[i] != tags[0] {
+ t.Fatalf("caller %d = %q, %v; want every caller to get %q", i, tags[i], errs[i], tags[0])
}
}
- if got := staticTags.hashed.Load() - before; got != 1 {
- t.Fatalf("the file was hashed %d times under a burst of %d requests, want once", got, len(tags))
+ if got := c.hashed.Load(); got != 1 {
+ t.Fatalf("the file was hashed %d times for %d overlapping callers, want once", got, callers)
+ }
+}
+
+// Large files hash a few at a time, and a request that cannot get a turn —
+// or whose file is still being hashed for someone else — is not made to wait
+// for it: past a short budget it is served without a tag. A small file never
+// queues behind them at all. Otherwise four requests for four large cold
+// assets would hold up the first load of every other static site on the box.
+func TestTagCacheDoesNotQueueBehindLargeHashes(t *testing.T) {
+ c := newTagCache()
+ c.wait = 20 * time.Millisecond
+ dir := t.TempDir()
+ for i := 0; i < tagHashers; i++ {
+ c.sem <- struct{}{} // every hasher busy with someone's large asset
+ }
+
+ fp, f, fi := openTagged(t, dir, "big.bin", strings.Repeat("x", tagInline+1))
+ if _, _, err := c.tag(context.Background(), fp, f, fi); !errors.Is(err, errTagBusy) {
+ t.Fatalf("a large file with no free hasher = %v, want errTagBusy", err)
+ }
+ if n := len(c.flights); n != 0 {
+ t.Fatalf("%d flights left registered after giving up", n)
+ }
+
+ fp, f, fi = openTagged(t, dir, "page.html", "<p>a small page</p>")
+ tag, ctype, err := c.tag(context.Background(), fp, f, fi)
+ if err != nil || tag == "" || !strings.HasPrefix(ctype, "text/html") {
+ t.Fatalf("a small file while every hasher is busy = %q %q %v; want it tagged at once", tag, ctype, err)
+ }
+
+ // Once a hasher frees up the large file is tagged, and stays tagged.
+ <-c.sem
+ fp, f, fi = openTagged(t, dir, "big.bin", strings.Repeat("x", tagInline+1))
+ if tag, _, err := c.tag(context.Background(), fp, f, fi); err != nil || tag == "" {
+ t.Fatalf("a large file with a free hasher = %q, %v", tag, err)
+ }
+}
+
+// The tag covers the Content-Type the response will carry, not the extension
+// it was derived from: two boxes with different MIME tables type the same
+// bytes differently, and a 304 from one must not confirm the other's page.
+func TestTagCacheTagCoversTheResolvedContentType(t *testing.T) {
+ c := newTagCache()
+ const body = "# notes\n\nsome text\n"
+ // The MIME table is process-wide and an added type stays added, so each
+ // run takes an extension of its own (-count=N would otherwise find the
+ // last run's mapping in place of an unknown one).
+ ext := fmt.Sprintf(".zzcarlos%d", time.Now().UnixNano())
+ fp, f, fi := openTagged(t, t.TempDir(), "notes"+ext, body)
+ sniffedTag, sniffedType, err := c.tag(context.Background(), fp, f, fi)
+ if err != nil || !strings.HasPrefix(sniffedType, "text/plain") {
+ t.Fatalf("an unknown extension = %q %v, want it sniffed as text/plain", sniffedType, err)
+ }
+
+ // "Another box": the same bytes under the same name, with the type known.
+ if err := mime.AddExtensionType(ext, "text/markdown; charset=utf-8"); err != nil {
+ t.Fatal(err)
+ }
+ fp, f, fi = openTagged(t, t.TempDir(), "notes"+ext, body)
+ knownTag, knownType, err := c.tag(context.Background(), fp, f, fi)
+ if err != nil || !strings.HasPrefix(knownType, "text/markdown") {
+ t.Fatalf("a mapped extension = %q %v, want text/markdown", knownType, err)
+ }
+ if knownTag == sniffedTag {
+ t.Fatalf("the same bytes typed %q and %q share the tag %s", sniffedType, knownType, knownTag)
}
}
@@ -387,10 +495,18 @@ func TestStaticTagDoesNotOutliveTheFileItDescribes(t *testing.T) {
t.Fatal("no ETag for /style.css")
}
- // Replace the file the way a fresh extraction would: new bytes under a
- // new inode, renamed into place.
+ // Replace the file the way a fresh extraction would — a new file renamed
+ // into place — and make it the hardest case to notice: the same length
+ // and the same mtime, so only the file's identity gives it away.
fp := filepath.Join(tree, "style.css")
- if err := os.WriteFile(fp+".new", []byte("body{color:red}"), 0o644); err != nil {
+ before, err := os.Stat(fp)
+ if err != nil {
+ t.Fatal(err)
+ }
+ if err := os.WriteFile(fp+".new", []byte("main{}"), 0o644); err != nil {
+ t.Fatal(err)
+ }
+ if err := os.Chtimes(fp+".new", before.ModTime(), before.ModTime()); err != nil {
t.Fatal(err)
}
if err := os.Rename(fp+".new", fp); err != nil {
@@ -398,7 +514,7 @@ func TestStaticTagDoesNotOutliveTheFileItDescribes(t *testing.T) {
}
resp, body := reqStatic(t, "GET", srv.URL, "/style.css", map[string]string{"If-None-Match": oldTag})
- if resp.StatusCode != http.StatusOK || body != "body{color:red}" {
+ if resp.StatusCode != http.StatusOK || body != "main{}" {
t.Fatalf("revalidating against a replaced file = %d %q, want 200 with the new bytes", resp.StatusCode, body)
}
if got := resp.Header.Get("ETag"); got == "" || got == oldTag {