fix(s3): conditional write and delete preconditions were never evaluated - #107
Merged
Merged
Conversation
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
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.
Fixes #104.
Mutations carrying preconditions succeeded unconditionally. All three of these returned success where S3 returns
412:PUTwithIf-None-Match: *on an existing key200412PUTwith a non-matchingIf-Match200412DELETEwith a non-matchingIf-Match204412Clients saw writes land that they had explicitly asked the server to reject. Found by @jeremydixon22 driving the
aws-s3-styleadapter with AWSSDK.S3 4.0.102.4, and reproduced with a hand-signed raw HTTP request carrying no SDK at all.If-Match,If-None-Match,If-Modified-Since, andIf-Unmodified-Sinceappear nowhere underadapters/aws-s3-style/.on_put_object,on_get_object,on_head_object, andon_delete_objectbranched on existence alone, andconformance/matrix.yamldid not record the gap.What changed
Preconditions are evaluated after auth, bucket, and object lookup, in fail-closed RFC 7232 order:
If-Match,If-Unmodified-Since,If-None-Match,If-Modified-Since.GETandHEADevaluate all four, comparing timestamps against a seconds-truncatedLast-Modified. A matchingIf-None-MatchorIf-Modified-Sincereturns304with an empty body, as RFC 7232 requires; any other failed precondition returns412 PreconditionFailedXML carrying<Condition>and<Key>.PUTandDELETEevaluate the ETag conditions only, matching S3 conditional writes, soDELETEon a missing object stays idempotent at204whileDELETEwithIf-Matchon a missing object returns412.If-None-MatchsuppressesIf-Modified-SinceandIf-MatchsuppressesIf-Unmodified-Since.400. The RFC 1123 parser accepts the GMT/UTC/UT forms real clients send and rejects two-digit years, numeric offsets, and impossible dates such asFeb 30.W/"...") compare as strong, recorded as a deviation.Two pre-existing bucket behaviors were left alone and are now documented as deviations in
conformance/matrix.yaml:DELETEagainst a missing bucket returns204, andGET/HEADagainst a missing bucket returnsNoSuchKey. Real S3 returnsNoSuchBucketfor both. Correcting those is a separate change.Verification
TestAWSS3Conditionalscovers the three preconditions above plus read-side304/412, missing-object precedence per operation, the RFC 7232 ignore rules,W/and*and quoted validators, and the date grammar including leap years andFeb 30.Through AWSSDK.S3 4.0.102.4 and a hand-signed raw HTTP request:
Two shared conformance cases that previously failed now pass:
EnforcesWritePreconditionsandDeletesObjectsIdempotentlyAndEnforcesPreconditions.stunt adapter lint adapters/aws-s3-styleis clean andjust conformance-matrixregenerates with no drift.