diff --git a/news/147.bugfix b/news/147.bugfix new file mode 100644 index 00000000..5310330e --- /dev/null +++ b/news/147.bugfix @@ -0,0 +1,6 @@ +Fixed the issue where SVG images containing extensive metadata were not being displayed +correctly (resulting in a width/height of 1px). This problem could occur when the + tag exceeded the MAX_INFO_BYTES limit. + +Fixes `issue 147 `_. +[mliebischer] \ No newline at end of file diff --git a/plone/namedfile/file.py b/plone/namedfile/file.py index 7c2d1ef1..f527aa1c 100644 --- a/plone/namedfile/file.py +++ b/plone/namedfile/file.py @@ -405,7 +405,8 @@ def _setData(self, data): super()._setData(data) firstbytes = self.getFirstBytes() res = getImageInfo(firstbytes) - if res == ("image/jpeg", -1, -1) or res == ("image/tiff", -1, -1): + if res == ("image/jpeg", -1, -1) or res == ("image/tiff", -1, -1) or \ + res == ("image/svg+xml", -1, -1): # header was longer than firstbytes start = len(firstbytes) length = max(0, MAX_INFO_BYTES - start) diff --git a/plone/namedfile/tests/__init__.py b/plone/namedfile/tests/__init__.py index 07f36b83..16d30e54 100644 --- a/plone/namedfile/tests/__init__.py +++ b/plone/namedfile/tests/__init__.py @@ -1,8 +1,8 @@ import os -def getFile(filename): +def getFile(filename, length=None): """return contents of the file with the given name""" filename = os.path.join(os.path.dirname(__file__), filename) with open(filename, "rb") as data_file: - return data_file.read() + return data_file.read(length) diff --git a/plone/namedfile/tests/image_large_header.svg b/plone/namedfile/tests/image_large_header.svg new file mode 100644 index 00000000..ce53a0ae --- /dev/null +++ b/plone/namedfile/tests/image_large_header.svg @@ -0,0 +1,661 @@ + + + + + + + + + + + + +
+
+
+

+ Object:Type +

+
+

field1 = value1
field2 = value2
field3 = value3 +

+

field4 = value4 +
+

+

field5 = value5 +
+

+

field6 = value6 +
+

+

field7 = value7 +
+

+

field8 = value8 +
+

+

field9 = value9 +
+

+

field10 = value10

+
+
+
+ + Object:Type... + + + + + + + +
+
+
+

+ Object:Type +

+
+

field1 = value1
field2 = value2
field3 = value3 +

+

field4 = value4 +
+

+

field5 = value5 +
+

+

field6 = value6 +
+

+

field7 = value7 +
+

+

field8 = value8

+
+
+
+ + Object:Type... + + + + + + + + +
+
+
+

+ Object:Type +

+
+

field1 = value1
field2 = value2
field3 = value3 +

+

field4 = value4 +
+

+

field5 = value5 +
+

+

field6 = value6 +
+

+

field7 = value7 +
+

+

field8 = value8 +
+

+

field9 = value9 +
+

+

field10 = value10

+
+
+
+ + Object:Type... + + + + + + + + +
+
+
+

+ Object:Type +

+
+

field1 = value1
field2 = value2
field3 = value3 +

+

field4 = value4 +
+

+

field5 = value5 +
+

+

field6 = value6 +
+

+

field7 = value7 +
+

+

field8 = value8 +
+

+

field9 = value9 +
+

+

field10 = value10 +
+

+

field11 = value11 +
+

+

field12 = value12

+
+
+
+ + Object:Type... + + + + + + + +
+
+
+

+ Object:Type +

+
+

field1 = value1
field2 = value2
field3 = value3 +

+

field4 = value4 +
+

+

field5 = value5 +
+

+

field6 = value6 +
+

+

field7 = value7 +
+

+

field8 = value8

+
+
+
+ + Object:Type... + + + + + + + +
+
+
+

+ Object:Type +

+
+

field1 = value1
field2 = value2
field3 = value3 +

+

field4 = value4

+
+
+
+ + Object:Type... + + + + + + + + + +
+
+
+

+ Object:Type +

+
+

field1 = value1
field2 = value2
field3 = value3 +

+

field4 = value4 +
+

+

field5 = value5 +
+

+

field6 = value6 +
+

+

field7 = value7 +
+

+

field8 = value8 +
+

+

field9 = value9 +
+

+

field10 = value10

+
+
+
+ + Object:Type... + + + + + + + + + +
+
+
+

+ Object:Type +

+
+

field1 = value1
field2 = value2
field3 = value3 +

+

field4 = value4 +
+

+

field5 = value5 +
+

+

field6 = value6 +
+

+

field7 = value7 +
+

+

field8 = value8

+
+
+
+ + Object:Type... + + + + + + + +
+
+
+

+ Object:Type +

+
+

field1 = value1
field2 = value2
field3 = value3 +

+

field4 = value4 +
+

+

field5 = value5 +
+

+

field6 = value6 +
+

+

field7 = value7 +
+

+

field8 = value8 +
+

+

field9 = value9 +
+

+

field10 = value10 +
+

+

field11 = value11 +
+

+

field12 = value12

+
+
+
+ + Object:Type... + + + + + + + + + +
+
+
+

+ Object:Type +

+
+

field1 = value1
field2 = value2
field3 = value3 +

+

field4 = value4 +
+

+

field5 = value5 +
+

+

field6 = value6 +
+

+

field7 = value7 +
+

+

field8 = value8 +
+

+

field9 = value9 +
+

