[PR #339] fix: stream ZIP downloads without buffering archives #330

New Issue

2026-06-06T22:10:15-04:00

yindo commented

2026-06-06 22:10:15 -04:00

📋 Pull Request Information

Original PR: https://github.com/vxcontrol/pentagi/pull/339
Author: @mason5052
Created: 6/5/2026
Status: 🔄 Open

Base: main ← Head: codex/issue-338-stream-zip-downloads

📝 Commits (2)

556692a fix: stream zip downloads without buffering archive
41524bd fix: return structured error when zip build fails before streaming

📊 Changes

4 files changed (+156 additions, -49 deletions)

View changed files

📝 backend/pkg/server/services/flow_files.go (+11 -26)
📝 backend/pkg/server/services/flow_files_test.go (+3 -0)
📝 backend/pkg/server/services/resources.go (+53 -23)
📝 backend/pkg/server/services/resources_test.go (+89 -0)

📄 Description

Summary

Directory and multi-path ZIP downloads built the entire archive in a bytes.Buffer before sending it, so the API process held the whole compressed archive on the heap before writing any response byte. This streams the archive straight to the HTTP response writer instead, keeping memory proportional to a single file's copy buffer rather than the full archive size.

Problem

DownloadResource (GET /resources/download) and DownloadFlowFile (GET /flows/{flowID}/files/download) both did, for every directory / multi-path request:

var buf bytes.Buffer
if err := resources.ZipResources(&buf, zipEntries); err != nil { ... }
c.DataFromReader(http.StatusOK, int64(buf.Len()), "application/zip", &buf, ...)

The full archive was materialized in buf purely so a Content-Length could be derived from buf.Len(). Combined with the 2 GB upload/pull limits, an authenticated User-role account can store (or pull) a large incompressible directory and request it as a ZIP, forcing an allocation roughly the size of the archive. A handful of concurrent requests can exhaust process/host memory and crash or severely degrade the API. The reporter's harness showed heap growth proportional to archive size through bytes.growSlice -> bytes.(*Buffer).Write -> archive/zip -> io.Copy -> resources.ZipResources.

Single-file downloads were already fine (they stream from disk); only the directory / multi-path ZIP paths were affected.

Solution

The underlying helpers (resources.ZipResources, flowfiles.ZipDirectory, flowfiles.ZipRelativePaths) already accept an io.Writer and stream each entry with io.Copy, so no archiving logic changed. A small shared helper writes the archive directly to c.Writer:

func streamZipArchive(c *gin.Context, filename string, build func(w io.Writer) error) error {
    c.Header("Content-Type", "application/zip")
    c.Header("Content-Disposition", mime.FormatMediaType("attachment", map[string]string{"filename": filename}))
    c.Status(http.StatusOK)
    if err := build(c.Writer); err != nil {
        c.Abort()
        return err
    }
    return nil
}

All four buffering call sites (two per handler) route through it. All pre-archive validation (auth, path sanitization, resource lookup, single-file 404) still runs before any byte is written and still returns normal error responses. Errors that occur after streaming has started (for example a blob that vanished from disk) are logged and the request is aborted, since the 200 status is already committed.

Filenames, archive entry names, ordering, permissions, and filtering are unchanged. The only observable differences: ZIP responses are now chunked (no Content-Length), and process memory no longer scales with archive size.

User Impact

Large directory / multi-path ZIP downloads no longer risk OOM-crashing the API; memory stays proportional to one file's copy buffer regardless of archive size.
ZIP responses use chunked transfer encoding (no Content-Length). Browsers still download normally via Content-Disposition: attachment; clients that relied on a ZIP Content-Length for a progress indicator will no longer receive one.
No API, schema, configuration, or single-file-download behavior change.

Test Plan

go build ./... (CGO_ENABLED=0) - passes.
go vet ./pkg/server/services/... - passes.
New TestStreamZipArchive (no DB dependency) - passes locally:
- streams a valid ZIP, asserts Content-Type: application/zip, asserts no Content-Length, and parses the body back into the expected entry;
- a build that fails after a partial flush propagates the error and aborts the request.
Extended TestResourceService_DownloadResourceScenarios and TestFlowFileService_DownloadFlowFileScenarios: every ZIP case now also asserts the response carries no Content-Length (locking in streaming), while keeping the existing valid-ZIP-content assertions.
The DB-backed scenario tests use a cgo SQLite driver and could not be executed in my Windows dev shell (no C compiler available; CGO_ENABLED=0). They compile cleanly and run in CI on Linux; locally they fail only at gorm.Open("sqlite3", ...), before the handler is invoked.

Closes #338

_{🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.}

## 📋 Pull Request Information **Original PR:** https://github.com/vxcontrol/pentagi/pull/339 **Author:** [@mason5052](https://github.com/mason5052) **Created:** 6/5/2026 **Status:** 🔄 Open **Base:** `main` ← **Head:** `codex/issue-338-stream-zip-downloads` --- ### 📝 Commits (2) - [`556692a`](https://github.com/vxcontrol/pentagi/commit/556692aeb13067fa75f50cb093a858ef8eb85ba1) fix: stream zip downloads without buffering archive - [`41524bd`](https://github.com/vxcontrol/pentagi/commit/41524bd07ed833b5f7545259080b5a4fb7ea702f) fix: return structured error when zip build fails before streaming ### 📊 Changes **4 files changed** (+156 additions, -49 deletions) <details> <summary>View changed files</summary> 📝 `backend/pkg/server/services/flow_files.go` (+11 -26) 📝 `backend/pkg/server/services/flow_files_test.go` (+3 -0) 📝 `backend/pkg/server/services/resources.go` (+53 -23) 📝 `backend/pkg/server/services/resources_test.go` (+89 -0) </details> ### 📄 Description ## Summary Directory and multi-path ZIP downloads built the entire archive in a `bytes.Buffer` before sending it, so the API process held the whole compressed archive on the heap before writing any response byte. This streams the archive straight to the HTTP response writer instead, keeping memory proportional to a single file's copy buffer rather than the full archive size. ## Problem `DownloadResource` (`GET /resources/download`) and `DownloadFlowFile` (`GET /flows/{flowID}/files/download`) both did, for every directory / multi-path request: ```go var buf bytes.Buffer if err := resources.ZipResources(&buf, zipEntries); err != nil { ... } c.DataFromReader(http.StatusOK, int64(buf.Len()), "application/zip", &buf, ...) ``` The full archive was materialized in `buf` purely so a `Content-Length` could be derived from `buf.Len()`. Combined with the 2 GB upload/pull limits, an authenticated `User`-role account can store (or pull) a large incompressible directory and request it as a ZIP, forcing an allocation roughly the size of the archive. A handful of concurrent requests can exhaust process/host memory and crash or severely degrade the API. The reporter's harness showed heap growth proportional to archive size through `bytes.growSlice -> bytes.(*Buffer).Write -> archive/zip -> io.Copy -> resources.ZipResources`. Single-file downloads were already fine (they stream from disk); only the directory / multi-path ZIP paths were affected. ## Solution The underlying helpers (`resources.ZipResources`, `flowfiles.ZipDirectory`, `flowfiles.ZipRelativePaths`) already accept an `io.Writer` and stream each entry with `io.Copy`, so no archiving logic changed. A small shared helper writes the archive directly to `c.Writer`: ```go func streamZipArchive(c *gin.Context, filename string, build func(w io.Writer) error) error { c.Header("Content-Type", "application/zip") c.Header("Content-Disposition", mime.FormatMediaType("attachment", map[string]string{"filename": filename})) c.Status(http.StatusOK) if err := build(c.Writer); err != nil { c.Abort() return err } return nil } ``` All four buffering call sites (two per handler) route through it. All pre-archive validation (auth, path sanitization, resource lookup, single-file 404) still runs before any byte is written and still returns normal error responses. Errors that occur after streaming has started (for example a blob that vanished from disk) are logged and the request is aborted, since the 200 status is already committed. Filenames, archive entry names, ordering, permissions, and filtering are unchanged. The only observable differences: ZIP responses are now chunked (no `Content-Length`), and process memory no longer scales with archive size. ## User Impact - Large directory / multi-path ZIP downloads no longer risk OOM-crashing the API; memory stays proportional to one file's copy buffer regardless of archive size. - ZIP responses use chunked transfer encoding (no `Content-Length`). Browsers still download normally via `Content-Disposition: attachment`; clients that relied on a ZIP `Content-Length` for a progress indicator will no longer receive one. - No API, schema, configuration, or single-file-download behavior change. ## Test Plan - `go build ./...` (`CGO_ENABLED=0`) - passes. - `go vet ./pkg/server/services/...` - passes. - New `TestStreamZipArchive` (no DB dependency) - passes locally: - streams a valid ZIP, asserts `Content-Type: application/zip`, asserts no `Content-Length`, and parses the body back into the expected entry; - a `build` that fails after a partial flush propagates the error and aborts the request. - Extended `TestResourceService_DownloadResourceScenarios` and `TestFlowFileService_DownloadFlowFileScenarios`: every ZIP case now also asserts the response carries no `Content-Length` (locking in streaming), while keeping the existing valid-ZIP-content assertions. - The DB-backed scenario tests use a cgo SQLite driver and could not be executed in my Windows dev shell (no C compiler available; `CGO_ENABLED=0`). They compile cleanly and run in CI on Linux; locally they fail only at `gorm.Open("sqlite3", ...)`, before the handler is invoked. Closes #338 --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>

yindo added the pull-request label 2026-06-06 22:10:15 -04:00

Sign in to join this conversation.

1 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: vxcontrol/pentagi#330