Files: a sweep of the edges

Found by walking every handler and the UI with odd inputs, then a battery of requests
against the live hub. Fixed:
- A save over the 8 MiB cap was cut short in place: the file was truncated, then the
  first 8 MiB written, and the caller told 204. Now the body is read whole first and a
  body over the cap is a 413 with the file untouched.
- A link to a file showed the link's own time; now the file's, so the row reads right
  and the preview key follows the file.
- Reading a link to nothing answered 200 with an empty body (the open failed after the
  headers in root mode); now a 404 that names the target.
- A play request with an unknown codec or size was answered by inspecting the file
  first, so an audio-only file said "no video stream" instead of what was wrong; the
  choice is checked before any work, and is a 400.
- A stream that ended in failure was handed back to the next identical request for ten
  minutes; a failed one is now dropped and a fresh one made.
- A player that left while its stream was still finding a first segment made the loop
  try the next recipe; it stops. A stream closed mid-start is not started again.
- Ending a root-mode ffmpeg could wait forever: the daemon cannot signal root's
  processes, so the wait is bounded and the helper's own grace does the rest.
- A page going away (a phone closing the tab) releases its stream by a beacon, rather
  than ten minutes later by the sweeper.
- The path box resolves like a shell — relative to the folder shown, . and .. applied,
  trailing slashes dropped — in the tab and in the Move picker, with a test. A typed path
  that is a file opens it; a remembered folder that is gone falls back to / once.
- A dropped folder arrived as a nameless empty file; it is refused with a word. The
  upload toast counts what was actually uploaded.
- The viewer names a device, pipe or socket, and a dangling link, instead of calling
  them binary files.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>

internal/api/files_test.go

@@ -1,6 +1,7 @@
 package api
 
 import (
+	"bytes"
 	"net/http/httptest"
 	"net/url"
 	"os"
@@ -60,3 +61,44 @@ func TestFilesReadRanges(t *testing.T) {
 		}
 	}
 }
+
+// A write over the cap is refused whole: the file is not truncated first.
+func TestFilesWriteCap(t *testing.T) {
+	p := filepath.Join(t.TempDir(), "f.txt")
+	if err := os.WriteFile(p, []byte("keep me"), 0o644); err != nil {
+		t.Fatal(err)
+	}
+	s := &Server{fsLocal: files.Local{}}
+	body := make([]byte, 8<<20+1)
+	r := httptest.NewRequest("PUT", "/api/files/write?path="+url.QueryEscape(p), bytes.NewReader(body))
+	w := httptest.NewRecorder()
+	s.filesWrite(w, r)
+	if w.Code != 413 {
+		t.Fatalf("got %d", w.Code)
+	}
+	if b, _ := os.ReadFile(p); string(b) != "keep me" {
+		t.Fatalf("the file was touched: %q", b)
+	}
+	r = httptest.NewRequest("PUT", "/api/files/write?path="+url.QueryEscape(p), bytes.NewReader([]byte("new")))
+	w = httptest.NewRecorder()
+	s.filesWrite(w, r)
+	if b, _ := os.ReadFile(p); w.Code != 204 || string(b) != "new" {
+		t.Fatalf("a write within the cap: %d %q", w.Code, b)
+	}
+}
+
+// A link to nothing is a 404, not an empty 200.
+func TestFilesReadDanglingLink(t *testing.T) {
+	dir := t.TempDir()
+	l := filepath.Join(dir, "broken")
+	if err := os.Symlink("nowhere", l); err != nil {
+		t.Fatal(err)
+	}
+	s := &Server{fsLocal: files.Local{}}
+	r := httptest.NewRequest("GET", "/api/files/read?path="+url.QueryEscape(l), nil)
+	w := httptest.NewRecorder()
+	s.filesRead(w, r)
+	if w.Code != 404 {
+		t.Fatalf("got %d: %s", w.Code, w.Body.String())
+	}
+}

internal/api/play.go

@@ -79,6 +79,10 @@ func (s *Server) playStart(w http.ResponseWriter, r *http.Request) {
 		jsonErr(w, 415, "not a video")
 		return
 	}
