Skip to content

1728 - Enhance/Fix ByteArrayConverter to support base64 deserialization of byte[] #1736

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Apr 24, 2024

Conversation

brendonparker
Copy link
Contributor

Issue #, if available: 1728

Description of changes:
Add support for deserializing a base64 string to a byte[].

If the token is a string, assume it is base64 encoded string and get bytes.

Otherwise, continue to do what it did before.

I added xunit tests for both conditions. To ensure my desired behavior works, and the existing behavior also works. I'm not strong in xunit, so if there are tweaks needed, please advise. Also wasn't sure where the best home/project for those tests might be.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

@brendonparker
Copy link
Contributor Author

One callout: serialization will still convert a byte[] to an array of numbers (ints) in json. No good way of supporting both write formats in one converter.

But in this approach, at least reading/deserializing can be flexible.

@normj normj changed the base branch from master to dev April 21, 2024 20:07
Copy link
Member

@normj normj left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the fix. I agree there isn't really anything we can do for writes without some pre knowledge of what the format is supposed to be.

If you want bump the version in the project file to 2.4.3 that will save us from creating another version bump BR. https://github.com/aws/aws-lambda-dotnet/blob/master/Libraries/src/Amazon.Lambda.Serialization.SystemTextJson/Amazon.Lambda.Serialization.SystemTextJson.csproj#L12C10-L12C23

@normj normj merged commit 92c729f into aws:dev Apr 24, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants