Ignore invalid character encodings.

matrix-org · Oct 14, 2021 · 76bff8a · abadger · Oct 14, 2021 · 76bff8a
1 parent e70a592
commit 76bff8a
Show file tree

Hide file tree

Showing 2 changed files with 33 additions and 16 deletions.
diff --git a/synapse/rest/media/v1/preview_url_resource.py b/synapse/rest/media/v1/preview_url_resource.py
@@ -632,9 +632,12 @@ def try_remove_parent_dirs(dirs: Iterable[str]) -> None:
             logger.debug("No media removed from url cache")
 
 
-def _normalise_encoding(encoding: str) -> str:
+def _normalise_encoding(encoding: str) -> Optional[str]:
     """Use the Python codec's name as the normalised entry."""
-    return codecs.lookup(encoding).name
+    try:
+        return codecs.lookup(encoding).name
+    except LookupError:
+        return None
 
 
 def get_html_media_encodings(body: bytes, content_type: Optional[str]) -> Iterable[str]:
@@ -668,16 +671,17 @@ def get_html_media_encodings(body: bytes, content_type: Optional[str]) -> Iterab
     match = _charset_match.search(body_start)
     if match:
         encoding = _normalise_encoding(match.group(1).decode("ascii"))
-        attempted_encodings.add(encoding)
-        yield encoding
+        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:
         encoding = _normalise_encoding(match.group(1).decode("ascii"))
-        if encoding not in attempted_encodings:
+        if encoding and encoding not in attempted_encodings:
             attempted_encodings.add(encoding)
             yield encoding
 
@@ -686,12 +690,12 @@ def get_html_media_encodings(body: bytes, content_type: Optional[str]) -> Iterab
         content_match = _content_type_match.match(content_type)
         if content_match:
             encoding = _normalise_encoding(content_match.group(1))
-            if encoding not in attempted_encodings:
+            if encoding and encoding not in attempted_encodings:
                 attempted_encodings.add(encoding)
                 yield encoding
 
     # Finally, fallback to UTF-8, then windows-1252.
-    for fallback in ("utf-8", "windows-1252"):
+    for fallback in ("utf-8", "cp1252"):
         if fallback not in attempted_encodings:
             yield fallback
 

diff --git a/tests/test_preview.py b/tests/test_preview.py
@@ -307,7 +307,7 @@ def test_invalid_encoding2(self):
         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 @@ def test_meta_charset(self):
         """,
             "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 @@ def test_meta_charset(self):
         """,
             "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 @@ def test_meta_charset_underscores(self):
         """,
             "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 @@ def test_xml_encoding(self):
         """,
             "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 @@ def test_meta_xml_encoding(self):
         """,
             "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,12 +399,12 @@ def test_content_type(self):
         )
         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."""
@@ -418,4 +418,17 @@ def test_duplicates(self):
         """,
             'text/html; charset="UTF_8"',
         )
-        self.assertEqual(list(encodings), ["utf-8", "windows-1252"])
+        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"])