perf(video): stream uploads into a single on-disk copy

[lstein]

Summary

Follow-on to #9163 (deferred non-merge-blocker), and the last of that PR's deferred list.

Declaring file: UploadFile makes Starlette parse the multipart body into its own spooled temp file before the route body runs. The route then copied that into a NamedTemporaryFile, because ffmpeg and videos.create need a real path — and a rolled-over SpooledTemporaryFile is an unlinked anonymous file with no path to reuse.

So every in-flight upload held two full-size copies in temp storage — up to 2 × MAX_UPLOAD_SIZE × MAX_CONCURRENT_VIDEO_UPLOADS = 4 GB — for the whole probe/thumbnail/create phase. #9163 only shrank the overlap by closing the spool right after the copy loop.

Approach

The route now parses the body itself with python_multipart's streaming parser (already a FastAPI dependency — this adds no new packages), writing the file part directly into the single temp file and buffering only the small metadata field, under its own 1 MB cap. Peak temp usage per upload is halved.

Two behavioral improvements fall out of streaming rather than spooling:

• The filename/MIME gate fires from the part headers, so an unsupported file is rejected before any of its bytes reach the disk — previously the whole body was spooled first, then rejected.
• MAX_UPLOAD_SIZE is enforced as bytes arrive, not after they've all landed.

Parsing runs in the thread pool, since the callbacks write to disk.

API contract

The request schema is pinned with openapi_extra reproducing exactly what the file + metadata parameters generated. The only difference in the regenerated schema.ts is that the body type is now inline instead of a Body_upload_video component — nothing references that component (the frontend builds its FormData by hand via UploadVideoArg), and the field names, types and requiredness are unchanged.

metadata JSON validation now runs after the body has streamed rather than before, since the field can legally arrive after the file part. Rejection is identical from the caller's point of view and still happens before videos.create.

Testing

New tests in test_video_upload_limits.py, driving the route with a chunked multipart body: single-copy write-through (byte-exact, in order), rejection from the part headers without consuming the rest of the body, missing-file-part → 422, oversized file part rejected mid-stream after ~the cap rather than the whole body, and the existing disk-full cleanup test ported to the new signature. test_configured_upload_slots_bound_peak_double_spool_usage becomes ..._peak_temp_storage_usage and now asserts the one-copy budget.

The end-to-end multipart paths (malformed MP4 → 415, spoofed container → 415, malformed metadata → 422, valid metadata → 201) are already covered in test_videos_multiuser.py and pass unchanged.

pytest tests/app/api tests/app/routers — 654 passed. Frontend lint:tsc / lint:prettier clean.

🤖 Generated with Claude Code

[JPPhoto]

Merge blockers:

• invokeai/app/api/routers/videos.py:406-419: A denied or unknown board_id raises before _stream_video_upload() reads request.stream(). The upload middleware has already incremented its global and per-user counters, but the route returns immediately and its finally releases those counters while a chunked client can keep sending the body, defeating the 429, idle-timeout, and duration accounting. Test: drive the ASGI stack with the maximum number of forbidden-board uploads whose receive() keeps yielding slow http.request chunks, then start another upload; it must remain 429 until the first requests disconnect, but this route returns 403 and releases the slots.

Other findings/issues:

• invokeai/app/api/routers/videos.py:299-305: _stream_video_upload() calls MultipartParser.finalize() and checks only saw_file_part; python-multipart finalization does not verify that the parser reached its end state. A body ending immediately after file bytes, without the closing multipart boundary, therefore returns saw_file_part=True and proceeds to MP4 validation/probe/create as if complete. This accepts malformed or interrupted uploads and can persist a truncated file when later media checks happen to pass. Test: send a valid file part with Content-Type: multipart/form-data; boundary=b but omit --b--, then assert a 400/422 response and that videos.create is not called; current parser state is PART_DATA, yet the route continues.

Suggestions:

• Consider explicitly aborting/draining rejected request bodies while retaining the middleware lease; this preserves early authorization rejection without a slow-upload quota hole.

• Consider a sink-backed Starlette multipart parser or explicit parser-contract tests; this would reduce custom parsing logic and prevent future API/schema drift.

[lstein]

Thanks — both blockers were real. Addressed in 38dd083, though the first one differently than suggested, for a reason worth spelling out.

Blocker 1 — leases released while the client is still sending

Confirmed, and it's a regression this PR introduced: with file: UploadFile FastAPI parsed the whole body before the route body ran, so the board check could never answer early.

I tried the suggested drain first and measured it. It doesn't work, for two independent reasons:

It can't cover the paths that matter. video_category and is_intermediate are required Query params, so RequestValidationError fires in solve_dependencies — before upload_video is entered at all. Driving the real stack:

valid query (control):      status=415  body chunks read at response: 20/20
video_category=bogus:       status=422  body chunks read at response:  0/20
missing query params:       status=422  body chunks read at response:  0/20

So POST /api/v1/videos/upload?video_category=bogus&is_intermediate=false reproduces the original complaint verbatim, and no amount of route-level draining reaches it. The invariant can't be established from inside the handler.

And it's a worse DoS than the hole. MAX_CONCURRENT_VIDEO_UPLOADS = 2, max_upload_duration_seconds = 1800, and in single-user mode per_user_key is None so the per-user cap of 1 doesn't apply. With the drain in place, two connections sending Content-Type: text/plain and one byte every 100s — about 16 bytes of traffic each — pin both global slots for half an hour:

	_active during attack	victim's upload
with drain	2	429
without	0	served

The thing being protected (an unread connection) is not scarce; upload slots are.

