Skip to content

fix: Base64 decoding to discard newline characters#1941

Merged
suztomo merged 7 commits intomainfrom
base64_encoding
May 16, 2024
Merged

fix: Base64 decoding to discard newline characters#1941
suztomo merged 7 commits intomainfrom
base64_encoding

Conversation

@suztomo
Copy link
Member

@suztomo suztomo commented May 15, 2024

Using b/274482271#comment3 by @eamonnmcmanus

  • Added test cases for new line characters.
  • This PR keeps the trim() method that was also added for the compatibility.

@suztomo suztomo requested a review from a team May 15, 2024 19:18
@product-auto-label product-auto-label bot added the size: s Pull request size is small. label May 15, 2024
@product-auto-label product-auto-label bot added size: m Pull request size is medium. and removed size: s Pull request size is small. labels May 15, 2024
Copy link

@eamonnmcmanus eamonnmcmanus left a comment

Choose a reason for hiding this comment

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

LGTM, with just one very small thing.

@Deprecated
public class Base64 {
// Special decoders that discards the new line character so that the behavior matches what
// we had with Apache Commmons Codec's decodeBase64.

Choose a reason for hiding this comment

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

I think it would be worth commenting on what the 64 means. (It doesn't mean very much, since it's only used for encoding and we don't do that.)

Copy link
Member Author

Choose a reason for hiding this comment

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

Added explanation of the 2nd parameter of the withSeparator method.

Copy link

@eamonnmcmanus eamonnmcmanus left a comment

Choose a reason for hiding this comment

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

Sorry for misunderstanding in my previous comment.

@suztomo suztomo merged commit 4e153db into main May 16, 2024
@suztomo suztomo deleted the base64_encoding branch May 16, 2024 17:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size: m Pull request size is medium.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants