Skip to content
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

fix: fix incorrect base64 padding assumption #12

Merged
merged 2 commits into from
Mar 6, 2024

Conversation

RisaI
Copy link
Contributor

@RisaI RisaI commented Mar 6, 2024

There was an oversight in 789d5ef, the previous decode call was replaced with the STANDARD_NO_PAD engine, which is not the correct engine to use. The previous decode function was indifferent to the padding, hence it passed the test and also worked on an actual config.json file. However, the config.json file always contains PADDED base64, so the library is now not working in the real world.

This PR introduces a custom engine config with indifferent padding to replicate the previous behavior.

Copy link
Owner

@keirlawson keirlawson left a comment

Choose a reason for hiding this comment

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

Looks great, could you add some sort of regression test so we can ensure this doesn't crop up again?

@RisaI
Copy link
Contributor Author

RisaI commented Mar 6, 2024

I added the decodes_regardless_of_padding test.

Copy link
Owner

@keirlawson keirlawson left a comment

Choose a reason for hiding this comment

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

lgtm

@keirlawson keirlawson merged commit 80a1697 into keirlawson:master Mar 6, 2024
1 check passed
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.

2 participants