+

field10 = value10

+
+
+
+ + Object:Type... + + + + + + + +
+
+
+

+ Object:Type +

+
+

field1 = value1
field2 = value2
field3 = value3 +

+

field4 = value4 +
+

+

field5 = value5 +
+

+

field6 = value6 +
+

+

field7 = value7 +
+

+

field8 = value8

+
+
+
+ + Object:Type... + + + + + + + +
+
+
+

+ Object:Type +

+
+

field1 = value1
field2 = value2
field3 = value3 +

+

field4 = value4 +
+

+

field5 = value5

+
+
+
+ + Object:Type... + + + + + + +
+
+
+

+ Object:Type +

+
+

field1 = value1
field2 = value2
field3 = value3 +

+

field4 = value4

+
+
+
+ + Object:Type... + + + + + + +
+
+
+

+ Object:Type +

+
+

field1 = value1
field2 = value2
field3 = value3 +

+

field4 = value4 +
+

+

field5 = value5

+
+
+
+ + Object:Type... + + + + + + +
+
+
+

+ Object:Type +

+
+

field1 = value1
field2 = value2
field3 = value3 +

+

field4 = value4 +
+

+

field5 = value5 +
+

+

field6 = value6 +
+

+

field7 = value7 +
+

+

field8 = value8 +
+

+

field9 = value9 +
+

+

field10 = value10 +
+

+

field11 = value11 +
+

+

field12 = value12

+
+
+
+ + Object:Type... + + + + + + + + Text is not SVG - cannot display + + + \ No newline at end of file diff --git a/plone/namedfile/tests/test_blobfile.py b/plone/namedfile/tests/test_blobfile.py index d7df2fc5..d7482462 100644 --- a/plone/namedfile/tests/test_blobfile.py +++ b/plone/namedfile/tests/test_blobfile.py @@ -27,6 +27,7 @@ from zope.component import provideUtility from zope.interface.verify import verifyClass +import os import struct import transaction import unittest @@ -79,7 +80,7 @@ def testInterface(self): self.assertTrue(INamedBlobImage.implementedBy(NamedBlobImage)) self.assertTrue(verifyClass(INamedBlobFile, NamedBlobImage)) - def testDataMutatorWithLargeHeader(self): + def testDataMutatorWithLargeJPGHeader(self): from plone.namedfile.file import IMAGE_INFO_BYTES bogus_header_length = struct.pack(">H", IMAGE_INFO_BYTES * 2) @@ -93,6 +94,23 @@ def testDataMutatorWithLargeHeader(self): image._setData(data) self.assertEqual(image.getImageSize(), (1024, 680)) + def testDataMutatorWithLargeSVGHeader(self): + from plone.namedfile.file import IMAGE_INFO_BYTES + + to_big_header_data = b'd' * (IMAGE_INFO_BYTES * 2) + + data = ( + b'' + b'"' + ) + image = self._makeImage() + image._setData(data) + self.assertEqual(image.getImageSize(), (1024, 680)) + self.assertGreater(len(to_big_header_data), IMAGE_INFO_BYTES) + class TestImageFunctional(unittest.TestCase): diff --git a/plone/namedfile/tests/test_svg.py b/plone/namedfile/tests/test_svg.py index 0619f6c8..cc2a31c4 100644 --- a/plone/namedfile/tests/test_svg.py +++ b/plone/namedfile/tests/test_svg.py @@ -1,11 +1,11 @@ +import unittest + from plone.namedfile.file import NamedImage from plone.namedfile.tests import getFile from plone.namedfile.utils import get_contenttype from plone.namedfile.utils.svg_utils import dimension_int from plone.namedfile.utils.svg_utils import process_svg -import unittest - class TestSvg(unittest.TestCase): def test_get_contenttype(self): @@ -23,6 +23,25 @@ def test_process_svg(self): self.assertEqual(width, 158) self.assertEqual(height, 40) + def test_process_svg__indicate_header_truncation(self): + """ Check that we can detect SVG files where the file header was + larger than the requested first bytes. process_svg() should + return -1 as dimensions to indicate the truncation.""" + + truncated_data = getFile("image_large_header.svg", length=1024) + content_type, width, height = process_svg(truncated_data) + self.assertEqual(content_type, "image/svg+xml") + self.assertEqual(width, -1) + self.assertEqual(height, -1) + + def test_process_svg__can_handle_large_header(self): + + data = getFile("image_large_header.svg") + content_type, width, height = process_svg(data) + self.assertEqual(content_type, "image/svg+xml") + self.assertEqual(width, 1041) + self.assertEqual(height, 751) + def test_dimension_int(self): self.assertEqual(dimension_int("auto"), 0) diff --git a/plone/namedfile/utils/svg_utils.py b/plone/namedfile/utils/svg_utils.py index 004dedc5..59c8dd50 100644 --- a/plone/namedfile/utils/svg_utils.py +++ b/plone/namedfile/utils/svg_utils.py @@ -21,15 +21,16 @@ def process_svg(data): w = dimension_int(el.attrib.get("width")) h = dimension_int(el.attrib.get("height")) break - except et.ParseError: + w = w if w > 1 else 1 + h = h if h > 1 else 1 + except et.ParseError as e: + log.debug(f"Failed to parse SVG dimensions: {e}") pass if tag == "{http://www.w3.org/2000/svg}svg" or ( size == 1024 and b"http://www.w3.org/2000/svg" in data ): content_type = "image/svg+xml" - w = w if w > 1 else 1 - h = h if h > 1 else 1 return content_type, w, h