Stream non-HTML responses past the overlay injector (audit L-5) #147

Merged

qwc merged 1 commit from fix/overlay-streaming into main

2026-08-30 20:43:11 +02:00

qwc commented

2026-08-30 20:26:24 +02:00

Owner

Audit item L-5, filed as low urgency. Independent of #144, #145 and #146.

What it did

overlayRecorder buffered the entire response into a bytes.Buffer, then looked at the Content-Type to decide whether to inject the overlay. Non-HTML responses were collected in full and copied out unchanged — the buffering bought nothing.

It matters more than "an HTML page is small" suggests, because serveVersionDoc routes anything extension-less through here:

maybeHTML := filePath == "" || strings.HasSuffix(filePath, "/") ||
    strings.HasSuffix(filePath, ".html") || strings.HasSuffix(filePath, ".htm") ||
    !strings.Contains(filePath, ".")

A file named download or manual inside someone's docs is served by that last clause and can be any size, so the buffer had no useful bound.

What it does now

The recorder decides as soon as the handler commits to a status — ServeContent sets Content-Type before writing — and from then on a non-injectable response goes straight to the client. HTML is still buffered; the overlay has to go in before </body>, so the page genuinely has to be held.

Two smaller things came with it:

A response that isn't a plain 200 now streams too, even when it says text/html. A 206 carries a byte range, and injecting an overlay into part of a file corrupts it — that was possible before. The visible change is that a 404 page served from a docs directory no longer gets the overlay; a broken link inside docs shows the project's own 404 rather than a framed one.
Flush passes through once streaming has started, so a slow large file isn't held by us. While buffering it stays a no-op, since nothing can be released before the overlay goes in.

The old header-copying loop is gone: Header() returns the embedded writer's own map, so it was copying a map onto itself.

Tests

Three added to internal/docs/overlay_test.go: a probe writer asserting a PDF's three writes arrive as three writes rather than one buffered copy, a 206 keeping its status and staying un-injected, and HTML still getting the overlay before </body> with the stale Content-Length dropped. The existing overlay tests — HTML injection, non-HTML passthrough, images, status preservation — pass unchanged.

Go suite and the 8 frontend tests green.

Assisted by Claude Opus 5.

Audit item **L-5**, filed as low urgency. Independent of #144, #145 and #146. ## What it did `overlayRecorder` buffered the entire response into a `bytes.Buffer`, *then* looked at the Content-Type to decide whether to inject the overlay. Non-HTML responses were collected in full and copied out unchanged — the buffering bought nothing. It matters more than "an HTML page is small" suggests, because `serveVersionDoc` routes anything extension-less through here: ```go maybeHTML := filePath == "" || strings.HasSuffix(filePath, "/") || strings.HasSuffix(filePath, ".html") || strings.HasSuffix(filePath, ".htm") || !strings.Contains(filePath, ".") ``` A file named `download` or `manual` inside someone's docs is served by that last clause and can be any size, so the buffer had no useful bound. ## What it does now The recorder decides as soon as the handler commits to a status — `ServeContent` sets Content-Type before writing — and from then on a non-injectable response goes straight to the client. HTML is still buffered; the overlay has to go in before `</body>`, so the page genuinely has to be held. Two smaller things came with it: - **A response that isn't a plain 200 now streams too, even when it says `text/html`.** A 206 carries a byte range, and injecting an overlay into part of a file corrupts it — that was possible before. The visible change is that a 404 page served from a docs directory no longer gets the overlay; a broken link inside docs shows the project's own 404 rather than a framed one. - **`Flush` passes through once streaming has started**, so a slow large file isn't held by us. While buffering it stays a no-op, since nothing can be released before the overlay goes in. The old header-copying loop is gone: `Header()` returns the embedded writer's own map, so it was copying a map onto itself. ## Tests Three added to `internal/docs/overlay_test.go`: a probe writer asserting a PDF's three writes arrive as three writes rather than one buffered copy, a 206 keeping its status and staying un-injected, and HTML still getting the overlay before `</body>` with the stale `Content-Length` dropped. The existing overlay tests — HTML injection, non-HTML passthrough, images, status preservation — pass unchanged. Go suite and the 8 frontend tests green. --- Assisted by Claude Opus 5.

qwc added 1 commit

2026-08-30 20:26:24 +02:00

Stream non-HTML responses past the overlay injector (audit L-5)

CI / test (pull_request) Successful in 1m19s

Details

CI / build (pull_request) Successful in 48s

Details

CI / docker (pull_request) Has been skipped

Details

cad7f16acc

The overlay recorder collected every response in memory before deciding
whether it was HTML, so anything served through this path was buffered
in full first. That path also handles extension-less URLs, which can be
any file at all, so the buffer had no useful upper bound.

The recorder now decides as soon as the handler commits to a status and
streams anything that is not an injectable HTML page straight to the
client. HTML is still buffered — the overlay has to go in before
</body>, so there is no way around holding the page.

A response that is not a plain 200 also streams now, even when it says
text/html: a 206 carries a byte range, and injecting an overlay into
part of a file corrupts it. Flush passes through once streaming has
started.

Assisted-by: Claude Opus 5

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Signed-off-by: Marcel M. Otte <marcel.otte@mmo.to>

qwc merged commit 37ce3b7d1d into main

2026-08-30 20:43:11 +02:00

qwc deleted branch fix/overlay-streaming

2026-08-30 20:43:11 +02:00

qwc referenced this pull request from a commit

2026-08-30 20:43:13 +02:00

Merge pull request 'Stream non-HTML responses past the overlay injector (audit L-5)' (#147) from fix/overlay-streaming into main