diff options
| -rw-r--r-- | changelog.d/11089.bugfix | 1 | ||||
| -rw-r--r-- | synapse/rest/media/v1/preview_url_resource.py | 34 | ||||
| -rw-r--r-- | tests/test_preview.py | 43 |
3 files changed, 64 insertions, 14 deletions
diff --git a/changelog.d/11089.bugfix b/changelog.d/11089.bugfix new file mode 100644 index 0000000000..dc35c86440 --- /dev/null +++ b/changelog.d/11089.bugfix @@ -0,0 +1 @@ +Fix a long-standing bug when attempting to preview URLs which are in the `windows-1252` character encoding. diff --git a/synapse/rest/media/v1/preview_url_resource.py b/synapse/rest/media/v1/preview_url_resource.py index 7ee91a0c05..278fd901e2 100644 --- a/synapse/rest/media/v1/preview_url_resource.py +++ b/synapse/rest/media/v1/preview_url_resource.py @@ -12,6 +12,7 @@ # WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. # See the License for the specific language governing permissions and # limitations under the License. +import codecs import datetime import errno import fnmatch @@ -22,7 +23,7 @@ import re import shutil import sys import traceback -from typing import TYPE_CHECKING, Dict, Generator, Iterable, Optional, Tuple, Union +from typing import TYPE_CHECKING, Dict, Generator, Iterable, Optional, Set, Tuple, Union from urllib import parse as urlparse import attr @@ -631,6 +632,14 @@ class PreviewUrlResource(DirectServeJsonResource): logger.debug("No media removed from url cache") +def _normalise_encoding(encoding: str) -> Optional[str]: + """Use the Python codec's name as the normalised entry.""" + try: + return codecs.lookup(encoding).name + except LookupError: + return None + + def get_html_media_encodings(body: bytes, content_type: Optional[str]) -> Iterable[str]: """ Get potential encoding of the body based on the (presumably) HTML body or the content-type header. @@ -652,30 +661,43 @@ def get_html_media_encodings(body: bytes, content_type: Optional[str]) -> Iterab Returns: The character encoding of the body, as a string. """ + # There's no point in returning an encoding more than once. + attempted_encodings: Set[str] = set() + # Limit searches to the first 1kb, since it ought to be at the top. body_start = body[:1024] # Check if it has an encoding set in a meta tag. match = _charset_match.search(body_start) if match: - yield match.group(1).decode("ascii") + encoding = _normalise_encoding(match.group(1).decode("ascii")) + if encoding: + attempted_encodings.add(encoding) + yield encoding # TODO Support <meta http-equiv="Content-Type" content="text/html; charset=utf-8"/> # Check if it has an XML document with an encoding. match = _xml_encoding_match.match(body_start) if match: - yield match.group(1).decode("ascii") + encoding = _normalise_encoding(match.group(1).decode("ascii")) + if encoding and encoding not in attempted_encodings: + attempted_encodings.add(encoding) + yield encoding # Check the HTTP Content-Type header for a character set. if content_type: content_match = _content_type_match.match(content_type) if content_match: - yield content_match.group(1) + encoding = _normalise_encoding(content_match.group(1)) + if encoding and encoding not in attempted_encodings: + attempted_encodings.add(encoding) + yield encoding # Finally, fallback to UTF-8, then windows-1252. - yield "utf-8" - yield "windows-1252" + for fallback in ("utf-8", "cp1252"): + if fallback not in attempted_encodings: + yield fallback def decode_body( diff --git a/tests/test_preview.py b/tests/test_preview.py index c6789017bc..9a576f9a4e 100644 --- a/tests/test_preview.py +++ b/tests/test_preview.py @@ -307,7 +307,7 @@ class CalcOgTestCase(unittest.TestCase): self.assertEqual(og, {"og:title": "ÿÿ Foo", "og:description": "Some text."}) def test_windows_1252(self): - """A body which uses windows-1252, but doesn't declare that.""" + """A body which uses cp1252, but doesn't declare that.""" html = b""" <html> <head><title>\xf3</title></head> @@ -333,7 +333,7 @@ class MediaEncodingTestCase(unittest.TestCase): """, "text/html", ) - self.assertEqual(list(encodings), ["ascii", "utf-8", "windows-1252"]) + self.assertEqual(list(encodings), ["ascii", "utf-8", "cp1252"]) # A less well-formed version. encodings = get_html_media_encodings( @@ -345,7 +345,7 @@ class MediaEncodingTestCase(unittest.TestCase): """, "text/html", ) - self.assertEqual(list(encodings), ["ascii", "utf-8", "windows-1252"]) + self.assertEqual(list(encodings), ["ascii", "utf-8", "cp1252"]) def test_meta_charset_underscores(self): """A character encoding contains underscore.""" @@ -358,7 +358,7 @@ class MediaEncodingTestCase(unittest.TestCase): """, "text/html", ) - self.assertEqual(list(encodings), ["Shift_JIS", "utf-8", "windows-1252"]) + self.assertEqual(list(encodings), ["shift_jis", "utf-8", "cp1252"]) def test_xml_encoding(self): """A character encoding is found via the meta tag.""" @@ -370,7 +370,7 @@ class MediaEncodingTestCase(unittest.TestCase): """, "text/html", ) - self.assertEqual(list(encodings), ["ascii", "utf-8", "windows-1252"]) + self.assertEqual(list(encodings), ["ascii", "utf-8", "cp1252"]) def test_meta_xml_encoding(self): """Meta tags take precedence over XML encoding.""" @@ -384,7 +384,7 @@ class MediaEncodingTestCase(unittest.TestCase): """, "text/html", ) - self.assertEqual(list(encodings), ["UTF-16", "ascii", "utf-8", "windows-1252"]) + self.assertEqual(list(encodings), ["utf-16", "ascii", "utf-8", "cp1252"]) def test_content_type(self): """A character encoding is found via the Content-Type header.""" @@ -399,9 +399,36 @@ class MediaEncodingTestCase(unittest.TestCase): ) for header in headers: encodings = get_html_media_encodings(b"", header) - self.assertEqual(list(encodings), ["ascii", "utf-8", "windows-1252"]) + self.assertEqual(list(encodings), ["ascii", "utf-8", "cp1252"]) def test_fallback(self): """A character encoding cannot be found in the body or header.""" encodings = get_html_media_encodings(b"", "text/html") - self.assertEqual(list(encodings), ["utf-8", "windows-1252"]) + self.assertEqual(list(encodings), ["utf-8", "cp1252"]) + + def test_duplicates(self): + """Ensure each encoding is only attempted once.""" + encodings = get_html_media_encodings( + b""" + <?xml version="1.0" encoding="utf8"?> + <html> + <head><meta charset="UTF-8"> + </head> + </html> + """, + 'text/html; charset="UTF_8"', + ) + self.assertEqual(list(encodings), ["utf-8", "cp1252"]) + + def test_unknown_invalid(self): + """A character encoding should be ignored if it is unknown or invalid.""" + encodings = get_html_media_encodings( + b""" + <html> + <head><meta charset="invalid"> + </head> + </html> + """, + 'text/html; charset="invalid"', + ) + self.assertEqual(list(encodings), ["utf-8", "cp1252"]) |