Preview opens file descriptor before admission and waits on an uncancelable context, pinning fds/goroutines

Status

Repository / Component

Plain-English Description

When generating an image preview, the app opens the file first and then waits for a free worker slot using a wait that ignores the client hanging up. Pending or abandoned requests keep holding open files and background tasks that nothing releases, so many at once can exhaust server resources.

Description of the Underlying Issue

createPreview (http/preview.go:113-119) opens file.Fs.Open(file.Path) before any admission control, and defer fd.Close() holds that descriptor for the entire call. It then calls Resize with context.Background() (preview.go:141) instead of r.Context(). Inside Resize, the first step is sem.Acquire(ctx,1) on the worker semaphore (img/service.go:147-151); with a Background context the acquire blocks with no deadline and ignores client disconnect while the fd is already open. With imageProcessors default 4 (cmd/root.go:93) the queue drains slowly, and the server sets only ReadHeaderTimeout — no WriteTimeout/IdleTimeout/TimeoutHandler (cmd/root.go:246-249) — so nothing reaps a stalled preview.

Potential Attack

A user opens many concurrent preview requests beyond the 4 worker slots (optionally disconnecting immediately). Each request opens a file descriptor and a goroutine that blocks on the Background-context semaphore acquire, which client disconnect cannot cancel and no server timeout reaps.

Outcomes of Potential Attack

Accumulation of held file descriptors and blocked goroutines that outlive the client connection, potentially reaching fd/goroutine exhaustion and denial of service for all users. Magnitude depends on host fd limits and connection volume.

Affected Scope

GET /api/preview for any Perm.Download user: the source fd is opened before admission control and the decode uses an uncancelable context, so pending requests pin fds/goroutines.

Suggested Fix (plain english)

Wait for a free worker slot before opening the file, use the request's own context so a client hang-up cancels the wait, and add server write/idle timeouts to reap stalled requests.

Suggested Fix (detailed)

Acquire the worker semaphore before opening the fd, and thread r.Context() into Resize/sem.Acquire (preview.go:141, service.go:147-151) so client disconnect cancels the wait and releases the goroutine. Reorder createPreview (preview.go:113-141) to open the source file only after admission. Add WriteTimeout and IdleTimeout (or a TimeoutHandler) to the server config (cmd/root.go:246-249). Verify by opening many concurrent previews, disconnecting, and confirming fds and goroutines are released promptly. Thread r.Context() into Resize/sem.Acquire so disconnect cancels the wait; acquire the semaphore before opening the fd; add server write/idle timeouts.

Validation

The fd-before-admission + uncancelable Background-context acquire is proven; the fd/goroutine exhaustion magnitude is gated on host limits and connection volume, so possibly_valid. Canonical of the pair (100062).

first_opinion

Full Evidence

http/preview.go:113-119 createPreview opens file.Fs.Open(file.Path) first, before any admission control; defer fd.Close() holds it for the whole call

http/preview.go:141 Resize(context.Background(), fd, ...) — request context deliberately not threaded

img/service.go:147-151 Resize's first step is sem.Acquire(ctx,1) with the Background ctx → blocks with no deadline and ignores client disconnect while the fd is already open

cmd/root.go:93 imageProcessors default 4 → slow drain

cmd/root.go:246-249 only ReadHeaderTimeout; no WriteTimeout/IdleTimeout/TimeoutHandler

Proven fact: The file descriptor is opened before the worker-semaphore admission, and the semaphore acquire uses context.Background(), so a pending preview holds an fd + goroutine that client disconnect cannot release — complete in source

Proven fact: No server write/idle timeout reaps stalled preview requests

Unvalidated fact: Whether this reaches fd/goroutine exhaustion depends on host ulimits, connection count, and any fronting proxy connection caps — runtime facts

http/preview.go:113-119 createPreview opens file.Fs.Open(file.Path) as its first action, before any admission control; defer fd.Close() holds it for the call's full duration

http/preview.go:141 Resize(context.Background(), fd, ...) — request context deliberately not threaded through

img/service.go:147-151 Resize does s.sem.Acquire(ctx,1) as its first step; the fd is already open and the acquire blocks on the (Background) ctx before any work

img/service.go:36-40 + cmd/root.go:93 semaphore width = imageProcessors, default 4

cmd/root.go:246-249 http.Server{Handler, ReadHeaderTimeout:60s} — no WriteTimeout/ReadTimeout/IdleTimeout and no http.TimeoutHandler wrap on the preview route

http/data.go:50-85 handle() has no panic recover, no concurrency cap, no queue bound; http/http.go:30-35 only global middleware sets CSP