feat: implement same-store CopyObject (x-amz-copy-source)#121
Merged
Conversation
CopyObject was rejected outright (501) since 0.6.0 because the presigned forward path drops the copy-source header and would overwrite the destination with an empty body. Delegate the copy to the backend S3 instead: a signed PUT to the destination carries x-amz-copy-source pointing at the source's backend bucket/key, via the same raw-signed send_raw path batch delete uses. Native S3 copy needs one endpoint that can read source and write destination, so this covers same-store copies (rename, metadata edit, cross-bucket within one account); cross-store and UploadPartCopy are rejected with a clear 501. - types.rs: add S3Operation::CopyObject (dest bucket/key + parsed source bucket/key/version); PUT method, PutObject action (dest write). Source read is authorized separately at dispatch. - api/request.rs: parse a copy-source PUT into CopyObject; parse_copy_source handles the `/bucket/key?versionId=` wire format and percent-decodes the key. UploadPartCopy (copy-source + uploadId) still 501s. - proxy.rs: execute_copy resolves+authorizes the source as a read, guards same_backing_store (same endpoint/region/creds), builds the backend copy-source header (build_copy_source_header re-encodes into the source key space), signs and sends the PUT, and passes the CopyObjectResult XML straight through. Copy-relevant client headers are forwarded and signed. - route_handler.rs: box HandlerAction::NeedsBody — the wider S3Operation pushed the enum past clippy's large-variant threshold. - docs: operations.md/errors.md now describe same-store copy, not a blanket rejection. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
📖 Docs preview deployed to https://multistore-docs-pr-121.development-seed.workers.dev
|
|
🚀 Latest commit deployed to https://multistore-proxy-pr-121.development-seed.workers.dev
|
An empty `versionId` (`bucket/key?versionId=`) failed the `!v.is_empty()` match guard and fell through to the fallback arm, which used the untrimmed string — leaving `?versionId=` glued onto the source key and silently copying from a non-existent key instead of erroring. Always split on the path; treat an empty version as `None`. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Add three tests driving the full copy path through resolve_request against the CaptureHeadersBackend, mirroring the multipart integration test: - copy_object_end_to_end_sends_resigned_backend_put: parse → authorize dest+source → same-store check → build backend copy-source → re-sign → send_raw, asserting the exact wire request (rewritten/encoded copy-source, forwarded copy-relevant headers, empty-payload hash, copy headers present in SignedHeaders). - copy_object_forwards_version_id_to_backend: versionId rides through. - cross_store_copy_is_rejected_501: different backends → 501, backend never contacted. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The `test_copy_object_rejected` test asserted the pre-implementation behavior (all copies → 501). Same-store copy now works, so replace it with two tests exercised live against MinIO through the Workers gateway: - test_copy_object_same_bucket: copy within one bucket returns 200 and the destination has the source bytes. - test_copy_object_cross_bucket_same_store: copy across two virtual buckets on the same backing store (public-data → private-uploads) is a native same-store copy and succeeds. Cross-store rejection stays covered by the Rust unit test (cross_store_copy_is_rejected_501); the integration env has a single backend. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
require_s3_backend now guards CopyObject as well as multipart, but its message was hardcoded to "multipart operations not supported", so a CopyObject to a non-S3 destination surfaced a misleading error. The helper has no operation context and shouldn't pretend to — drop the operation name rather than thread it through. Status (InvalidRequest) is unchanged. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
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 join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
What I'm changing
CopyObjecthas been rejected outright with501 NotImplementedsince 0.6.0 (#88): the presigned forward path drops thex-amz-copy-sourceheader, so a forwarded copy PUT would silently overwrite the destination with an empty body. Rejecting was the safe stopgap; this PR implements the operation.The key constraint is architectural — multistore forwards each request to exactly one backing store, but a copy has a source and a destination. A native S3 server-side copy only works when both resolve to the same S3 backend (one endpoint that can read the source and write the destination). That covers the overwhelming majority of real copies: rename, metadata edit, and cross-bucket copies within one account. Cross-store copy would require proxy-side read-then-write streaming (with >5 GB handling and S3's "error embedded in a 200 OK" case) and is deferred — rejected with a clear
501for now.How I did it
crates/core/src/types.rs— newS3Operation::CopyObjectvariant carrying the destination (bucket/key) plus the parsed source (src_bucket/src_key/src_version).method()= PUT,action()=PutObject(the destination write; the source read is authorized separately at dispatch). Added the arm to the fourS3Operationimpls.crates/core/src/api/request.rs— aPUTcarryingx-amz-copy-sourcenow parses intoCopyObjectinstead of being rejected.parse_copy_sourcehandles the[/]bucket/key[?versionId=…]wire format and percent-decodes the key to its client-visible form.UploadPartCopy(copy-source +uploadId/partNumber) remains501 NotImplemented.crates/core/src/proxy.rs—execute_copyresolves and authorizes the source as a read (a syntheticGetObjectthroughget_bucket, reusing the exact GET authorization), guardssame_backing_store(both S3, same endpoint/region/credentials — backend bucket names may differ), then signs and sends aPUTvia the existingsend_rawraw-signed path (same mechanism as batch delete).build_copy_source_headerrewrites the source into its backend key space and re-encodes it with S3's strict path set. The backend'sCopyObjectResultXML — and any error, including the 200-embedded case — is passed straight through (it carries only ETag + LastModified, neither of which needs rewriting). Copy-relevant client headers (x-amz-metadata-directive,x-amz-tagging*,x-amz-copy-source-if-*,x-amz-meta-*, SSE, ACL, storage-class) are forwarded and signed.crates/core/src/route_handler.rs— boxedHandlerAction::NeedsBody(Box<PendingRequest>). The widerS3Operationpushed the enum past clippy'slarge_enum_variantthreshold; boxing the (less-hot) multipart/batch-delete variant is the localized fix.docs/—operations.mdanderrors.mdnow document same-store copy and its limits instead of a blanket rejection.Test plan
cargo fmt --checkcargo clippy(native) — cleancargo clippy --target wasm32-unknown-unknown -p multistore-cf-workers— cleancargo test -p multistore— 135 pass, incl. new parse tests (copy-source with/without leading slash, versionId, encoded key, malformed values, UploadPartCopy rejection) and unit tests forsame_backing_storeandbuild_copy_source_header🤖 Generated with Claude Code