[ADD] *: support SVG images

Introduce official support for SVG files in the framework, including the
following parts:

1. When client-side SVG images are uploaded, the content is displayed until
you save using data URI scheme according RFC 2397 [1]. This scheme requires
to specify content format. Using hardcoded "image/png" works for all images
types except SVG.
Type-sniffing is done using "magic byte" detection via the first base64
encode byte, so that the proper data URI scheme can be used.
This should not cause SVG-related security problems as the file is
displayed through `<img>` tag, which does not allow SVG scripting [2].

2. Make /web/image controller compatible with SVG

3. Add support for SVG files for company logo, which uses a dedicated
controller.

4. Resizing of SVG files is a no-op, as it makes little sense for a
vector-based format. We also want to avoid micro-alterations to the SVG
document (in "natural" viewport parameters) as we would store multiple
copies of the files in the filestore.

5. Because SVG files are inherently dangerous, upload of SVG files is
restricted to administrators, either by blocking it directly before
saving it in the database (binary fields with attachment=False), or by
neutering them to text/plain mimetype (for binary fields with
attachment=True)

6. Add tests for the SVG upload cases and for the non-admin uploads.

[1] https://tools.ietf.org/html/rfc2397
[2] https://www.w3.org/wiki/SVG_Security

Closes #26635
This commit is contained in:
Pedro M. Baeza
2018-10-03 17:48:01 +02:00
committed by Olivier Dony
parent 376361c15c
commit 1be50fdeaf
14 changed files with 134 additions and 8 deletions
+6 -2
View File
@@ -37,6 +37,7 @@ import odoo.modules.registry
from odoo.api import call_kw, Environment
from odoo.modules import get_resource_path
from odoo.tools import crop_image, topological_sort, html_escape, pycompat
from odoo.tools.mimetypes import guess_mimetype
from odoo.tools.translate import _
from odoo.tools.misc import str2bool, xlwt, file_open
from odoo.tools.safe_eval import safe_eval
@@ -1218,8 +1219,11 @@ class Binary(http.Controller):
if row and row[0]:
image_base64 = base64.b64decode(row[0])
image_data = io.BytesIO(image_base64)
imgext = '.' + (imghdr.what(None, h=image_base64) or 'png')
response = http.send_file(image_data, filename=imgname + imgext, mtime=row[1])
mimetype = guess_mimetype(image_base64, default='image/png')
imgext = '.' + mimetype.split('/')[1]
if imgext == '.svg+xml':
imgext = '.svg'
response = http.send_file(image_data, filename=imgname + imgext, mimetype=mimetype, mtime=row[1])
else:
response = http.send_file(placeholder('nologo.png'))
except Exception:
@@ -1471,12 +1471,19 @@ var FieldBinaryImage = AbstractFieldBinary.extend({
},
}),
supportedFieldTypes: ['binary'],
file_type_magic_word: {
'/': 'jpg',
'R': 'gif',
'i': 'png',
'P': 'svg+xml',
},
_render: function () {
var self = this;
var url = this.placeholder;
if (this.value) {
if (!utils.is_bin_size(this.value)) {
url = 'data:image/png;base64,' + this.value;
// Use magic-word technique for detecting image type
url = 'data:image/' + (this.file_type_magic_word[this.value[0]] || 'png') + ';base64,' + this.value;
} else {
url = session.url('/web/image', {
model: this.model,
@@ -166,6 +166,12 @@ var KanbanRecord = Widget.extend({
var colorID = this._getColorID(variable);
return KANBAN_RECORD_COLORS[colorID];
},
file_type_magic_word: {
'/': 'jpg',
'R': 'gif',
'i': 'png',
'P': 'svg+xml',
},
/**
* @private
* @param {string} model the name of the model
@@ -179,7 +185,8 @@ var KanbanRecord = Widget.extend({
options = options || {};
var url;
if (this.record[field] && this.record[field].value && !utils.is_bin_size(this.record[field].value)) {
url = 'data:image/png;base64,' + this.record[field].value;
// Use magic-word technique for detecting image type
url = 'data:image/' + this.file_type_magic_word[this.record[field].value[0]] + ';base64,' + this.record[field].value;
} else if (this.record[field] && ! this.record[field].value) {
url = "/web/static/src/img/placeholder.png";
} else {
@@ -363,6 +363,7 @@ var ImageWidget = MediaWidget.extend({
if (!noRender) {
this.$('input.url').val('').trigger('input').trigger('change');
}
// TODO: Expand this for adding SVG
var domain = this.domain.concat(['|', ['mimetype', '=', false], ['mimetype', this.options.document ? 'not in' : 'in', ['image/gif', 'image/jpe', 'image/jpeg', 'image/jpg', 'image/gif', 'image/png']]]);
if (needle && needle.length) {
domain.push('|', ['datas_fname', 'ilike', needle], ['name', 'ilike', needle]);
@@ -298,6 +298,7 @@ var SlideDialog = Widget.extend({
});
return res;
},
// TODO: Remove this part, as now SVG support in image resize tools is included
//Python PIL does not support SVG, so converting SVG to PNG
svg_to_png: function () {
var img = this.$el.find("img#slide-image")[0];
+2 -1
View File
@@ -254,7 +254,8 @@ class IrAttachment(models.Model):
def _check_contents(self, values):
mimetype = values['mimetype'] = self._compute_mimetype(values)
xml_like = 'ht' in mimetype or 'xml' in mimetype # hta, html, xhtml, etc.
force_text = (xml_like and (not self.env.user._is_admin() or
user = self.env.context.get('binary_field_real_user', self.env.user)
force_text = (xml_like and (not user._is_system() or
self.env.context.get('attachments_mime_plainxml')))
if force_text:
values['mimetype'] = 'text/plain'
+1 -1
View File
@@ -449,7 +449,7 @@ class Users(models.Model):
if values['company_id'] not in self.env.user.company_ids.ids:
del values['company_id']
# safe fields only, so we write as super-user to bypass access rights
self = self.sudo()
self = self.sudo().with_context(binary_field_real_user=self.env.user)
res = super(Users, self).write(values)
if 'company_id' in values:
+14
View File
@@ -16,6 +16,11 @@ AAAAAAAAAAAAA/9oACAEBAAEFAn//xAAUEQEAAAAAAAAAAAAAAAAAAAAA/9oACAEDAQE/AX//xAAUEQE
AA/9oACAECAQE/AX//xAAUEAEAAAAAAAAAAAAAAAAAAAAA/9oACAEBAAY/An//xAAUEAEAAAAAAAAAAAAAAAAAAAAA/9oACAEBA
AE/IX//2gAMAwEAAgADAAAAEB//xAAUEQEAAAAAAAAAAAAAAAAAAAAA/9oACAEDAQE/EH//xAAUEQEAAAAAAAAAAAAAAAAAAAAA
/9oACAECAQE/EH//xAAUEAEAAAAAAAAAAAAAAAAAAAAA/9oACAEBAAE/EH//2Q=="""
SVG = b"""PD94bWwgdmVyc2lvbj0iMS4wIiBlbmNvZGluZz0iaXNvLTg4NTktMSI/Pgo8IURPQ1RZUEUgc3ZnIFBVQkxJQyAiL
S8vVzNDLy9EVEQgU1ZHIDIwMDAxMTAyLy9FTiIKICJodHRwOi8vd3d3LnczLm9yZy9UUi8yMDAwL0NSLVNWRy0yMDAwMTEwMi9E
VEQvc3ZnLTIwMDAxMTAyLmR0ZCI+Cgo8c3ZnIHdpZHRoPSIxMDAlIiBoZWlnaHQ9IjEwMCUiPgogIDxnIHRyYW5zZm9ybT0idHJ
hbnNsYXRlKDUwLDUwKSI+CiAgICA8cmVjdCB4PSIwIiB5PSIwIiB3aWR0aD0iMTUwIiBoZWlnaHQ9IjUwIiBzdHlsZT0iZmlsbD
pyZWQ7IiAvPgogIDwvZz4KCjwvc3ZnPgo="""
@tagged('standard', 'at_install')
@@ -57,6 +62,15 @@ class test_guess_mimetype(unittest.TestCase):
mimetype = guess_mimetype(content, default='test')
self.assertEqual(mimetype, 'image/gif')
def test_mimetype_svg(self):
content = base64.b64decode(SVG)
mimetype = guess_mimetype(content, default='test')
self.assertTrue(mimetype.startswith('image/svg'))
# Tests that whitespace padded SVG are not detected as SVG
mimetype = guess_mimetype(b" " + content, default='test')
self.assertNotIn("svg", mimetype)
if __name__ == '__main__':
unittest.main()
@@ -21,3 +21,4 @@ access_test_new_api_compute_protected,access_test_new_api_compute_protected,mode
access_test_new_api_multi_compute_inverse,access_test_new_api_multi_compute_inverse,model_test_new_api_multi_compute_inverse,,1,1,1,1
access_test_new_api_recursive,access_test_new_api_recursive,model_test_new_api_recursive,,1,1,1,1
access_test_new_api_cascade,access_test_new_api_cascade,model_test_new_api_cascade,,1,1,1,1
access_test_new_api_binary_svg,access_test_new_api_binary_svg,model_test_new_api_binary_svg,,1,1,1,1
1 id name model_id:id group_id:id perm_read perm_write perm_create perm_unlink
21 access_test_new_api_multi_compute_inverse access_test_new_api_multi_compute_inverse model_test_new_api_multi_compute_inverse 1 1 1 1
22 access_test_new_api_recursive access_test_new_api_recursive model_test_new_api_recursive 1 1 1 1
23 access_test_new_api_cascade access_test_new_api_cascade model_test_new_api_cascade 1 1 1 1
24 access_test_new_api_binary_svg access_test_new_api_binary_svg model_test_new_api_binary_svg 1 1 1 1
+9
View File
@@ -459,3 +459,12 @@ class ComputeCascade(models.Model):
def _compute_baz(self):
for record in self:
record.baz = "<%s>" % (record.bar or "")
class BinarySvg(models.Model):
_name = 'test_new_api.binary_svg'
_description = 'Test SVG upload'
name = fields.Char(required=True)
image_attachment = fields.Binary(attachment=True)
image_wo_attachment = fields.Binary(attachment=False)
@@ -998,6 +998,62 @@ class TestFields(common.TransactionCase):
self.assertEqual(count(message), 1)
self.assertEqual(count(message1), 0)
def test_90_binary_svg(self):
from odoo.addons.base.tests.test_mimetypes import SVG
# This should work without problems
self.env['test_new_api.binary_svg'].create({
'name': 'Test without attachment',
'image_wo_attachment': SVG,
})
# And this gives error
with self.assertRaises(UserError):
self.env['test_new_api.binary_svg'].sudo(
self.env.ref('base.user_demo'),
).create({
'name': 'Test without attachment',
'image_wo_attachment': SVG,
})
def test_91_binary_svg_attachment(self):
from odoo.addons.base.tests.test_mimetypes import SVG
# This doesn't neuter SVG with admin
record = self.env['test_new_api.binary_svg'].create({
'name': 'Test without attachment',
'image_attachment': SVG,
})
attachment = self.env['ir.attachment'].search([
('res_model', '=', record._name),
('res_field', '=', 'image_attachment'),
('res_id', '=', record.id),
])
self.assertEqual(attachment.mimetype, 'image/svg+xml')
# ...but this should be neutered with demo user
record = self.env['test_new_api.binary_svg'].sudo(
self.env.ref('base.user_demo'),
).create({
'name': 'Test without attachment',
'image_attachment': SVG,
})
attachment = self.env['ir.attachment'].search([
('res_model', '=', record._name),
('res_field', '=', 'image_attachment'),
('res_id', '=', record.id),
])
self.assertEqual(attachment.mimetype, 'text/plain')
def test_92_binary_self_avatar_svg(self):
from odoo.addons.base.tests.test_mimetypes import SVG
demo_user = self.env.ref('base.user_demo')
# User demo changes his own avatar
demo_user.sudo(demo_user).image = SVG
# The SVG file should have been neutered
attachment = self.env['ir.attachment'].search([
('res_model', '=', demo_user.partner_id._name),
('res_field', '=', 'image'),
('res_id', '=', demo_user.partner_id.id),
])
self.assertEqual(attachment.mimetype, 'text/plain')
class TestX2many(common.TransactionCase):
def test_search_many2many(self):
+13 -1
View File
@@ -10,6 +10,7 @@ from functools import partial
from operator import attrgetter
import itertools
import logging
import base64
import pytz
@@ -27,6 +28,7 @@ from .tools import float_repr, float_round, frozendict, html_sanitize, human_siz
from .tools import DEFAULT_SERVER_DATE_FORMAT as DATE_FORMAT
from .tools import DEFAULT_SERVER_DATETIME_FORMAT as DATETIME_FORMAT
from .tools.translate import html_translate, _
from .tools.mimetypes import guess_mimetype
DATE_LENGTH = len(date.today().strftime(DATE_FORMAT))
DATETIME_LENGTH = len(datetime.now().strftime(DATETIME_FORMAT))
@@ -1752,6 +1754,14 @@ class Binary(Field):
# on purpose - non base64 data must be passed as a 8bit byte strings.
if not value:
return None
# Detect if the binary content is an SVG for restricting its upload
# only to system users.
if value[:1] == b'P': # Fast detection of first 6 bits of '<' (0x3C)
decoded_value = base64.b64decode(value)
# Full mimetype detection
if (guess_mimetype(decoded_value).startswith('image/svg') and
not record.env.user._is_system()):
raise UserError(_("Only admins can upload SVG files."))
if isinstance(value, bytes):
return psycopg2.Binary(value)
try:
@@ -1793,7 +1803,9 @@ class Binary(Field):
# create the attachments that store the values
env = record_values[0][0].env
with env.norecompute():
env['ir.attachment'].sudo().create([{
env['ir.attachment'].sudo().with_context(
binary_field_real_user=env.user,
).create([{
'name': self.name,
'res_model': self.model_name,
'res_field': self.name,
+4 -1
View File
@@ -51,7 +51,10 @@ def image_resize_image(base64_source, size=(1024, 1024), encoding='base64', file
"""
if not base64_source:
return False
if size == (None, None):
# Return unmodified content if no resize or we etect first 6 bits of '<'
# (0x3C) for SVG documents - This will bypass XML files as well, but it's
# harmless for these purposes
if size == (None, None) or base64_source[:1] == b'P':
return base64_source
image_stream = io.BytesIO(codecs.decode(base64_source, encoding))
image = Image.open(image_stream)
+10
View File
@@ -102,6 +102,13 @@ def _check_olecf(data):
return 'application/vnd.ms-powerpoint'
return False
def _check_svg(data):
"""This simply checks the existence of the opening and ending SVG tags"""
if b'<svg' in data and b'/svg>' in data:
return 'image/svg+xml'
# for "master" formats with many subformats, discriminants is a list of
# functions, tried in order and the first non-falsy value returned is the
# selected mime type. If all functions return falsy values, the master
@@ -115,6 +122,9 @@ _mime_mappings = (
_Entry('image/png', [b'\x89PNG\r\n\x1A\n'], []),
_Entry('image/gif', [b'GIF87a', b'GIF89a'], []),
_Entry('image/bmp', [b'BM'], []),
_Entry('image/svg+xml', [b'<'], [
_check_svg,
]),
# OLECF files in general (Word, Excel, PPT, default to word because why not?)
_Entry('application/msword', [b'\xD0\xCF\x11\xE0\xA1\xB1\x1A\xE1', b'\x0D\x44\x4F\x43'], [
_check_olecf