feat(dgw): zip multi-clip recording downloads - #1923
Conversation
Add GET /jet/jrec/pull/{id} that streams a ZIP of recording.json and every
clip listed in the session manifest. Callers no longer need to guess a
single clip filename when a reconnect produced multiple files. The
existing per-file pull route is unchanged for player and granular access.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Let maintainers know that an action is required on their side
|
|
Implementation notes:
Note LLM-assisted content (no human feedback). |
Map missing or corrupt manifests to 404, require pull operation tokens, fail the HTTP body on mid-stream packaging errors, and stop ZIP work on gateway shutdown so clients do not treat truncated archives as success. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
|
Addressed code-review findings in
Note LLM-assisted content (no human feedback). |
There was a problem hiding this comment.
Pull request overview
Adds authenticated ZIP downloads for complete JREC sessions, including the manifest and all available clips.
Changes:
- Adds the session ZIP streaming endpoint.
- Adds ZIP generation and validation tests.
- Regenerates OpenAPI documentation and clients.
Reviewed changes
Copilot reviewed 9 out of 10 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
devolutions-gateway/src/openapi.rs |
Registers the endpoint in OpenAPI. |
devolutions-gateway/src/api/jrec.rs |
Implements ZIP creation and streaming. |
devolutions-gateway/openapi/ts-angular-client/api/jrec.service.ts |
Adds the Angular client method. |
devolutions-gateway/openapi/gateway-api.yaml |
Documents the endpoint contract. |
devolutions-gateway/openapi/dotnet-client/src/Devolutions.Gateway.Client/Api/JrecApi.cs |
Adds .NET client methods. |
devolutions-gateway/openapi/dotnet-client/README.md |
Lists the new .NET API. |
devolutions-gateway/openapi/dotnet-client/docs/JrecApi.md |
Documents .NET usage. |
devolutions-gateway/openapi/doc/index.adoc |
Adds generated API documentation. |
devolutions-gateway/Cargo.toml |
Adds async_zip. |
Cargo.lock |
Locks the new dependency graph. |
💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| return Err(HttpError::not_found().msg("requested recording does not exist")); | ||
| } | ||
|
|
||
| let entries = match list_recording_zip_entries(&recording_dir).await { |
There was a problem hiding this comment.
Fixed in 2013b996.
Download membership is now a one-shot snapshot (RecordingZipPlan): we read recording.json once, derive the clip list from those bytes, and archive those exact bytes as the ZIP’s recording.json (never re-open the live file). A reconnect that rewrites the on-disk manifest after the snapshot cannot widen or desync the package. Clips are still opened once at package time from that frozen name list.
Added regression test zip_keeps_snapshotted_manifest_when_disk_manifest_changes.
Note
LLM-assisted content (no human feedback).
| let builder = ZipEntryBuilder::new(file_name.clone().into(), Compression::Stored); | ||
| let mut entry_writer = zip_writer | ||
| .write_entry_stream(builder) |
There was a problem hiding this comment.
Fixed in 2013b996.
Replaced async_zip streaming entries with the standard zip crate writing STORED entries to a temp file (known sizes/CRC in local headers), then streaming that finished archive. This avoids data-descriptor / null-CRC local headers that some OS unzippers reject.
Interop coverage: streams_interoperable_zip_with_all_listed_clips round-trips through zip::ZipArchive (not the writer’s own reader).
Note
LLM-assisted content (no human feedback).
Refuse session package pulls that exceed 128 files or 2 GiB uncompressed with HTTP 413 so pathological manifests cannot pin the gateway on huge bulk transfers while normal multi-clip sessions stay unaffected. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Address review feedback on session package downloads: freeze the recording.json bytes that define membership so reconnects cannot widen the archive mid-download, and build STORED entries with the zip crate so local-file headers carry real sizes/CRCs accepted by common unzip tools. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
e34e0a4
into
master
Add GET /jet/jrec/pull/{id} that streams a ZIP of recording.json and every clip listed in the session manifest. Callers no longer need to guess a single clip filename when a reconnect produced multiple files. The existing per-file pull route is unchanged for player and granular access.