fix invalid SQS message decoding #8180
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
1 task
thrau
approved these changes
Apr 21, 2023
There was a problem hiding this comment.
This is both practical and ridiculous. love it 💯
In seriousness though: I think this is the solution we had once discussed as a potential workaround and it seems fine for now. I also don't see a way around something like this right now without putting significant effort into our own serialization approach.
so LGTM 👍
my only comment would be to perhaps add a few more tests with different variants of strings so we're guarded against regressions, and that can maybe also document limitations. but not necessary in this PR imo 👍
1 task
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.
This PR slightly adjusts the special message encoding for SQS which has been introduced with #5298.
With #5298, an SQS specific serializer has been introduced which performs a special escaping logic for the node text content in the XML responses. This is only done by SQS, and since it's a very rare practice, it's not supported by any popular XML serialization libraries.
#8002 raised the issue that this behavior causes issues when these escaping sequences are used in the sent message itself.
A message
""""sent would be received as"""".This was due to the nature of the initial implementation:
""""==>""""&):"==>""""""""==>""""""""==>""""With this PR, the special escaping is done after the XML node is serialized:
""""==>__marker__"__marker__""__marker__"__marker____marker__"__marker__""__marker__"__marker__==>__marker__"__marker__""__marker__"__marker____marker__"__marker__""__marker__"__marker__==>""""""""==>""""This approach now would not work if the "marked" / internally escaped representation of the character is used in the raw message. f.e. a message
"__marker__"__marker"would be received as""".However, the only approach I can think of right now to properly implement this would be to avoid the specific pre- and post-processing and this could only be done by fixing the serialization itself (i.e. implementing our own XML serialization).
I'm happy for any feedback or ideas!
Fixes #8002.