From 2ccdbe289a03630f79765b7a137c8e4d3c968a7f Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Tue, 14 Jun 2022 12:13:42 -0400 Subject: [PATCH 1/6] Factor out getting the Open Graph tag contents. --- synapse/rest/media/v1/preview_html.py | 49 +++++++++++++++++++-------- 1 file changed, 35 insertions(+), 14 deletions(-) diff --git a/synapse/rest/media/v1/preview_html.py b/synapse/rest/media/v1/preview_html.py index ed8f21a483df..96792a57e9c0 100644 --- a/synapse/rest/media/v1/preview_html.py +++ b/synapse/rest/media/v1/preview_html.py @@ -146,6 +146,38 @@ def decode_body( return etree.fromstring(body, parser) +def _get_meta_tags( + tree: "etree.Element", property: str, prefix: str +) -> Dict[str, Optional[str]]: + """ + Search for meta tags prefixed with a particular string. + + Args: + tree: The parsed HTML document. + property: The name of the property which contains the tag name, e.g. + "property" for Open Graph. + prefix: The prefix on the property to search for, e.g. "og" for Open Graph. + + Returns: + A map of tag name to value. + """ + results: Dict[str, Optional[str]] = {} + for tag in tree.xpath( + f"//*/meta[starts-with(@{property}, '{prefix}:')][@content][not(@content='')]" + ): + # if we've got more than 50 tags, someone is taking the piss + if len(results) >= 50: + logger.warning( + "Skipping parsing of Open Graph for page with too many '%s:' tags", + prefix, + ) + return {} + + results[tag.attrib[property]] = tag.attrib["content"] + + return results + + def parse_html_to_open_graph(tree: "etree.Element") -> Dict[str, Optional[str]]: """ Parse the HTML document into an Open Graph response. @@ -160,10 +192,8 @@ def parse_html_to_open_graph(tree: "etree.Element") -> Dict[str, Optional[str]]: The Open Graph response as a dictionary. """ - # if we see any image URLs in the OG response, then spider them - # (although the client could choose to do this by asking for previews of those - # URLs to avoid DoSing the server) - + # Search for Open Graph (og:) meta tags, e.g.: + # # "og:type" : "video", # "og:url" : "https://www.youtube.com/watch?v=LXDBoHyjmtw", # "og:site_name" : "YouTube", @@ -176,16 +206,7 @@ def parse_html_to_open_graph(tree: "etree.Element") -> Dict[str, Optional[str]]: # "og:video:height" : "720", # "og:video:secure_url": "https://www.youtube.com/v/LXDBoHyjmtw?version=3", - og: Dict[str, Optional[str]] = {} - for tag in tree.xpath( - "//*/meta[starts-with(@property, 'og:')][@content][not(@content='')]" - ): - # if we've got more than 50 tags, someone is taking the piss - if len(og) >= 50: - logger.warning("Skipping OG for page with too many 'og:' tags") - return {} - - og[tag.attrib["property"]] = tag.attrib["content"] + og = _get_meta_tags(tree, "property", "og") # TODO: grab article: meta tags too, e.g.: From bd5660e2b5d2382f65d5503263db9ebda66d5fc9 Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Tue, 14 Jun 2022 12:17:28 -0400 Subject: [PATCH 2/6] Parse twitter: meta tags. --- synapse/rest/media/v1/preview_html.py | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/synapse/rest/media/v1/preview_html.py b/synapse/rest/media/v1/preview_html.py index 96792a57e9c0..2b5b2624ef51 100644 --- a/synapse/rest/media/v1/preview_html.py +++ b/synapse/rest/media/v1/preview_html.py @@ -217,6 +217,23 @@ def parse_html_to_open_graph(tree: "etree.Element") -> Dict[str, Optional[str]]: # "article:published_time" content="2016-03-31T19:58:24+00:00" /> # "article:modified_time" content="2016-04-01T18:31:53+00:00" /> + # Search for Twitter Card (twitter:) meta tags, e.g.: + # + # "twitter:site" : "@matrixdotorg" + # "twitter:creator" : "@matrixdotorg" + # + # Twitter cards tags also duplicate Open Graph tags. + # + # See https://developer.twitter.com/en/docs/twitter-for-websites/cards/guides/getting-started + twitter = _get_meta_tags(tree, "name", "twitter") + # Merge the Twitter values with the Open Graph values, but do not overwrite + # information from Open Graph tags. + for key, value in twitter.items(): + # Convert from Twitter to Open Graph properties. + key = "og" + key[7:] + if key not in og: + og[key] = value + if "og:title" not in og: # Attempt to find a title from the title tag, or the biggest header on the page. title = tree.xpath("((//title)[1] | (//h1)[1] | (//h2)[1] | (//h3)[1])/text()") From 46343867149e30708bac6b3b045e0f94b55327be Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Tue, 14 Jun 2022 12:19:59 -0400 Subject: [PATCH 3/6] Clarify a comment. --- synapse/rest/media/v1/preview_html.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/synapse/rest/media/v1/preview_html.py b/synapse/rest/media/v1/preview_html.py index 2b5b2624ef51..bb7cf918d097 100644 --- a/synapse/rest/media/v1/preview_html.py +++ b/synapse/rest/media/v1/preview_html.py @@ -208,8 +208,9 @@ def parse_html_to_open_graph(tree: "etree.Element") -> Dict[str, Optional[str]]: og = _get_meta_tags(tree, "property", "og") - # TODO: grab article: meta tags too, e.g.: - + # TODO: Search for properties specific to the different Open Graph types, + # such as article: meta tags, e.g.: + # # "article:publisher" : "https://www.facebook.com/thethudonline" /> # "article:author" content="https://www.facebook.com/thethudonline" /> # "article:tag" content="baby" /> From f211ce922e0e181a43dd3e1629b247c0a48726fa Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Tue, 14 Jun 2022 12:25:59 -0400 Subject: [PATCH 4/6] Ignore some invalid Open Graph properties from Twitter. --- synapse/rest/media/v1/preview_html.py | 51 +++++++++++++++++++++++---- 1 file changed, 45 insertions(+), 6 deletions(-) diff --git a/synapse/rest/media/v1/preview_html.py b/synapse/rest/media/v1/preview_html.py index bb7cf918d097..c826a13093d0 100644 --- a/synapse/rest/media/v1/preview_html.py +++ b/synapse/rest/media/v1/preview_html.py @@ -15,7 +15,16 @@ import itertools import logging import re -from typing import TYPE_CHECKING, Dict, Generator, Iterable, Optional, Set, Union +from typing import ( + TYPE_CHECKING, + Callable, + Dict, + Generator, + Iterable, + Optional, + Set, + Union, +) if TYPE_CHECKING: from lxml import etree @@ -147,7 +156,10 @@ def decode_body( def _get_meta_tags( - tree: "etree.Element", property: str, prefix: str + tree: "etree.Element", + property: str, + prefix: str, + property_mapper: Optional[Callable[[str], Optional[str]]] = None, ) -> Dict[str, Optional[str]]: """ Search for meta tags prefixed with a particular string. @@ -157,6 +169,8 @@ def _get_meta_tags( property: The name of the property which contains the tag name, e.g. "property" for Open Graph. prefix: The prefix on the property to search for, e.g. "og" for Open Graph. + property_mapper: An optional callable to map the property to the Open Graph + form. Can return None for a key to ignore that key. Returns: A map of tag name to value. @@ -173,11 +187,38 @@ def _get_meta_tags( ) return {} - results[tag.attrib[property]] = tag.attrib["content"] + key = tag.attrib[property] + if property_mapper: + key = property_mapper(key) + # None is a special value used to ignore a value. + if key is None: + continue + + results[key] = tag.attrib["content"] return results +def _map_twitter_to_open_graph(key: str) -> Optional[str]: + """ + Map a Twitter card property to the analogous Open Graph property. + + Args: + key: The Twitter card property (starts with "twitter:"). + + Returns: + The Open Graph property (starts with "og:") or None to have this property + be ignored. + """ + # Twitter card properties with no analogous Open Graph property. + if key == "twitter:card" or key == "twitter:creator": + return None + if key == "twitter:site": + return "og:site_name" + # Otherwise, swap twitter to og. + return "og" + key[7:] + + def parse_html_to_open_graph(tree: "etree.Element") -> Dict[str, Optional[str]]: """ Parse the HTML document into an Open Graph response. @@ -226,12 +267,10 @@ def parse_html_to_open_graph(tree: "etree.Element") -> Dict[str, Optional[str]]: # Twitter cards tags also duplicate Open Graph tags. # # See https://developer.twitter.com/en/docs/twitter-for-websites/cards/guides/getting-started - twitter = _get_meta_tags(tree, "name", "twitter") + twitter = _get_meta_tags(tree, "name", "twitter", _map_twitter_to_open_graph) # Merge the Twitter values with the Open Graph values, but do not overwrite # information from Open Graph tags. for key, value in twitter.items(): - # Convert from Twitter to Open Graph properties. - key = "og" + key[7:] if key not in og: og[key] = value From 1eec5ac0d8ffd73f0a1a02eb67235060582cb639 Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Tue, 14 Jun 2022 12:58:05 -0400 Subject: [PATCH 5/6] Add tests. --- tests/rest/media/v1/test_html_preview.py | 41 ++++++++++++++++++++++++ 1 file changed, 41 insertions(+) diff --git a/tests/rest/media/v1/test_html_preview.py b/tests/rest/media/v1/test_html_preview.py index ea9e5889bf7b..cbdf210aef9f 100644 --- a/tests/rest/media/v1/test_html_preview.py +++ b/tests/rest/media/v1/test_html_preview.py @@ -370,6 +370,47 @@ def test_windows_1252(self) -> None: og = parse_html_to_open_graph(tree) self.assertEqual(og, {"og:title": "รณ", "og:description": "Some text."}) + def test_twitter_tag(self) -> None: + """Twitter card tags should be used if nothing else is available.""" + html = b""" + + + + + + """ + tree = decode_body(html, "http://example.com/test.html") + og = parse_html_to_open_graph(tree) + self.assertEqual( + og, + { + "og:title": None, + "og:description": "Description", + "og:site_name": "@matrixdotorg", + }, + ) + + # But they shouldn't override Open Graph values. + html = b""" + + + + + + + + """ + tree = decode_body(html, "http://example.com/test.html") + og = parse_html_to_open_graph(tree) + self.assertEqual( + og, + { + "og:title": None, + "og:description": "Real Description", + "og:site_name": "matrix.org", + }, + ) + class MediaEncodingTestCase(unittest.TestCase): def test_meta_charset(self) -> None: From fc90a7f3a9bf6ba516c8897379677eb4f5f90b45 Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Tue, 14 Jun 2022 13:40:37 -0400 Subject: [PATCH 6/6] Newsfragment --- changelog.d/13056.feature | 1 + 1 file changed, 1 insertion(+) create mode 100644 changelog.d/13056.feature diff --git a/changelog.d/13056.feature b/changelog.d/13056.feature new file mode 100644 index 000000000000..219e2f6c1e24 --- /dev/null +++ b/changelog.d/13056.feature @@ -0,0 +1 @@ +Improve URL previews for sites which only provide Twitter Card metadata, e.g. LWN.net.