OCA / storage

GNU Affero General Public License v3.0
70 stars 159 forks source link

[16.0][FIX] fs_attachment: compute the mimetype from new datas or raw if not present #327

Closed benwillig closed 5 months ago

benwillig commented 8 months ago

Before this PR, if you update the data of an attachment and if the new mimetype doesn't match the old one, we can have unexpected errors.

In our instance, we migrated from version 13.0 to 16.0. In 13.0, we have a .png icon for the HR menu. In 16.0 we have a .svg. As we provide the old mimetype, Odoo tries to resize the image as it thinks it is a png, but its a svg so it crashes.

  File "odoo/addons/base/models/ir_attachment.py", line 368, in _check_contents
    values = self._postprocess_contents(values)
             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "odoo/addons/base/models/ir_attachment.py", line 338, in _postprocess_contents
    w, h = img.image.size
           ^^^^^^^^^^^^^^
AttributeError: 'bool' object has no attribute 'size'

After this PR, we try to guess the mimetype using the Odoo method, which guess the mimetype from the new data. If we get "application/octet-stream", we do the old process.

But to be honest, I don't understand why we should take the old mimetype if not given, as the new data might not match.

OCA-git-bot commented 8 months ago

Hi @lmignon, some modules you are maintaining are being modified, check this out!

lmignon commented 8 months ago

@benwillig Can you provide a test to reproduce the error?

benwillig commented 8 months ago

Here is a test to reproduce the error

data_png = b'iVBORw0KGgoAAAANSUhEUgAAADMAAAAhCAIAAAD73QTtAAAAA3NCSVQICAjb4U/gAAAAP0lEQVRYhe3OMQGAMBAAsVL/nh8FDDfxQ6Igz8ycle7fgU9mnVln1pl1Zp1ZZ9aZdWadWWfWmXVmnVln1u2dvfL/Az+TRcv4AAAAAElFTkSuQmCC'

attachment = env['ir.attachment'].create({
    "name": "test.png",
    "datas": data_png,
})

data_svg = b'PD94bWwgdmVyc2lvbj0iMS4wIiBzdGFuZGFsb25lPSJubyI/Pgo8IURPQ1RZUEUgc3ZnIFBVQkxJQyAiLS8vVzNDLy9EVEQgU1ZHIDIwMDEwOTA0Ly9FTiIKICJodHRwOi8vd3d3LnczLm9yZy9UUi8yMDAxL1JFQy1TVkctMjAwMTA5MDQvRFREL3N2ZzEwLmR0ZCI+CjxzdmcgdmVyc2lvbj0iMS4wIiB4bWxucz0iaHR0cDovL3d3dy53My5vcmcvMjAwMC9zdmciCiB3aWR0aD0iNTEuMDAwMDAwcHQiIGhlaWdodD0iMzMuMDAwMDAwcHQiIHZpZXdCb3g9IjAgMCA1MS4wMDAwMDAgMzMuMDAwMDAwIgogcHJlc2VydmVBc3BlY3RSYXRpbz0ieE1pZFlNaWQgbWVldCI+Cgo8ZyB0cmFuc2Zvcm09InRyYW5zbGF0ZSgwLjAwMDAwMCwzMy4wMDAwMDApIHNjYWxlKDAuMTAwMDAwLC0wLjEwMDAwMCkiCmZpbGw9IiMwMDAwMDAiIHN0cm9rZT0ibm9uZSI+CjwvZz4KPC9zdmc+Cg=='
attachment.write({
    "datas": data_svg,
})
lmignon commented 8 months ago

Here is a test to reproduce the error

data_png = b'iVBORw0KGgoAAAANSUhEUgAAADMAAAAhCAIAAAD73QTtAAAAA3NCSVQICAjb4U/gAAAAP0lEQVRYhe3OMQGAMBAAsVL/nh8FDDfxQ6Igz8ycle7fgU9mnVln1pl1Zp1ZZ9aZdWadWWfWmXVmnVln1u2dvfL/Az+TRcv4AAAAAElFTkSuQmCC'

attachment = env['ir.attachment'].create({
    "name": "test.png",
    "datas": data_png,
})

data_svg = b'PD94bWwgdmVyc2lvbj0iMS4wIiBzdGFuZGFsb25lPSJubyI/Pgo8IURPQ1RZUEUgc3ZnIFBVQkxJQyAiLS8vVzNDLy9EVEQgU1ZHIDIwMDEwOTA0Ly9FTiIKICJodHRwOi8vd3d3LnczLm9yZy9UUi8yMDAxL1JFQy1TVkctMjAwMTA5MDQvRFREL3N2ZzEwLmR0ZCI+CjxzdmcgdmVyc2lvbj0iMS4wIiB4bWxucz0iaHR0cDovL3d3dy53My5vcmcvMjAwMC9zdmciCiB3aWR0aD0iNTEuMDAwMDAwcHQiIGhlaWdodD0iMzMuMDAwMDAwcHQiIHZpZXdCb3g9IjAgMCA1MS4wMDAwMDAgMzMuMDAwMDAwIgogcHJlc2VydmVBc3BlY3RSYXRpbz0ieE1pZFlNaWQgbWVldCI+Cgo8ZyB0cmFuc2Zvcm09InRyYW5zbGF0ZSgwLjAwMDAwMCwzMy4wMDAwMDApIHNjYWxlKDAuMTAwMDAwLC0wLjEwMDAwMCkiCmZpbGw9IiMwMDAwMDAiIHN0cm9rZT0ibm9uZSI+CjwvZz4KPC9zdmc+Cg=='
attachment.write({
    "datas": data_svg,
})

Can you add this test into the testsuite plz....

lmignon commented 5 months ago

/ocabot merge patch

OCA-git-bot commented 5 months ago

Hey, thanks for contributing! Proceeding to merge this for you. Prepared branch 16.0-ocabot-merge-pr-327-by-lmignon-bump-patch, awaiting test results.

OCA-git-bot commented 5 months ago

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