fix: kinesis CBOR blob handling #11220
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
S3 Image Test Results (AMD64 / ARM64) 2 files 2 suites 3m 25s ⏱️ Results for commit e380b77. ♻️ This comment has been updated with latest results. |
85fc90a to
ae6fa28
Compare
alexrashed
approved these changes
Jul 17, 2024
There was a problem hiding this comment.
Thanks a lot for digging into this issue (which is quite tedious due to the lack of CBOR support in botocore)!
And I'm sorry I introduced this regression with the last iteration of the CBOR support... In retrospect I can't really tell what lead me to the conclusion that the CBOR content needs to be parsed as base64 too... 😅
The changeset looks good, and your analysis was great and thorough! 💯
We want to re-use the HTTP request generation in an upcoming test
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
A report from a customer showed that a round trip of data through a Kinesis stream from LocalStack did not preserve the data content.
For example:
PutRecordin Kinesis stream,"test data"GetRecordsfrom kinesis stream, convert 1st record toString, which turns out different from "test data":��-u�ZThey were using the Java SDK. When testing on AWS, a
GetRecordsrequest returns the message body (decoded as a String) of"test data"correctly.Since this is exercised via the Java SDK, it is the handling of the CBOR format that it as fault. It turned out that we did not parse
CBORblobs correctly.BaseJSONRequestParser._parse_blobmethod was removed.RequestParser._parse_blobmethod, which incorrectly base64-decoded the value.tests/unit/aws/protocol/test_parser.py::test_json_cbor_blob_parsingto test this incorrect behaviour.Changes
This PR reverts the removal of
BaseJSONRequestParser._parse_bloband updates the relevant test accordingly.