Conversation
texastony
commented
Jan 30, 2026
texastony
commented
- chore: use HTTPS for spec
- remove spec
- duvet: point specification to private
- chore(duvet): modernize duvet
… collision check - Split isV1V2InObjectMetadata() into separate isV1InObjectMetadata() and isV2InObjectMetadata() methods that properly check for version-exclusive keys - Add hasExclusiveKeyCollision() to detect when multiple version-exclusive keys (x-amz-key, x-amz-key-v2, x-amz-3) are present in metadata - Update decode() to throw exception when exclusive key collision is detected (requirement 106) - Fix Duvet annotations for requirements 102, 103, 104, and 106 - Update tests to use new version detection methods
…ag length bug - Add annotation for V1 format exclusive key requirement in MetadataKeyConstants - Fix bug: tag length was incorrectly written for CBC (should only be for GCM) - Add annotations for both GCM and CBC tag length requirements in ContentMetadataEncodingStrategy
Add Duvet annotation for requirement: 'If a mapkey exclusive to other (non-V3) format versions is present, the S3EC SHOULD throw an exception.' Implementation: MetadataKeyConstants.isV3Format() checks that V1/V2 keys are not present when validating V3 format at ContentMetadataDecodingStrategy line 92. Test coverage: Existing testExclusiveKeysCollision validates exception is thrown for conflicting version keys.
…ement Add Duvet annotations for requirement: 'The mapkey "x-amz-unencrypted-content-length" SHOULD be present for V2 format objects.' Annotations marked as type=exception since the implementation does not write or check this field for V2 objects. Locations: - ContentMetadataDecodingStrategy line 277-279 (V2 decoding) - ContentMetadataEncodingStrategy line 228-230 (V2 encoding) Test coverage: Existing V2 format tests validate decoding without this field.
texastony
force-pushed
the
tonyknap/duvet-v3
branch
from
105cbf7 to
256218e
Compare
| [[source]] | ||
| pattern = "src/**/*.java" | ||
|
|
||
| # Include required specifications here |
There was a problem hiding this comment.
nit: is there a specific spec file we want to ignore? can we just give the glob for specification/s3-encryption?
also nit: i think we're missing Algorithm Suite, but i am also pretty sure that one doesn't contain RFC 2119 words
There was a problem hiding this comment.
I can try and refactor for a glob.
src/test/java/software/amazon/encryption/s3/internal/ContentMetadataStrategyTest.java
Show resolved
Hide resolved
| @@ -1,3 +1,4 @@ | |||
| [submodule "specification"] | |||
| [submodule "private_aws"] | |||
There was a problem hiding this comment.
procedure: i would like to merge the Spec PR then move this back to public before merging this PR
There was a problem hiding this comment.
Yes. I would to.
This was me demo-ing everything to be sure everything works.
There was a problem hiding this comment.
Also, I wanted to keep making progress, but was blocked on the spec PR, so I just worked around that here.
| @@ -0,0 +1,56 @@ | |||
| # Duvet Annotation Context | |||
There was a problem hiding this comment.
I would prefer not checking this in,
or at least make it evergreen / general to any Duvet change,
not this specific one
There was a problem hiding this comment.
Oh, this was not supposed to be checked in at all.
Good Catch.
2 participants
