Fix RPC v2 CBOR timestamp parsing for float#13541
Merged
Conversation
1 task
alexrashed
approved these changes
Dec 17, 2025
Member
alexrashed
left a comment
There was a problem hiding this comment.
Nice! Great catch with the timestamp precision, and really nice to see this being explicitly covered in an AWS validated snapshot test! 💯 🦸🏽 🐛 🔨
Member
There was a problem hiding this comment.
praise: Nice to see the patches introduced with #13103 are removed here since the official specs in botocore already contain these changes now! 💯 🧹
| from cbor2._decoder import loads as cbor2_loads | ||
|
|
||
| # import the unpatched cbor2 on purpose to avoid being polluted by Kinesis-only patches | ||
| from cbor2 import loads as cbor2_loads |
S3 Image Test Results (AMD64 / ARM64) 2 files 2 suites 8m 12s ⏱️ Results for commit ffd14ad. ♻️ This comment has been updated with latest results. |
Test Results - Alternative Providers180 tests - 1 288 39 ✅ - 848 2m 29s ⏱️ - 30m 40s Results for commit ffd14ad. ± Comparison against base commit 14d1ccb. This pull request removes 1288 tests. |
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.
Motivation
As reported with #13538, we have an issue parsing CBOR datetime values that are encoded as float. In CBOR, datetime values can encoded as different major types: integer and float (0, 1, 7, 25, 26, 27). See:
https://smithy.io/2.0/additional-specs/protocols/smithy-rpc-v2.html#timestamp-type-serialization and https://datatracker.ietf.org/doc/html/rfc8949.html#section-3.4
We missed it in our testing that the Java SDK v2 would encode outgoing values as Double with Length 8 (27 major type).
This PR also removes the spec patches that are not relevant anymore since the last ASF update, as the new CloudWatch spec contains the switch and the needed changes to support
jsonandsmithy-rpc-v2-cborprotocols for the service.Changes
The fix is to not delegate the usual datetime parsing to the base parser, as the Smithy specs are pretty adamant that CBOR has its own handling. Instead, we now make sure we cap the maximum precision to millisecond, and parse the incoming timestamp normally.
I've done additional testing with the Java SDK v2 to make sure it is now able to send requests and receive our responses without issues.
I've adapted the tests to make sure we return the right format as well, same as AWS (datetime: type 6 with tag 1, encoded as 27 type float).
Tests
I've done extensive testing, and also added Java SDK v2 tests, can be found attached. Not sure how reproducible they are as I'm running them in the context of the LocalStack Utils Java library, but I could confirm I was having the same issue as reported before, and now it works.
Details
Related