Repository navigation
Commit f0cc16e
fix(service-storage): presigned S3 PUT URLs no longer carry the CRC32 of an empty body (#21164)
Fixes #21147
Clause-②: no
## What changed
The S3 adapter's `S3Client` now sets `requestChecksumCalculation` and
`responseChecksumValidation` to `WHEN_REQUIRED`
(`packages/services/service-storage/src/s3-storage-adapter.ts`,
`getClient()`). Before, the SDK default (`WHEN_SUPPORTED`) put
`x-amz-checksum-crc32=AAAAAA==` (the CRC32 of an empty body) and
`x-amz-sdk-checksum-algorithm=CRC32` into the signed query of every
presigned PUT, because a presign prepares the command before any byte
exists. A store that enforces a query-signed checksum against the
uploaded body would refuse every browser upload while server-side
`upload()` kept working.
Landing is exactly the card's file surface: the producer is this
adapter, no other package builds an `S3Client`.
| File | Change |
|---|---|
| `packages/services/service-storage/src/s3-storage-adapter.ts` | the
two client options plus the reason |
|
`packages/services/service-storage/src/s3-storage-adapter.presign-checksum.test.ts`
| new offline pin, real SDK |
| `.changeset/21147-s3-presign-no-checksum.md` | `patch` for
`@objectstack/service-storage` (not private; `dist` ships the option,
grep hit in `dist/index.js` and `dist/index.cjs`) |
3 files, +164 / -1.
## Evidence
**Red before, green after.** The new test drives the real `S3Client` and
the real presigner, fully offline: fake credentials, fixed endpoint
`http://127.0.0.1:9000`, no socket opened. A test that mocks
`@aws-sdk/client-s3` (as `s3-storage-adapter.key-prefix.test.ts` does)
cannot see this defect, because the checksum is stamped by the SDK's own
middleware.
- On `origin/main`'s adapter (base `70dae533`), the new file: 2 failed.
- upload case: `expected [ 'x-amz-checksum-crc32', …(1) ] to deeply
equal []`, received `x-amz-checksum-crc32`,
`x-amz-sdk-checksum-algorithm`.
- download case: `expected [ 'x-amz-checksum-mode' ] to deeply equal
[]`.
- With the fix: `Tests 2 passed (2)`; whole package `pnpm --filter
@objectstack/service-storage test`: `Test Files 41 passed (41)`, `Tests
629 passed (629)`; `typecheck` (including the test-layer check) exit 0.
**Ablation of the pin** (committed fix at HEAD, each leg through
`scripts/ablation-replace.mjs`: anchor hit once, blob hash moved,
restore proven by blob equal to HEAD and an empty `git diff HEAD`). The
subject resolves to `src/`, not a dependency's `dist/`, so no rebuild
leg applies.
| Removed | upload case | download case |
|---|---|---|
| both options | red (`crc32`, `sdk-checksum-algorithm`) | red
(`checksum-mode`) |
| `requestChecksumCalculation` only | red | red (`crc32` on the GET URL)
|
| `responseChecksumValidation` only | green | red (`checksum-mode`) |
**The two options are a pair.** Presigning with `@aws-sdk/client-s3`
3.1090.0 under each combination:
| request / response | PUT URL | GET URL |
|---|---|---|
| default / default | crc32, sdk-checksum-algorithm | checksum-mode |
| WHEN_REQUIRED / default | none | checksum-mode |
| default / WHEN_REQUIRED | crc32, sdk-checksum-algorithm | crc32 (worse
than default) |
| WHEN_REQUIRED / WHEN_REQUIRED | none | none |
Setting only the response option puts a CRC32 on a GET URL that had
none, so the pair is set together and the test header records the table.
**What changes for `upload()` and the other doors** (measured against a
loopback server over both plain HTTP and a self-signed HTTPS listener,
same result): a server-side `PutObject` with a Buffer body and an
`UploadPart` no longer send `x-amz-sdk-checksum-algorithm` /
`x-amz-checksum-crc32`; `x-amz-content-sha256` (the SigV4 payload hash)
is identical before and after, and the request still succeeds. A
`GetObject` no longer sends `x-amz-checksum-mode: ENABLED`, so
`download()` stops asking for and validating a response checksum.
`getPresignedDownload` loses the `x-amz-checksum-mode=ENABLED` query
parameter. This is the pre-flexible-checksum behaviour; the SDK's
default added it. None of the nine commands the adapter issues is
checksum-required: in `@aws-sdk/client-s3` 3.1090.0
`requestChecksumRequired: true` is bound to bucket-config, ACL, tagging,
retention, `DeleteObjects` and similar commands, and `PutObject`,
`GetObject`, `DeleteObject`, `HeadObject`, `ListObjectsV2` and the four
multipart commands are not among them. Peer range is `^3.0.0`: SDKs
older than the flexible-checksum release ignore the two options and
never stamped a checksum.
## Local verification (HEAD `33e30baf`)
- Dependency closure built first: `pnpm --workspace-concurrency=2
--filter '@objectstack/service-storage^...' build` exit 0 (direction:
upstream dependencies of this package, the closure the test imports
through `dist`).
- Gates: `node scripts/pm/dispatch-gates.mjs --repo
objectstack-ai/objectstack --commands` derived 63 commands, each run
with its exit code recorded; `--ran` reconciliation: `63 derived
famil(ies) accounted for — 61 run, 2 NOT-MEASURED (2 DERIVED from a
recorded exit 3)`. 61 exited 0.
- NOT MEASURED: `pnpm check:dual-build-cjs-loads` and `pnpm check:i18n`,
both exit 3, `PREREQUISITE NOT MET` (they read built output of dozens of
packages outside this closure; CI builds them). Substitute for the
first, narrower than the gate: `pnpm --filter
@objectstack/service-storage build` exit 0 and `node -e
"require('./dist/index.cjs')"` loads, `S3StorageAdapter` is a function.
The diff touches no translation, extract config or metadata form, so
`check:i18n`'s population is not moved by it, but that is a reading of
the diff, not a measurement.
- One turbo-driven gate appended its managed block to `AGENTS.md`;
restored with `git checkout HEAD -- AGENTS.md`, never staged.
## Optional live leg
Not done. The repo has no container pattern for an enforcing store: no
testcontainers or localstack dependency and no MinIO service in any
workflow (`git grep` hits for MinIO are config prose and settings labels
only). Whether AWS S3 or MinIO enforce a query-signed checksum stays NOT
MEASURED, as on the card; this change removes the dependence on the
answer.
## Acceptance notes
- Observation, not filed: the option-pair coupling in the table above is
an SDK behaviour; it is recorded only in the test header and the adapter
comment, so a future SDK bump that changes it surfaces as the upload or
download case going red.
- The new test file is deliberately a single self-contained file with no
mock of the SDK; it does not replace the mocked command-level coverage
in `s3-storage-adapter.key-prefix.test.ts`.
---
_Generated by [Claude
Code](https://claude.ai/code/session_01DiCSbmJrkzNhuEAier4VoJ)_
---------
Co-authored-by: Claude <noreply@anthropic.com>1 parent bafb8c9 commit f0cc16e
3 files changed
Lines changed: 164 additions & 1 deletion
File tree
- .changeset
- packages/services/service-storage/src
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
| 1 | + | |
| 2 | + | |
| 3 | + | |
| 4 | + | |
| 5 | + | |
| 6 | + | |
| 7 | + | |
| 8 | + | |
| 9 | + | |
| 10 | + | |
| 11 | + | |
| 12 | + | |
| 13 | + | |
| 14 | + | |
| 15 | + | |
| 16 | + | |
| 17 | + | |
| 18 | + | |
| 19 | + | |
| 20 | + | |
| 21 | + | |
| 22 | + | |
| 23 | + | |
| 24 | + | |
| 25 | + | |
| 26 | + | |
| 27 | + | |
| 28 | + | |
| 29 | + | |
| 30 | + | |
| 31 | + | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
| 1 | + | |
| 2 | + | |
| 3 | + | |
| 4 | + | |
| 5 | + | |
| 6 | + | |
| 7 | + | |
| 8 | + | |
| 9 | + | |
| 10 | + | |
| 11 | + | |
| 12 | + | |
| 13 | + | |
| 14 | + | |
| 15 | + | |
| 16 | + | |
| 17 | + | |
| 18 | + | |
| 19 | + | |
| 20 | + | |
| 21 | + | |
| 22 | + | |
| 23 | + | |
| 24 | + | |
| 25 | + | |
| 26 | + | |
| 27 | + | |
| 28 | + | |
| 29 | + | |
| 30 | + | |
| 31 | + | |
| 32 | + | |
| 33 | + | |
| 34 | + | |
| 35 | + | |
| 36 | + | |
| 37 | + | |
| 38 | + | |
| 39 | + | |
| 40 | + | |
| 41 | + | |
| 42 | + | |
| 43 | + | |
| 44 | + | |
| 45 | + | |
| 46 | + | |
| 47 | + | |
| 48 | + | |
| 49 | + | |
| 50 | + | |
| 51 | + | |
| 52 | + | |
| 53 | + | |
| 54 | + | |
| 55 | + | |
| 56 | + | |
| 57 | + | |
| 58 | + | |
| 59 | + | |
| 60 | + | |
| 61 | + | |
| 62 | + | |
| 63 | + | |
| 64 | + | |
| 65 | + | |
| 66 | + | |
| 67 | + | |
| 68 | + | |
| 69 | + | |
| 70 | + | |
| 71 | + | |
| 72 | + | |
| 73 | + | |
| 74 | + | |
| 75 | + | |
| 76 | + | |
| 77 | + | |
| 78 | + | |
| 79 | + | |
| 80 | + | |
| 81 | + | |
| 82 | + | |
| 83 | + | |
| 84 | + | |
| 85 | + | |
| 86 | + | |
| 87 | + | |
| 88 | + | |
| 89 | + | |
| 90 | + | |
| 91 | + | |
| 92 | + | |
| 93 | + | |
| 94 | + | |
| 95 | + | |
| 96 | + | |
| 97 | + | |
| 98 | + | |
| 99 | + | |
| 100 | + | |
| 101 | + | |
| 102 | + | |
| 103 | + | |
| 104 | + | |
| 105 | + | |
| 106 | + | |
| 107 | + | |
| 108 | + | |
| 109 | + | |
| 110 | + | |
| 111 | + | |
| 112 | + | |
| 113 | + | |
| 114 | + | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
230 | 230 | | |
231 | 231 | | |
232 | 232 | | |
233 | | - | |
| 233 | + | |
| 234 | + | |
| 235 | + | |
| 236 | + | |
| 237 | + | |
| 238 | + | |
| 239 | + | |
| 240 | + | |
| 241 | + | |
| 242 | + | |
| 243 | + | |
| 244 | + | |
| 245 | + | |
| 246 | + | |
| 247 | + | |
| 248 | + | |
| 249 | + | |
| 250 | + | |
| 251 | + | |
234 | 252 | | |
235 | 253 | | |
236 | 254 | | |
| |||
0 commit comments