[FIX] core: prevent upload of large files
The limit was only enforced by the front-end, meaning that anybody could forge a request with a huge file and get it processed by Odoo. According to the documentation of werkzeug[^1], such limit should be enforced by the server server instead of the wsgi application. It is the case for Odoo Online but on-premise customers might not configure their servers. The `web.max_file_upload_size` system paramter is now enforced upon parsing the content of the request. It defaults at 128 MiB which is enough for most documents and images. We do not want to host large files (e.g. videos) in the Odoo filestore. [^1]: https://werkzeug.palletsprojects.com/en/2.0.x/request_data/ Fixes: #124646 Part-of: odoo/odoo#126914
This commit is contained in:
committed by
Julien Carion (juca)
parent
7b638c7e3a
commit
cc5a14b6a9
@@ -187,7 +187,7 @@ class Binary(http.Controller):
|
||||
try:
|
||||
attachment = Model.create({
|
||||
'name': filename,
|
||||
'datas': base64.encodebytes(ufile.read()),
|
||||
'raw': ufile.read(),
|
||||
'res_model': model,
|
||||
'res_id': int(id)
|
||||
})
|
||||
@@ -200,7 +200,7 @@ class Binary(http.Controller):
|
||||
else:
|
||||
args.append({
|
||||
'filename': clean(filename),
|
||||
'mimetype': ufile.content_type,
|
||||
'mimetype': attachment.mimetype,
|
||||
'id': attachment.id,
|
||||
'size': attachment.file_size
|
||||
})
|
||||
|
||||
@@ -6,7 +6,7 @@ import logging
|
||||
|
||||
import odoo
|
||||
from odoo import api, http, models
|
||||
from odoo.http import request
|
||||
from odoo.http import request, DEFAULT_MAX_CONTENT_LENGTH
|
||||
from odoo.tools import file_open, image_process, ustr
|
||||
from odoo.tools.misc import str2bool
|
||||
|
||||
@@ -84,7 +84,7 @@ class Http(models.AbstractModel):
|
||||
IrConfigSudo = self.env['ir.config_parameter'].sudo()
|
||||
max_file_upload_size = int(IrConfigSudo.get_param(
|
||||
'web.max_file_upload_size',
|
||||
default=128 * 1024 * 1024, # 128MiB
|
||||
default=DEFAULT_MAX_CONTENT_LENGTH,
|
||||
))
|
||||
mods = odoo.conf.server_wide_modules or []
|
||||
if request.db:
|
||||
|
||||
@@ -2,12 +2,13 @@
|
||||
|
||||
import base64
|
||||
from datetime import datetime, timedelta
|
||||
from http import HTTPStatus
|
||||
from os.path import basename, join as opj
|
||||
from unittest.mock import patch
|
||||
from freezegun import freeze_time
|
||||
|
||||
import odoo
|
||||
from odoo.tests import new_test_user, tagged
|
||||
from odoo.tests import new_test_user, tagged, RecordCapturer
|
||||
from odoo.tools import config, file_open, image_process
|
||||
|
||||
from .test_common import TestHttpBase
|
||||
@@ -395,6 +396,7 @@ class TestHttpStaticLogo(TestHttpStaticCommon):
|
||||
self.assertDownloadLogoDefault(company=self.company2, user=self.user_of_company_of_superuser)
|
||||
|
||||
|
||||
@tagged('post_install', '-at_install')
|
||||
class TestHttpStaticCache(TestHttpStaticCommon):
|
||||
@freeze_time(datetime.utcnow())
|
||||
def test_static_cache0_standard(self, domain=''):
|
||||
@@ -448,3 +450,63 @@ class TestHttpStaticCache(TestHttpStaticCommon):
|
||||
})
|
||||
res2.raise_for_status()
|
||||
self.assertEqual(res2.status_code, 304, "We should not download the file again.")
|
||||
|
||||
|
||||
@tagged('post_install', '-at_install')
|
||||
class TestHttpStaticUpload(TestHttpStaticCommon):
|
||||
def test_upload_small_file(self):
|
||||
new_test_user(self.env, 'jackoneill')
|
||||
self.authenticate('jackoneill', 'jackoneill')
|
||||
|
||||
with RecordCapturer(self.env['ir.attachment'], []) as capture, \
|
||||
file_open('test_http/static/src/img/gizeh.png', 'rb') as file:
|
||||
file_content = file.read()
|
||||
file_size = len(file_content)
|
||||
file.seek(0)
|
||||
res = self.opener.post(
|
||||
f'{self.base_url()}/web/binary/upload_attachment',
|
||||
files={'ufile': file},
|
||||
data={
|
||||
'csrf_token': odoo.http.Request.csrf_token(self),
|
||||
'model': 'test_http.stargate',
|
||||
'id': self.env.ref('test_http.earth').id,
|
||||
},
|
||||
)
|
||||
res.raise_for_status()
|
||||
|
||||
self.assertEqual(len(capture.records), 1, "An attachment should have been created")
|
||||
self.assertEqual(capture.records.name, 'gizeh.png')
|
||||
self.assertEqual(capture.records.raw, file_content)
|
||||
self.assertEqual(capture.records.mimetype, 'image/png')
|
||||
|
||||
self.assertEqual(res.json(), [{
|
||||
'filename': 'gizeh.png',
|
||||
'mimetype': 'image/png',
|
||||
'id': capture.records.id,
|
||||
'size': file_size,
|
||||
}])
|
||||
|
||||
|
||||
def test_upload_large_file(self):
|
||||
new_test_user(self.env, 'jackoneill')
|
||||
self.authenticate('jackoneill', 'jackoneill')
|
||||
|
||||
with RecordCapturer(self.env['ir.attachment'], []) as capture, \
|
||||
file_open('test_http/static/src/img/gizeh.png', 'rb') as file:
|
||||
file_size = file.seek(0, 2)
|
||||
file.seek(0)
|
||||
self.env['ir.config_parameter'].sudo().set_param(
|
||||
'web.max_file_upload_size', file_size - 1,
|
||||
)
|
||||
res = self.opener.post(
|
||||
f'{self.base_url()}/web/binary/upload_attachment',
|
||||
files={'ufile': file},
|
||||
data={
|
||||
'csrf_token': odoo.http.Request.csrf_token(self),
|
||||
'model': 'test_http.stargate',
|
||||
'id': self.env.ref('test_http.earth').id,
|
||||
'callback': 'callmemaybe',
|
||||
},
|
||||
)
|
||||
self.assertFalse(capture.records, "No attachment should have been created")
|
||||
self.assertEqual(res.status_code, HTTPStatus.REQUEST_ENTITY_TOO_LARGE)
|
||||
|
||||
@@ -232,6 +232,8 @@ def get_default_session():
|
||||
'session_token': None,
|
||||
}
|
||||
|
||||
DEFAULT_MAX_CONTENT_LENGTH = 128 * 1024 * 1024 # 128MiB
|
||||
|
||||
# Two empty objects used when the geolocalization failed. They have the
|
||||
# sames attributes as real countries/cities except that accessing them
|
||||
# evaluates to None.
|
||||
@@ -1482,6 +1484,18 @@ class Request:
|
||||
:returns: The merged key-value pairs.
|
||||
:rtype: dict
|
||||
"""
|
||||
if self.env:
|
||||
ICP = self.env['ir.config_parameter'].sudo()
|
||||
try:
|
||||
key = 'web.max_file_upload_size'
|
||||
self.httprequest.max_content_length = int(ICP.get_param(
|
||||
key, DEFAULT_MAX_CONTENT_LENGTH
|
||||
))
|
||||
except ValueError: # better not crash on ALL requests
|
||||
_logger.error("invalid %s: %r, use %s instead",
|
||||
key, ICP.get_param(key), self.httprequest.max_content_length,
|
||||
)
|
||||
|
||||
params = {
|
||||
**self.httprequest.args,
|
||||
**self.httprequest.form,
|
||||
@@ -2121,6 +2135,7 @@ class Application:
|
||||
httprequest.user_agent_class = UserAgent # use vendored userAgent since it will be removed in 2.1
|
||||
httprequest.parameter_storage_class = (
|
||||
werkzeug.datastructures.ImmutableOrderedMultiDict)
|
||||
httprequest.max_content_length = DEFAULT_MAX_CONTENT_LENGTH
|
||||
request = Request(httprequest)
|
||||
_request_stack.push(request)
|
||||
request._post_init()
|
||||
|
||||
Reference in New Issue
Block a user