Skip to content

[18.0][FIX] dms: Always b64encode contents from messages - #502

Merged
OCA-git-bot merged 1 commit into
OCA:18.0from
houzefa-abba:dms-always-b64encode
Aug 17, 2026
Merged

[18.0][FIX] dms: Always b64encode contents from messages#502
OCA-git-bot merged 1 commit into
OCA:18.0from
houzefa-abba:dms-always-b64encode

Conversation

@houzefa-abba

Copy link
Copy Markdown
Member

Previous implementation was failing to convert strings to base64, which is the case when we receive plain HTML encoded as base64; as shown in mail02.eml.

This in turn lead to inconsistencies when saving/reading these contents afterwards, as the rest of the code always assumes dms.file::content contains base64 data.

This was falling through cracks because although the resulting data is invalid base64, the b64decode impl in python 3.12/3.13 is lax enough it would ignore failures in this case because our test string is "base64-ish" enough.

However, when running Odoo on Debian with the patch https://sources.debian.org/patches/python3.13/3.13.5-2+deb13u3/CVE-2026-3446.patch/ applied, this test on mail02.eml produced errors in dms.file::_inverse_content when trying to b64decode:

<class 'binascii.Error'> Invalid base64-encoded string: number of data characters (429) cannot be 1 more than a multiple of 4

@houzefa-abba houzefa-abba changed the title [18.0] [FIX] dms: Always b64encode contents from messages [18.0][FIX] dms: Always b64encode contents from messages Aug 6, 2026
@houzefa-abba
houzefa-abba force-pushed the dms-always-b64encode branch from 32e3d0b to 02f8a2a Compare August 7, 2026 07:35
@pedrobaeza
pedrobaeza requested review from etobella and victoralmau and removed request for etobella August 11, 2026 11:21
@pedrobaeza pedrobaeza added this to the 18.0 milestone Aug 11, 2026

@victoralmau victoralmau left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

IMO, the change makes sense.

@pedrobaeza pedrobaeza left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Please remove the change in the test, as it's arbitrary and the test is not adding value.

Previous implementation was failing to convert strings to base64, which
is the case when we receive plain HTML encoded as base64; as shown in
mail02.eml.

This in turn lead to inconsistencies when saving/reading these contents
afterwards, as the rest of the code always assumes dms.file::content
contains base64 data.

This was falling through cracks because although the resulting data is
invalid base64, the b64decode impl in python 3.12/3.13 is lax enough it
would ignore failures in this case because our test string is
"base64-ish" enough.

However, when running Odoo on Debian with the patch
https://sources.debian.org/patches/python3.13/3.13.5-2+deb13u3/CVE-2026-3446.patch/
applied, this test on mail02.eml produced errors in
dms.file::_inverse_content when trying to b64decode:

> <class 'binascii.Error'> Invalid base64-encoded string: number of data characters (429) cannot be 1 more than a multiple of 4
@houzefa-abba

Copy link
Copy Markdown
Member Author

Please remove the change in the test, as it's arbitrary and the test is not adding value.

Hmm I added that test bit to show what this PR fixes: the test ensures b64 contents got saved in DB, not raw contents.

I agree the hard-coded 876/657 are ugly, I just pushed a simpler check that tests base64.b64decode(db_contents) != db_contents.

@pedrobaeza pedrobaeza left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

/ocabot merge patch

@OCA-git-bot

Copy link
Copy Markdown
Contributor

This PR looks fantastic, let's merge it!
Prepared branch 18.0-ocabot-merge-pr-502-by-pedrobaeza-bump-patch, awaiting test results.

@OCA-git-bot

Copy link
Copy Markdown
Contributor

This PR has the approved label and has been created more than 5 days ago. It should therefore be ready to merge by a maintainer (or a PSC member if the concerned addon has no declared maintainer). 🤖

@OCA-git-bot
OCA-git-bot merged commit 2ee7c8d into OCA:18.0 Aug 17, 2026
7 checks passed
@OCA-git-bot

Copy link
Copy Markdown
Contributor

Congratulations, your PR was merged at 50ccbfe. Thanks a lot for contributing to OCA. ❤️

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants