Do not fail completely if oEmbed autodiscovery fails. (#15092)
Previously if an autodiscovered oEmbed request failed (e.g. the oEmbed endpoint is down or does not exist) then the entire URL preview would fail. Instead we now return everything we can, even if this additional request fails.
This commit is contained in:
Patrick Cloke
GitHub
parent
f8a584ed02
commit
682151a464
|
|
@ -0,0 +1 @@
|
||||||
|
Fix a long-standing bug where a URL preview would break if the discovered oEmbed failed to download.
|
||||||
|
|
@ -163,6 +163,10 @@ class PreviewUrlResource(DirectServeJsonResource):
|
||||||
7. Stores the result in the database cache.
|
7. Stores the result in the database cache.
|
||||||
4. Returns the result.
|
4. Returns the result.
|
||||||
|
|
||||||
|
If any additional requests (e.g. from oEmbed autodiscovery, step 5.3 or
|
||||||
|
image thumbnailing, step 5.4 or 6.4) fails then the URL preview as a whole
|
||||||
|
does not fail. As much information as possible is returned.
|
||||||
|
|
||||||
The in-memory cache expires after 1 hour.
|
The in-memory cache expires after 1 hour.
|
||||||
|
|
||||||
Expired entries in the database cache (and their associated media files) are
|
Expired entries in the database cache (and their associated media files) are
|
||||||
|
|
@ -364,16 +368,25 @@ class PreviewUrlResource(DirectServeJsonResource):
|
||||||
oembed_url = self._oembed.autodiscover_from_html(tree)
|
oembed_url = self._oembed.autodiscover_from_html(tree)
|
||||||
og_from_oembed: JsonDict = {}
|
og_from_oembed: JsonDict = {}
|
||||||
if oembed_url:
|
if oembed_url:
|
||||||
oembed_info = await self._handle_url(
|
try:
|
||||||
oembed_url, user, allow_data_urls=True
|
oembed_info = await self._handle_url(
|
||||||
)
|
oembed_url, user, allow_data_urls=True
|
||||||
(
|
)
|
||||||
og_from_oembed,
|
except Exception as e:
|
||||||
author_name,
|
# Fetching the oEmbed info failed, don't block the entire URL preview.
|
||||||
expiration_ms,
|
logger.warning(
|
||||||
) = await self._handle_oembed_response(
|
"oEmbed fetch failed during URL preview: %s errored with %s",
|
||||||
url, oembed_info, expiration_ms
|
oembed_url,
|
||||||
)
|
e,
|
||||||
|
)
|
||||||
|
else:
|
||||||
|
(
|
||||||
|
og_from_oembed,
|
||||||
|
author_name,
|
||||||
|
expiration_ms,
|
||||||
|
) = await self._handle_oembed_response(
|
||||||
|
url, oembed_info, expiration_ms
|
||||||
|
)
|
||||||
|
|
||||||
# Parse Open Graph information from the HTML in case the oEmbed
|
# Parse Open Graph information from the HTML in case the oEmbed
|
||||||
# response failed or is incomplete.
|
# response failed or is incomplete.
|
||||||
|
|
|
||||||
|
|
@ -657,7 +657,7 @@ class URLPreviewTests(unittest.HomeserverTestCase):
|
||||||
"""If the preview image doesn't exist, ensure some data is returned."""
|
"""If the preview image doesn't exist, ensure some data is returned."""
|
||||||
self.lookups["matrix.org"] = [(IPv4Address, "10.1.2.3")]
|
self.lookups["matrix.org"] = [(IPv4Address, "10.1.2.3")]
|
||||||
|
|
||||||
end_content = (
|
result = (
|
||||||
b"""<html><body><img src="http://cdn.matrix.org/foo.jpg"></body></html>"""
|
b"""<html><body><img src="http://cdn.matrix.org/foo.jpg"></body></html>"""
|
||||||
)
|
)
|
||||||
|
|
||||||
|
|
@ -678,8 +678,8 @@ class URLPreviewTests(unittest.HomeserverTestCase):
|
||||||
b"HTTP/1.0 200 OK\r\nContent-Length: %d\r\n"
|
b"HTTP/1.0 200 OK\r\nContent-Length: %d\r\n"
|
||||||
b'Content-Type: text/html; charset="utf8"\r\n\r\n'
|
b'Content-Type: text/html; charset="utf8"\r\n\r\n'
|
||||||
)
|
)
|
||||||
% (len(end_content),)
|
% (len(result),)
|
||||||
+ end_content
|
+ result
|
||||||
)
|
)
|
||||||
|
|
||||||
self.pump()
|
self.pump()
|
||||||
|
|
@ -688,6 +688,44 @@ class URLPreviewTests(unittest.HomeserverTestCase):
|
||||||
# The image should not be in the result.
|
# The image should not be in the result.
|
||||||
self.assertNotIn("og:image", channel.json_body)
|
self.assertNotIn("og:image", channel.json_body)
|
||||||
|
|
||||||
|
def test_oembed_failure(self) -> None:
|
||||||
|
"""If the autodiscovered oEmbed URL fails, ensure some data is returned."""
|
||||||
|
self.lookups["matrix.org"] = [(IPv4Address, "10.1.2.3")]
|
||||||
|
|
||||||
|
result = b"""
|
||||||
|
<title>oEmbed Autodiscovery Fail</title>
|
||||||
|
<link rel="alternate" type="application/json+oembed"
|
||||||
|
href="http://example.com/oembed?url=http%3A%2F%2Fmatrix.org&format=json"
|
||||||
|
title="matrixdotorg" />
|
||||||
|
"""
|
||||||
|
|
||||||
|
channel = self.make_request(
|
||||||
|
"GET",
|
||||||
|
"preview_url?url=http://matrix.org",
|
||||||
|
shorthand=False,
|
||||||
|
await_result=False,
|
||||||
|
)
|
||||||
|
self.pump()
|
||||||
|
|
||||||
|
client = self.reactor.tcpClients[0][2].buildProtocol(None)
|
||||||
|
server = AccumulatingProtocol()
|
||||||
|
server.makeConnection(FakeTransport(client, self.reactor))
|
||||||
|
client.makeConnection(FakeTransport(server, self.reactor))
|
||||||
|
client.dataReceived(
|
||||||
|
(
|
||||||
|
b"HTTP/1.0 200 OK\r\nContent-Length: %d\r\n"
|
||||||
|
b'Content-Type: text/html; charset="utf8"\r\n\r\n'
|
||||||
|
)
|
||||||
|
% (len(result),)
|
||||||
|
+ result
|
||||||
|
)
|
||||||
|
|
||||||
|
self.pump()
|
||||||
|
self.assertEqual(channel.code, 200)
|
||||||
|
|
||||||
|
# The image should not be in the result.
|
||||||
|
self.assertEqual(channel.json_body["og:title"], "oEmbed Autodiscovery Fail")
|
||||||
|
|
||||||
def test_data_url(self) -> None:
|
def test_data_url(self) -> None:
|
||||||
"""
|
"""
|
||||||
Requesting to preview a data URL is not supported.
|
Requesting to preview a data URL is not supported.
|
||||||
|
|
|
||||||
Loading…
Reference in New Issue