feat(export): Zip64 for direct download, S3 PUT capped at 5 GiB - #1481
Merged
Conversation
Follow-up to #1475. The original implementation pinned the bundle ceiling at 4 GiB and explicitly disabled Zip64 in both ZipStream constructors. The reasoning was consumer-tool compatibility, but: - Windows 10+ Explorer, macOS Archive Utility, every cloud-console previewer, and every CLI `unzip` since ~2008 handle Zip64 reliably. - For the s3-upload arm the binding constraint is the 5 GiB single-PUT cap that AWS S3, Cloudflare R2, MinIO, Backblaze B2, and Wasabi share (multipart upload above 5 GiB is a fundamentally different flow than presigned single-PUT). So 4 GiB was the wrong number on both arms: too low for direct download (operators with a large demo archive get gated out of the panel-internal flow), and not the load-bearing constraint for S3 anyway. This PR makes the cap mode-conditional: - Drops `enableZip64: false` from both ZipStream constructors; the v3.x library default (`enableZip64: true`) takes over. - Renames `Manifest::MAX_BUNDLE_BYTES` (4 GiB) to `MAX_S3_PUT_BYTES` (5 GiB) and reframes the constant's docblock around the S3 single- PUT limit. - Adds `Manifest::s3PutCapBytes()` + `Manifest::computeExceedsCap()` as pure static helpers so the cap math has a single source AND the boundary predicate is unit-testable without a 5 GiB DB fixture. - `BundleWriter` gains a nullable `?int $capBytes` ctor arg. ZIP mode passes `null` (the running-byte gate no-ops); S3 mode passes `Manifest::s3PutCapBytes()` so the in-flight gate stays armed as defence-in-depth against pre-flight estimate undershoot. - `ManifestBuilder` drops `buildOrThrow()`; the s3-mode short-circuit moves to `web/export.php` where it can be conditionally applied. - The admin form's ZIP submit button is always enabled (uncapped); the S3 submit stays gated on `$exceeds_cap`. The empty-state copy that paints when over the S3 cap points the operator at "use ZIP download instead" as the escape hatch. - Cap-error toast / docs troubleshooting copy aligned around the same story: "5 GiB S3 PUT limit; use direct ZIP download (uncapped) or prune data and retry". `manifest.json` wire format is preserved (no `format_version` bump per the design plan's Option B). `cap_bytes` and `exceeds_cap` retain their field names but now refer specifically to the S3 PUT cap; they're informational on a direct-ZIP-download bundle's manifest. No new permission flag, no schema change. Validation: PHPStan green, 48/48 Export+Manifest+S3Presigned tests green, 6/6 admin-data-export E2E tests green. Adversarial review findings folded in (Manifest cap-predicate extraction + boundary unit tests, docblock drift sweeps on ExportError, AGENTS.md / ARCHITECTURE.md ordering and terminology fixes, MiB/GiB unit normalisation on the admin form stats grid).
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Relaxes the data-export feature's hardcoded 4 GiB ZIP 2.0 cap. Caps are now mode-conditional:
mode=zip): no cap. Zip64 is enabled (the v3.xmaennchen/zipstream-phpdefault), so the archive itself has no structural size ceiling. Operators with a large demo archive can stream a multi-tens-of-GB bundle if their browser / network tolerates it.mode=s3): hard cap at 5 GiB (minus a 64 MiB safety margin). This is the structural S3 single-PUT object-size limit shared by AWS S3, Cloudflare R2, MinIO, Backblaze B2, and Wasabi — multipart upload above 5 GiB is a fundamentally different flow than presigned single-PUT, and the panel doesn't speak it.Motivation and Context
Follow-up to #1475. The original implementation pinned the ceiling at 4 GiB on both arms and explicitly disabled Zip64. The reasoning was consumer-tool compatibility, but:
unzipsince ~2008 handle Zip64 reliably.So 4 GiB was the wrong number on both arms: too low for direct download (operators with a large demo archive get gated out of the panel-internal flow with no escape hatch), and not the load-bearing constraint for S3 anyway.
What changed
enableZip64: falsefrom bothZipStreamconstructors inweb/export.phpandweb/tests/integration/ExportBundleWriterTest.php.Manifest::MAX_BUNDLE_BYTES(4 GiB) toMAX_S3_PUT_BYTES(5 GiB); reframes the constant's docblock around the S3 single-PUT limit.Manifest::s3PutCapBytes()+Manifest::computeExceedsCap()as pure static helpers. Single source for the cap math, AND the boundary predicate is now unit-testable at every transition without spinning up a 5 GiB DB fixture.BundleWritergains a nullable?int $capBytesctor arg. ZIP mode passesnull(the running-byte gate no-ops); S3 mode passesManifest::s3PutCapBytes()so the in-flight gate stays armed as defence-in-depth against pre-flight estimate undershoot.ManifestBuilderdropsbuildOrThrow(); the s3-mode short-circuit moves toweb/export.phpwhere it can be conditionally applied.$exceeds_cap. The empty-state copy that paints when over the S3 cap points the operator at "use ZIP download instead" as the escape hatch.Wire format preserved
manifest.json'sformat_versionstays at1(per the design plan's Option B).cap_bytesandexceeds_capretain their field names but now refer specifically to the S3 PUT cap. They're informational on a direct-ZIP-download bundle. No new permission flag, no schema change, no paired updater migration.Downstream-consumer note
Tooling that reads
manifest.json'scap_bytes/exceeds_capas "this is the panel's bundle ceiling" should re-read them as "this is the S3 PUT cap; ZIP download is uncapped". The fields are still present and still typed the same way, so the parse contract is byte-stable; only the semantic meaning has shifted.How Has This Been Tested?
./sbpp.sh phpstan— green (252 files, no errors)../sbpp.sh test --filter='Export|Manifest|S3Presigned'— 48/48 tests green (619 assertions).ManifestBuilderTest::testS3PutCapBytesReturnsConstantMinusSafetyMargin+testComputeExceedsCapBoundaryMatrix(covers0/< cap/== cap/> cap/>> captransitions).ManifestBuilderTest::testManifestExposesExceedsCapFlagreshaped to drive the helper../sbpp.sh e2e --grep='admin data export'— 6/6 tests green (desktop + mobile chromium): admin page renders, ZIP download streams + parses as Zip64, manifest-first contract holds,format_version: 1+ valid UUIDv4 +pii_policy.password_hashes: \"never\"attestation present, GET to/export.phpstill returns 405.Adversarial review
A reviewer pass flagged 4 MEDIUM + 5 LOW findings (no CRITICAL / HIGH). Addressed in-PR:
AGENTS.mdclaimed the entry point checks permission "BEFORE the CSRF gate even runs". Code runs CSRF first (line 101), then permission gate (line 109). Convention text fixed.AGENTS.mddocumented thememory_limitbump as covering "ZIP-mode in-flight encryption buffers"; the panel never encrypts the bundle (no ZipStream encryption parameter, no form control, docs recommend HTTPS / S3 SSE for at-rest). Reworded as "in-flight DEFLATE compression workspace".ManifestBuilderTest::testManifestExposesExceedsCapFlagwas a tautology (it constructed Manifest DTOs with hardcoded flag values rather than driving the predicate). Extracted the cap math toManifest::s3PutCapBytes()+Manifest::computeExceedsCap()and added a boundary matrix test.ExportErrordocblock +ARCHITECTURE.md+AGENTS.mdcitedpublic readonly string $codebut the actual property is$errorCode(the parent\\Exception::$codeisint-typed and can't be narrowed). All three docblocks fixed to reference$errorCode+ thecode()accessor.BundleWriter::checkCaperror message hardcoded a "5 GiB" string prefix. Swapped for "MAX_S3_PUT_BYTES" so a future cap-value change doesn't desync the message.estimated_bundle_bytescould be misread as the field being s3-mode-only (it's unconditional). Tightened.sbpp_export_redirect_failure()has a dead\$contextparam) intentionally deferred — pre-existing dead code, out of scope for the Zip64 rework.Types of changes
The
manifest.jsonwire format is preserved (noformat_versionbump, no field-name changes); downstream consumers that readcap_bytes/exceeds_capget the same shape but the semantic meaning has shifted from "the bundle ceiling" to "the S3 PUT ceiling". This is a soft semantic change, not a structural break.Checklist
docs/src/content/docs/configuring/data-export.mdx,AGENTS.md,ARCHITECTURE.md).