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 base64 encode/decode bug due to Pydantic issue #48

Merged
merged 1 commit into from
Sep 19, 2024

Conversation

facutuesca
Copy link
Collaborator

Pydantic's Base64Bytes encodes bytes to base64 inserting newlines (b'\n') every 76 characters. This is not correct for our use case, so this PR switches to our own Base64Bytes with a custom encoder.

See pydantic/pydantic#9072 (comment) for more context.

Comment on lines +37 to +58
class Base64EncoderSansNewline(Base64Encoder):
r"""A Base64Encoder that doesn't insert newlines when encoding.

Pydantic's Base64Bytes type inserts newlines b'\n' every 76 characters because they
use `base64.encodebytes()` instead of `base64.b64encode()`. Pydantic maintainers
have stated that they won't fix this, and that users should work around it by
defining their own Base64 type with a custom encoder.
See https://github.com/pydantic/pydantic/issues/9072 for more details.
"""

@classmethod
def encode(cls, value: bytes) -> bytes:
"""Encode bytes to base64."""
return base64.b64encode(value)

@classmethod
def decode(cls, value: bytes) -> bytes:
"""Decode base64 bytes."""
return base64.b64decode(value, validate=True)


Base64Bytes = Annotated[bytes, EncodedBytes(encoder=Base64EncoderSansNewline)]
Copy link
Member

Choose a reason for hiding this comment

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

Let's add a backstop test for this type as well, to assert that it absolutely never contains newlines in its encodings and fails when newlines are present while decoding.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

done!

Copy link
Member

@woodruffw woodruffw left a comment

Choose a reason for hiding this comment

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

Thanks @facutuesca! One comment + let's add a bugfix changelog entry for this 🙂

Copy link
Member

@woodruffw woodruffw left a comment

Choose a reason for hiding this comment

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

Thanks @facutuesca!

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