[15.0][FIX] attachment_mimetype_restriction: skip binary field storage - #1896
Draft
AungKoKoLin1997 wants to merge 4 commits into
Draft
[15.0][FIX] attachment_mimetype_restriction: skip binary field storage#1896AungKoKoLin1997 wants to merge 4 commits into
AungKoKoLin1997 wants to merge 4 commits into
Conversation
Binary/image fields default to attachment=True, so the ORM stores their values as ir.attachment records with res_field set (partner/product image_1920 and its resized variants, company logo and favicon). These are field storage rather than user-facing uploads, and their content type is chosen by Odoo, not by the uploader. Validating them means that any configured allowlist that does not cover those content types makes ordinary record saves fail. Creating a company always fails, since res.company.create() injects a default favicon that no allowlist would reasonably include.
Contributor
|
Hi @AungKoKoLin1997, @yostashiro, |
OCA-git-bot
added
series:15.0
mod:attachment_mimetype_restriction
AungKoKoLin1997
marked this pull request as ready for review
August 4, 2026 01:57
yostashiro
suggested changes
Aug 4, 2026
| ] | ||
| ) | ||
| self.assertEqual(attachment.mimetype, "image/png") | ||
| self.partner.image_1920 = PNG_DATA |
Member
There was a problem hiding this comment.
Please use different data to confirm the write path works. Using the same data ends up testing nothing as it won't reach ir.attachment.write().
- Use content-based detection (guess_mimetype) instead of relying on _compute_mimetype, which trusts a caller-supplied mimetype or filename extension. Falls back to filename only when the content sniffer returns application/octet-stream (unrecognizable content). - Fix write() validating the stale record mimetype instead of sniffing the incoming content. Drop mimetype from check_vals when new datas/raw is present so guess_mimetype runs on the actual bytes. - Fix upload_attachment controller crash: core returns a str, not a Response, so response.get_data() raised AttributeError on every blocked upload. - Look up the allowlist before evaluating attachments in _message_post_process_attachments, so errors in _get_allowed_mimetypes cannot block attachments when the feature is disabled (empty allowlist). - Pass unrecognised attachment tuples through to super() instead of silently discarding them. - Reword the blocked-attachment notice (remove "from the email above" since the notice is posted before the email) and wrap in _() for i18n. - Add tests for content/extension mismatch, caller-supplied mimetype bypass, and content-based detection on write.
AungKoKoLin1997
marked this pull request as draft
August 4, 2026 07:29
AungKoKoLin1997
force-pushed
the
15.0-fix-attachment_mimetype_restriction
branch
from
August 4, 2026 08:12
a4b4c4a to
410b2ae
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Binary/image fields default to
attachment=True, so the ORM stores their values asir.attachmentrecords withres_fieldset —image_1920and its resized variants on partners/products, the company logo, the company favicon. Nothing in_validate_mimetype_from_valsexempted those, so they were validated against the allowlist even though they are field storage rather than user-facing uploads, and their content type is chosen by Odoo rather than by the uploader.The effect is that a configured allowlist which does not happen to cover those content types breaks ordinary record saves. Creating a company fails unconditionally, because
res.company.create()injects a default favicon (res_company.py) whose mimetype no allowlist would reasonably include:This PR returns early when
res_fieldis set, and addsres_fieldto the vals dict rebuilt inwrite()so the update path is covered too — replacing an existing image goes throughatts.write({'datas': value})infields.Binary.write(), not throughcreate().Added a regression test that sets
res.partner.image_1920under atext/plain-only allowlist. It fails on 15.0 today withValidationError: File type 'image/png' is not allowed.and passes with this change.Restriction of user-uploaded attachments (chatter uploads, incoming mail attachments) is unchanged.
@qrtl QT5832