fix: keep rewritten CopyObject data and metadata consistent - #86
Merged
Conversation
CopyObjectHandler recorded the source compression metadata whenever the copy was metadata-only, on the assumption that the object layer would then leave the stored bytes alone. That assumption does not hold. Both erasureServerPools.CopyObject and erasureSets.CopyObject only skip a data rewrite in three cases, and otherwise fall back to a full PutObject. The reachable gap is a copy whose source is a null version on a bucket that gained versioning after the object was written. Neither version ID is set, so the self-referential version branch is skipped, the data is rewritten as plaintext, and the preserved compression metadata then described bytes that no longer exist. A subsequent GET failed with "s2: corrupt input". Mirror the object layer's decision in copyRewritesObjectData and record the compression metadata from it, so the metadata always describes whichever bytes are finally stored. The source version selection that lets a versioned metadata-only copy add a self-referential version moves next to the same decision, since both depend on the effective metadata-only value. Signed-off-by: Feng Ruohang <rh@vonng.com> Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
A key rotation rewraps the object key held in metadata; it never re-encrypts the stored bytes. CopyObjectHandler took that shortcut whenever the request looked like a same-object SSE-C rotation, on the assumption that the object layer would then leave the stored bytes alone. That is the same assumption copyRewritesObjectData() was added to stop making. When the source is a null version on a bucket that gained versioning after the object was written, the object layer cannot reference that version and falls back to PutObject. The reader at that point holds plaintext decrypted with the old key and no EncryptFn is set, so the destination ends up storing plaintext under metadata that claims the object is SSE-C encrypted. A subsequent GET failed with "sio: unsupported version". Gate the rotation shortcut on the same prediction the compression metadata already uses. When the object layer stores new object data the rotation falls through to the regular re-encrypting copy, which decrypts with the old key and re-encrypts with the new one. The source version selection moves next to the gate because both decisions need it. That fallback authenticates the source key through the source decryptor, which GetObjectNInfo does not build for a zero byte object. Check the key explicitly before the destination is written, so the gate cannot turn a rotation that the shortcut rejected with AccessDenied into one that succeeds. The re-encrypting copy regenerates the encrypted ETag, unlike an in-place rotation; the test records that difference. The other three object layer CopyObject callers that set metadataOnly - PostRestoreObjectHandler, updateRestoreMetadata and batchKeyRotate - address the same version on both sides and never set Versioned, so they only reach the two in-place cases already covered by the copyRewritesObjectData table. Signed-off-by: Feng Ruohang <rh@vonng.com> Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Fk3PAD7DHCYzcyegYWqAmt
Exercise the silent compression-on-copy path, compressed SSE-C re-encryption, and the equal-invalid-key error contract on both object-layer backends. Signed-off-by: Feng Ruohang <rh@vonng.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.
Summary
Dependency
This PR is intentionally based on
codex/server-prerelease-foundation; the destination-key checksum fix must precede these CopyObject changes.Validation
Refs #83. Never merge the old
codex/issue-64branch; it contains a duplicate of an already-merged federation fix.