+	if err := media.ValidChoice(body.Codec, body.Quality); err != nil {
+		jsonErr(w, 400, err.Error())
+		return
+	}
 	if _, err := fsys.Stat(r.Context(), p); err != nil {
 		fsErr(w, err)
 		return
@@ -115,6 +119,8 @@ func playErr(w http.ResponseWriter, err error) {
 		jsonErr(w, 422, de.Msg)
 	case strings.Contains(err.Error(), "already being re-encoded"):
 		jsonErr(w, 429, err.Error())
+	case strings.HasPrefix(err.Error(), "unknown ") || strings.HasPrefix(err.Error(), "nothing on this machine encodes"):
+		jsonErr(w, 400, err.Error())
 	case errors.Is(err, context.DeadlineExceeded):
 		jsonErr(w, 504, "the file took too long to read")
 	default:
@@ -132,7 +138,8 @@ func (s *Server) playStatus(w http.ResponseWriter, r *http.Request) {
 	jsonOK(w, 200, st.Status())
 }
 
-// playStop: DELETE /api/files/play/{id}
+// playStop: DELETE /api/files/play/{id}, or POST …/close — the form a page can send on its
+// way out (navigator.sendBeacon), so a phone closing the tab releases the machine at once.
 func (s *Server) playStop(w http.ResponseWriter, r *http.Request) {
 	s.plays.Close(r.PathValue("id"))
 	w.WriteHeader(204)

internal/api/server.go

@@ -317,6 +317,7 @@ func (s *Server) routes() *http.ServeMux {
 	m.HandleFunc("POST /api/files/play", s.auth(s.playStart))
 	m.HandleFunc("GET /api/files/play/{id}", s.auth(s.playStatus))
 	m.HandleFunc("DELETE /api/files/play/{id}", s.auth(s.playStop))
+	m.HandleFunc("POST /api/files/play/{id}/close", s.auth(s.playStop))
 	m.HandleFunc("GET /api/files/play/{id}/{file}", s.auth(s.playFile))
 	m.HandleFunc("GET /api/hardware", s.auth(s.hardwareReport))
 	m.HandleFunc("GET /api/network", s.auth(s.networkOverview))
@@ -1931,6 +1932,10 @@ func (s *Server) filesRead(w http.ResponseWriter, r *http.Request) {
 		jsonErr(w, 400, "not a regular file (device, pipe or socket)")
 		return
 	}
+	if in.Broken {
+		jsonErr(w, 404, "a link to nothing: "+in.Target)
+		return
+	}
 	mx, _ := strconv.ParseInt(r.URL.Query().Get("max"), 10, 64)
 	if mx > 0 && in.Size > mx {
 		jsonErr(w, 413, "file too large")
@@ -1989,7 +1994,14 @@ func (s *Server) filesWrite(w http.ResponseWriter, r *http.Request) {
 	if !ok {
 		return
 	}
-	if err := fsys.Write(r.Context(), p, io.LimitReader(r.Body, 8<<20)); err != nil {
+	// Whole in memory first: the write truncates the file, so a body over the cap must be
+	// refused before that, not cut short after it.
+	body, err := io.ReadAll(http.MaxBytesReader(w, r.Body, 8<<20))
+	if err != nil {
+		jsonErr(w, 413, "too large to save here (8 MiB at most)")
+		return
+	}
+	if err := fsys.Write(r.Context(), p, bytes.NewReader(body)); err != nil {
 		fsErr(w, err)
 		return
 	}

internal/files/files.go

@@ -142,7 +142,7 @@ func entryOf(dir, name string, lst os.FileInfo) Entry {
 		} else if st.IsDir() {
 			e.Type = "dir" // keep dir semantics for navigation; Target says it's a link
 		} else {
-			e.Size = st.Size()
+			e.Size, e.Mtime = st.Size(), st.ModTime().UnixMilli() // the file's, not the link's
 		}
 	case lst.IsDir():
 		e.Type = "dir"

internal/files/move_test.go

@@ -166,3 +166,30 @@ func TestMoveSameFilesystem(t *testing.T) {
 		t.Fatalf("onto an existing file: %v", err)
 	}
 }
+
+// A link to a file carries the file's size and time, so the row and the preview key follow
+// the file; a link to a folder is a folder; a link to nothing says so.
+func TestEntryOfLinks(t *testing.T) {
+	dir := t.TempDir()
+	if err := os.WriteFile(filepath.Join(dir, "f"), []byte("12345"), 0o644); err != nil {
+		t.Fatal(err)
+	}
+	old := time.Date(2021, 1, 1, 0, 0, 0, 0, time.UTC)
+	_ = os.Chtimes(filepath.Join(dir, "f"), old, old)
+	_ = os.Mkdir(filepath.Join(dir, "d"), 0o755)
+	for _, l := range [][2]string{{"lf", "f"}, {"ld", "d"}, {"lx", "nowhere"}} {
+		if err := os.Symlink(l[1], filepath.Join(dir, l[0])); err != nil {
+			t.Fatal(err)
+		}
+	}
+	ent := func(n string) Entry { st, _ := os.Lstat(filepath.Join(dir, n)); return entryOf(dir, n, st) }
+	if e := ent("lf"); e.Type != "link" || e.Size != 5 || e.Mtime != old.UnixMilli() || e.Target != "f" || e.Broken {
+		t.Fatalf("link to file: %+v", e)
+	}
+	if e := ent("ld"); e.Type != "dir" || e.Target != "d" {
+		t.Fatalf("link to dir: %+v", e)
+	}
+	if e := ent("lx"); e.Type != "link" || !e.Broken || e.Preview != "" {
+		t.Fatalf("dangling: %+v", e)
+	}
+}

internal/media/encode.go

@@ -39,6 +39,17 @@ func qualityByID(id string) (Quality, bool) {
 	return Quality{}, false
 }
 
+// ValidChoice says whether a codec and quality are ones on offer — checked before any work.
+func ValidChoice(codec, quality string) error {
+	if !codecKnown(codec) {
+		return fmt.Errorf("unknown codec %q", codec)
+	}
+	if _, ok := qualityByID(quality); !ok {
+		return fmt.Errorf("unknown quality %q", quality)
+	}
+	return nil
+}
+
 func codecKnown(id string) bool {
 	for _, c := range Codecs {
 		if c.ID == id {

internal/media/encode_test.go

@@ -21,7 +21,7 @@ func TestFit(t *testing.T) {
 		{"original", src("hevc", 3840, 2160, 30), "h264", 3840, 2160, 20000},
 		{"720p", src("h264", 2292, 960, 60), "h264", 1718, 720, 5000},  // odd width made even
 		{"1080p", src("h264", 1280, 720, 30), "h264", 1280, 720, 4000}, // never upscaled
-		{"480p", src("h264", 1920, 1080, 30), "hevc", 852, 480, 1200}, // 853.3 rounds down, then even
+		{"480p", src("h264", 1920, 1080, 30), "hevc", 852, 480, 1200},  // 853.3 rounds down, then even
 		{"original", src("av1", 640, 360, 24), "av1", 640, 360, 1200},
 	} {
 		w, h, k := fit(q(c.q), c.in, c.codec)
@@ -117,3 +117,12 @@ func TestParseProgress(t *testing.T) {
 		t.Fatalf("got %v %v", s, sp)
 	}
 }
+
+func TestValidChoice(t *testing.T) {
+	if err := ValidChoice("h264", "720p"); err != nil {
+		t.Fatal(err)
+	}
+	if ValidChoice("vp8", "720p") == nil || ValidChoice("h264", "9p") == nil || ValidChoice("", "") == nil {
+		t.Fatal("bad choices must be refused")
+	}
+}

internal/media/ffrun.go

@@ -84,8 +84,11 @@ func (p *ffproc) end() {
 	select {
 	case <-p.done:
 	case <-time.After(5 * time.Second):
-		_ = syscall.Kill(-p.cmd.Process.Pid, syscall.SIGKILL)
-		<-p.done
+		_ = syscall.Kill(-p.cmd.Process.Pid, syscall.SIGKILL) // a root helper ignores this; it ends on its own within its own grace
+		select {
+		case <-p.done:
+		case <-time.After(10 * time.Second):
+		}
 	}
 }
 

internal/media/hls.go

@@ -47,6 +47,7 @@ type Stream struct {
 	proc     *ffproc
 	finished bool
 	failure  string
+	gone     bool // forgotten by the Streamer
 }
 
 // StreamStatus is what the player asks for while the file is still going through.
@@ -93,20 +94,25 @@ func (m *Streamer) Open(ctx context.Context, c Caps, root bool, path, codec, qua
 	if c.Error != "" {
 		return nil, errors.New(c.Error)
 	}
+	if err := ValidChoice(codec, quality); err != nil {
+		return nil, err
+	}
+	q, _ := qualityByID(quality)
 	if src.Video == nil {
 		return nil, &DecodeError{Msg: "no video stream"}
 	}
-	q, ok := qualityByID(quality)
-	if !ok {
-		return nil, errors.New("unknown quality " + quality)
-	}
-	if !codecKnown(codec) {
-		return nil, errors.New("unknown codec " + codec)
+	if src.Video.Width <= 0 || src.Video.Height <= 0 {
+		return nil, &DecodeError{Msg: "the picture's size is unknown"}
 	}
 	key := fmt.Sprintf("%s|%d|%.0f|%s|%s|%v", path, src.Size, src.Duration, codec, quality, root)
 	m.mu.Lock()
 	for _, s := range m.streams {
 		if s.key == key {
+			if s.failed() { // not the stream to hand back: let it go and make a fresh one
+				delete(m.streams, s.ID)
+				_ = os.RemoveAll(s.dir)
+				break
+			}
 			m.mu.Unlock()
 			s.touch()
 			return s, nil
@@ -152,15 +158,19 @@ func (m *Streamer) Open(ctx context.Context, c Caps, root bool, path, codec, qua
 		s.mu.Lock()
 		s.proc, s.Engine = p, r.Engine
 		s.mu.Unlock()
-		if msg, ok := s.waitFirstSegment(ctx); ok {
+		msg, ok := s.waitFirstSegment(ctx)
+		if ok {
 			go s.watch()
 			log.Printf("media: stream %s: %s → %s %s %dx%d %dk on %s", s.ID, filepath.Base(path), codec, quality, s.Width, s.Height, s.Kbps, r.Engine)
 			return s, nil
-		} else {
-			last = r.Engine + ": " + msg
-			log.Printf("media: stream %s on %s: %s", filepath.Base(path), r.Engine, msg)
 		}
 		p.end()
+		if ctx.Err() != nil || s.closed() { // the caller left, or somebody closed it meanwhile: not the recipe's fault
+			m.forget(s)
+			return nil, errors.New("cancelled")
+		}
+		last = r.Engine + ": " + msg
+		log.Printf("media: stream %s on %s: %s", filepath.Base(path), r.Engine, msg)
 	}
 	m.forget(s)
 	return nil, &DecodeError{Msg: last}
@@ -240,9 +250,20 @@ func (m *Streamer) forget(s *Stream) {
 	m.mu.Lock()
 	delete(m.streams, s.ID)
 	m.mu.Unlock()
+	s.mu.Lock()
+	s.gone = true
+	s.mu.Unlock()
 	_ = os.RemoveAll(s.dir)
 }
 
+func (s *Stream) closed() bool { s.mu.Lock(); defer s.mu.Unlock(); return s.gone }
+
+func (s *Stream) failed() bool {
+	s.mu.Lock()
+	defer s.mu.Unlock()
+	return s.finished && s.failure != ""
+}
+
 // Sweep closes what nobody has touched for a while.
 func (m *Streamer) Sweep(idle time.Duration) {
 	m.mu.Lock()