forked from MirrorHub/synapse
Do not break URL previews if an image is unreachable. (#12950)
Avoid breaking a URL preview completely if the chosen image 404s or is unreachable for some other reason (e.g. DNS).
This commit is contained in:
parent
1acc897c31
commit
148fe58a24
3 changed files with 53 additions and 6 deletions
1
changelog.d/12950.bugfix
Normal file
1
changelog.d/12950.bugfix
Normal file
|
@ -0,0 +1 @@
|
||||||
|
Fix a long-standing bug where a URL preview would break if the image failed to download.
|
|
@ -586,12 +586,16 @@ class PreviewUrlResource(DirectServeJsonResource):
|
||||||
og: The Open Graph dictionary. This is modified with image information.
|
og: The Open Graph dictionary. This is modified with image information.
|
||||||
"""
|
"""
|
||||||
# If there's no image or it is blank, there's nothing to do.
|
# If there's no image or it is blank, there's nothing to do.
|
||||||
if "og:image" not in og or not og["og:image"]:
|
if "og:image" not in og:
|
||||||
|
return
|
||||||
|
|
||||||
|
# Remove the raw image URL, this will be replaced with an MXC URL, if successful.
|
||||||
|
image_url = og.pop("og:image")
|
||||||
|
if not image_url:
|
||||||
return
|
return
|
||||||
|
|
||||||
# The image URL from the HTML might be relative to the previewed page,
|
# The image URL from the HTML might be relative to the previewed page,
|
||||||
# convert it to an URL which can be requested directly.
|
# convert it to an URL which can be requested directly.
|
||||||
image_url = og["og:image"]
|
|
||||||
url_parts = urlparse(image_url)
|
url_parts = urlparse(image_url)
|
||||||
if url_parts.scheme != "data":
|
if url_parts.scheme != "data":
|
||||||
image_url = urljoin(media_info.uri, image_url)
|
image_url = urljoin(media_info.uri, image_url)
|
||||||
|
@ -599,7 +603,16 @@ class PreviewUrlResource(DirectServeJsonResource):
|
||||||
# FIXME: it might be cleaner to use the same flow as the main /preview_url
|
# FIXME: it might be cleaner to use the same flow as the main /preview_url
|
||||||
# request itself and benefit from the same caching etc. But for now we
|
# request itself and benefit from the same caching etc. But for now we
|
||||||
# just rely on the caching on the master request to speed things up.
|
# just rely on the caching on the master request to speed things up.
|
||||||
|
try:
|
||||||
image_info = await self._handle_url(image_url, user, allow_data_urls=True)
|
image_info = await self._handle_url(image_url, user, allow_data_urls=True)
|
||||||
|
except Exception as e:
|
||||||
|
# Pre-caching the image failed, don't block the entire URL preview.
|
||||||
|
logger.warning(
|
||||||
|
"Pre-caching image failed during URL preview: %s errored with %s",
|
||||||
|
image_url,
|
||||||
|
e,
|
||||||
|
)
|
||||||
|
return
|
||||||
|
|
||||||
if _is_media(image_info.media_type):
|
if _is_media(image_info.media_type):
|
||||||
# TODO: make sure we don't choke on white-on-transparent images
|
# TODO: make sure we don't choke on white-on-transparent images
|
||||||
|
@ -611,13 +624,11 @@ class PreviewUrlResource(DirectServeJsonResource):
|
||||||
og["og:image:width"] = dims["width"]
|
og["og:image:width"] = dims["width"]
|
||||||
og["og:image:height"] = dims["height"]
|
og["og:image:height"] = dims["height"]
|
||||||
else:
|
else:
|
||||||
logger.warning("Couldn't get dims for %s", og["og:image"])
|
logger.warning("Couldn't get dims for %s", image_url)
|
||||||
|
|
||||||
og["og:image"] = f"mxc://{self.server_name}/{image_info.filesystem_id}"
|
og["og:image"] = f"mxc://{self.server_name}/{image_info.filesystem_id}"
|
||||||
og["og:image:type"] = image_info.media_type
|
og["og:image:type"] = image_info.media_type
|
||||||
og["matrix:image:size"] = image_info.media_length
|
og["matrix:image:size"] = image_info.media_length
|
||||||
else:
|
|
||||||
del og["og:image"]
|
|
||||||
|
|
||||||
async def _handle_oembed_response(
|
async def _handle_oembed_response(
|
||||||
self, url: str, media_info: MediaInfo, expiration_ms: int
|
self, url: str, media_info: MediaInfo, expiration_ms: int
|
||||||
|
|
|
@ -656,6 +656,41 @@ class URLPreviewTests(unittest.HomeserverTestCase):
|
||||||
server.data,
|
server.data,
|
||||||
)
|
)
|
||||||
|
|
||||||
|
def test_nonexistent_image(self) -> None:
|
||||||
|
"""If the preview image doesn't exist, ensure some data is returned."""
|
||||||
|
self.lookups["matrix.org"] = [(IPv4Address, "10.1.2.3")]
|
||||||
|
|
||||||
|
end_content = (
|
||||||
|
b"""<html><body><img src="http://cdn.matrix.org/foo.jpg"></body></html>"""
|
||||||
|
)
|
||||||
|
|
||||||
|
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(end_content),)
|
||||||
|
+ end_content
|
||||||
|
)
|
||||||
|
|
||||||
|
self.pump()
|
||||||
|
self.assertEqual(channel.code, 200)
|
||||||
|
|
||||||
|
# The image should not be in the result.
|
||||||
|
self.assertNotIn("og:image", channel.json_body)
|
||||||
|
|
||||||
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 a new issue