Merge remote-tracking branch 'origin/master'

internal/media/store.go

@@ -265,6 +265,16 @@ func (s *Store) Put(kind Kind, mime, source string, data []byte) (Blob, error) {
 		}
 	}
 
+	// Every return past the reservation has to give it back, so the defer owns
+	// that rather than each error path: a path that forgot over-counted the
+	// store until the next Open re-walked the directory.
+	stored := false
+	defer func() {
+		if fresh && !stored {
+			s.release(b.Size)
+		}
+	}()
+
 	// The sidecar goes first. Written second, a full disk or a crash between
 	// the two left the bytes on disk with no sidecar, and List only sees
 	// sidecars, so Prune could never collect them: Put returned an error and an
@@ -274,11 +284,9 @@ func (s *Store) Put(kind Kind, mime, source string, data []byte) (Blob, error) {
 	}
 	if err := writeFile(blobPath, data); err != nil {
 		_ = os.Remove(metaPath)
-		if fresh {
-			s.release(b.Size)
-		}
 		return Blob{}, err
 	}
+	stored = true
 	return b, nil
 }
 
@@ -331,12 +339,22 @@ func (s *Store) PutFile(kind Kind, mime, source, src string) (Blob, error) {
 			return Blob{}, err
 		}
 	}
+	// Same reasoning as Put: the reservation is released by one defer, not by
+	// whichever error path remembered to.
+	stored := false
+	defer func() {
+		if fresh && !stored {
+			s.release(b.Size)
+		}
+	}()
+
 	if err := writeMeta(metaPath, b); err != nil {
 		return Blob{}, err
 	}
 	if !fresh {
 		// Same bytes already here. Drop the spool copy.
 		_ = os.Remove(src)
+		stored = true
 		return b, nil
 	}
 	if err := os.Chmod(src, filePerm); err != nil {
@@ -344,9 +362,9 @@ func (s *Store) PutFile(kind Kind, mime, source, src string) (Blob, error) {
 	}
 	if err := os.Rename(src, blobPath); err != nil {
 		_ = os.Remove(metaPath)
-		s.release(b.Size)
 		return Blob{}, fmt.Errorf("media: move spool: %w", err)
 	}
+	stored = true
 	return b, nil
 }
 

internal/media/store_test.go

@@ -287,6 +287,73 @@ func TestPutLeavesNothingWhenTheBytesCannotBeWritten(t *testing.T) {
 	}
 }
 
+// An over-counted store answers ErrStoreFull while the disk has room, and only
+// the next Open corrects it. So every failed write has to give its reservation
+// back, not just the one that remembered to.
+func TestPutReleasesTheBudgetWhenTheSidecarCannotBeWritten(t *testing.T) {
+	s := testStore(t)
+	data := []byte("no sidecar for this")
+	blockSidecar(t, s, KindImage, data)
+	if _, err := s.Put(KindImage, "image/png", "web:upload", data); err == nil {
+		t.Fatal("put must fail")
+	}
+	if s.Total() != 0 {
+		t.Errorf("total = %d, want the failed put not counted", s.Total())
+	}
+}
+
+func TestPutFileReleasesTheBudgetWhenTheSidecarCannotBeWritten(t *testing.T) {
+	s := testStore(t)
+	data := []byte("no sidecar for this either")
+	blockSidecar(t, s, KindAudio, data)
+	src := filepath.Join(t.TempDir(), "capture.wav")
+	if err := os.WriteFile(src, data, 0o600); err != nil {
+		t.Fatal(err)
+	}
+	if _, err := s.PutFile(KindAudio, "audio/wav", "meeting", src); err == nil {
+		t.Fatal("put file must fail")
+	}
+	if s.Total() != 0 {
+		t.Errorf("total = %d, want the failed put not counted", s.Total())
+	}
+}
+
+// The chmod arm is PutFile's alone: Put never touches a spool file.
+func TestPutFileReleasesTheBudgetWhenTheSpoolCannotBeChmodded(t *testing.T) {
+	if os.Geteuid() == 0 {
+		t.Skip("root can chmod a file it does not own")
+	}
+	// A symlink to a file owned by somebody else. Stat and the hash follow it
+	// and succeed; chmod follows it too and is refused.
+	src := filepath.Join(t.TempDir(), "capture.wav")
+	if err := os.Symlink("/etc/hosts", src); err != nil {
+		t.Fatal(err)
+	}
+	info, err := os.Stat(src)
+	if err != nil || info.Size() == 0 {
+		t.Skip("no readable /etc/hosts to point at")
+	}
+	s := testStore(t)
+	if _, err := s.PutFile(KindAudio, "audio/wav", "meeting", src); err == nil {
+		t.Fatal("put file must fail")
+	}
+	if s.Total() != 0 {
+		t.Errorf("total = %d, want the failed put not counted", s.Total())
+	}
+}
+
+// blockSidecar puts a directory where the sidecar for data has to go, so
+// writeMeta fails while the blob path is still free.
+func blockSidecar(t *testing.T, s *Store, kind Kind, data []byte) {
+	t.Helper()
+	sum := sha256.Sum256(data)
+	id := hex.EncodeToString(sum[:])
+	bucket := filepath.Join(s.dir, string(kind), id[:2])
+	if err := os.MkdirAll(filepath.Join(bucket, id+".json"), 0o700); err != nil {
+		t.Fatal(err)
+	}
+}
+
 // The per-blob cap bounds one call and nothing bounded their sum. 64 MiB per
 // call times unlimited calls inside a seven-day window fills the disk mavend's
 // database lives on.