What I did instead: VideoUploadLimitASGIMiddleware now adds Connection: close to any response sent before the body has been read to completion. That ends the client's in-flight upload along with the response, so there's nothing left to account for — O(1) instead of a 30-minute slot hold — and because it lives in the middleware it covers its own 401/413/429s and every early answer beneath it, FastAPI's query validation included. Verified against a live uvicorn (--http h11): the attack request now gets connection: close and dies with a broken pipe after one 64 KiB chunk, _active stays 0, and the victim's upload is served.

Two details that are load-bearing:

• It is deliberately not seeded from Content-Length. h11 accepts Content-Length: 0 alongside Transfer-Encoding: chunked (chunked framing wins), so trusting the header let a client suppress the close and then stream freely with no lease held — I had that bug in an intermediate version and measured 13 MB streamed before catching it.
• The byte-cap, idle-timeout and duration-cap aborts leave the flag alone, since they fire precisely because the client is still uploading.

Uploads that finish sending keep their connection — verified live, two uploads over one socket, no close header.

Worth flagging: behind a body-buffering reverse proxy (nginx's default, per the multi-user admin guide) Connection is hop-by-hop, so this only closes the proxy↔app hop. There the proxy absorbs the whole upload before the app is invoked, so the hole doesn't arise in the first place. All of this is in the class docstring.

Blocker 2 — finalize() doesn't verify the end state

Confirmed against the pinned python_multipart 0.0.32: MultipartParser.finalize() is literally pass with a # TODO: verify that we're in the state MultipartState.END. The on_end callback now feeds a saw_end flag, checked after finalize; a body with no closing boundary is 422 and never reaches the probe or videos.create.

This also closes an adjacent hole found while tracing it: max_size silently truncates rather than raising, so an over-cap body previously produced a short file that looked complete.

I fuzzed for false rejections before trusting it — on_end fires at the second hyphen of --boundary--, so no trailing CRLF is needed: closing delimiter with and without CRLF, LF-only, epilogue, trailing whitespace, extra parts after the file, empty file part, file data containing a near-boundary, and every single split offset of the body into two chunks. Zero false 422s.

Suggestions

A sink-backed Starlette parser isn't available — MultiPartParser hardcodes SpooledTemporaryFile with no injection point — so I took the other half: test_python_multipart_contract_the_upload_route_depends_on pins the three library behaviours the hand-rolled parsing rests on (on_end fires exactly once at the closing boundary; finalize() does not validate the end state; max_size truncates rather than raising), so a version bump fails loudly instead of silently changing upload behaviour.

Also fixed while in here

• A malformed multipart body was an unhandled 500; now 422.
• A client-side disconnect (including the middleware's own byte/idle/duration aborts) was a 500 raised above the middleware, which both misreported a client abort as a server fault and bypassed the connection-close handling; now 400.
• Content-Type media type compared case-insensitively — MULTIPART/FORM-DATA is legal per RFC 7231 and was a 422.
• The MAX_CONCURRENT_VIDEO_UPLOADS comment still claimed two temp copies per upload.

Testing

tests/app/api tests/app/routers — 695 passed. Every production line added here was mutation-tested: reverting any one of the twelve individually turns exactly one test red, including the Content-Length seed, the HTTP/2 guard, the duplicate-connection-header filter, and each of the three abort paths. Two findings in this round came from mutation testing rather than review, so it earned its keep.

[JPPhoto]

Merge blockers:

• invokeai/app/services/image_files/image_files_disk.py:485-497: Recovery checks exists() before restoring staged files, then drops the journal without rechecking. A concurrent delete can remove the record and purge while files remain staged; recovery then restores orphan files and deletes the only journal. Effect: permanent unreferenced image/thumbnail. Likelihood: Low, but reachable with shared output storage. Recovery: restart cannot find it. Test: race exists() before restore, delete the record, then assert no files remain. Docs: docs/src/content/docs/features/gallery.mdx promises startup cleanup.

Other findings/issues:

• invokeai/app/api/routers/images.py:239-252: ImageRecordNotFoundException from images.delete() becomes HTTP 500. If another request deletes the image after DTO lookup, the requested postcondition is already satisfied but the client receives a failure. Effect: false error/toast and retry churn. Likelihood: Medium with double-clicks or multiple clients. Test: delete between get_dto() and service delete(); expect 404 or idempotent success.

Suggestions:

• Instead of manual recovery restore, reuse rollback_delete() or recheck record existence after restoring and before removing the journal.
• Consider catching ImageRecordNotFoundException around service deletion and treating it as 404/idempotent success. Focused storage/service/record tests passed; router run did not complete under xdist, and the context collector could not reach GitHub API.

[lstein]

The 2026-08-29 review on this PR is the round-4 review of #9361 (transactional image deletion) — every cited path (image_files_disk.py, images.py, gallery.mdx) is in that PR's diff and none are in this one, and the text is identical to the review posted there 17 minutes earlier. Both findings are addressed on #9361 (see cdf8458 there). Nothing changes here; this PR is still at d6dc37b awaiting re-review of the two blockers fixed in 38dd083.

[JPPhoto]

Sorry about the earlier snafu. Here's a proper review. I am approving this and the following can be noted and either dealt with later on or ignored.

There are no merge blockers but there is a slight potential for a hostile attack:

• invokeai/app/api/routers/videos.py:218,290-297: Tiny multipart chunks create one retained bytes object per metadata chunk. A 1 MiB field sent one byte at a time retains about 42 MiB in CPython; two concurrent uploads can add about 84 MiB. Effect: avoidable memory pressure or OOM. Likelihood: plausible hostile edge. Recovery: automatic after part completion, but OOM is unrecoverable. Test: feed 1 MiB metadata one byte per parser.write() and measure allocations.

Suggestions:

• Instead of storing metadata in list[bytes], append to a bounded bytearray; this keeps the 1 MiB limit aligned with actual memory use and prevents per-chunk object amplification.