Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix content length on federation /thumbnail responses #17532

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions changelog.d/17532.bugfix
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Fix content-length on federation /thumbnail responses.
22 changes: 11 additions & 11 deletions synapse/media/_base.py
Original file line number Diff line number Diff line change
Expand Up @@ -60,8 +60,6 @@

if TYPE_CHECKING:
from synapse.server import HomeServer
from synapse.storage.databases.main.media_repository import LocalMedia


logger = logging.getLogger(__name__)

Expand Down Expand Up @@ -290,7 +288,9 @@ async def respond_with_multipart_responder(
clock: Clock,
request: SynapseRequest,
responder: "Optional[Responder]",
media_info: "LocalMedia",
media_type: str,
media_length: Optional[int],
upload_name: Optional[str],
) -> None:
"""
Responds to requests originating from the federation media `/download` endpoint by
Expand All @@ -314,24 +314,24 @@ async def respond_with_multipart_responder(
)
return

if media_info.media_type.lower().split(";", 1)[0] in INLINE_CONTENT_TYPES:
if media_type.lower().split(";", 1)[0] in INLINE_CONTENT_TYPES:
disposition = "inline"
else:
disposition = "attachment"

def _quote(x: str) -> str:
return urllib.parse.quote(x.encode("utf-8"))

if media_info.upload_name:
if _can_encode_filename_as_token(media_info.upload_name):
if upload_name:
if _can_encode_filename_as_token(upload_name):
disposition = "%s; filename=%s" % (
disposition,
media_info.upload_name,
upload_name,
)
else:
disposition = "%s; filename*=utf-8''%s" % (
disposition,
_quote(media_info.upload_name),
_quote(upload_name),
)

from synapse.media.media_storage import MultipartFileConsumer
Expand All @@ -341,14 +341,14 @@ def _quote(x: str) -> str:
multipart_consumer = MultipartFileConsumer(
clock,
request,
media_info.media_type,
media_type,
{},
disposition,
media_info.media_length,
media_length,
)

logger.debug("Responding to media request with responder %s", responder)
if media_info.media_length is not None:
if media_length is not None:
content_length = multipart_consumer.content_length()
assert content_length is not None
request.setHeader(b"Content-Length", b"%d" % (content_length,))
Expand Down
6 changes: 3 additions & 3 deletions synapse/media/media_repository.py
Original file line number Diff line number Diff line change
Expand Up @@ -471,7 +471,7 @@ async def get_local_media(
responder = await self.media_storage.fetch_media(file_info)
if federation:
await respond_with_multipart_responder(
self.clock, request, responder, media_info
self.clock, request, responder, media_type, media_length, upload_name
)
else:
await respond_with_responder(
Expand Down Expand Up @@ -1008,7 +1008,7 @@ async def generate_local_exact_thumbnail(
t_method: str,
t_type: str,
url_cache: bool,
) -> Optional[str]:
) -> Optional[Tuple[str, FileInfo]]:
input_path = await self.media_storage.ensure_media_is_in_local_cache(
FileInfo(None, media_id, url_cache=url_cache)
)
Expand Down Expand Up @@ -1070,7 +1070,7 @@ async def generate_local_exact_thumbnail(
t_len,
)

return output_path
return output_path, file_info

# Could not generate thumbnail.
return None
Expand Down
32 changes: 26 additions & 6 deletions synapse/media/thumbnailer.py
Original file line number Diff line number Diff line change
Expand Up @@ -348,7 +348,12 @@ async def select_or_generate_local_thumbnail(
if responder:
if for_federation:
await respond_with_multipart_responder(
self.hs.get_clock(), request, responder, media_info
self.hs.get_clock(),
request,
responder,
info.type,
info.length,
None,
)
return
else:
Expand All @@ -360,7 +365,7 @@ async def select_or_generate_local_thumbnail(
logger.debug("We don't have a thumbnail of that size. Generating")

# Okay, so we generate one.
file_path = await self.media_repo.generate_local_exact_thumbnail(
thumbnail_result = await self.media_repo.generate_local_exact_thumbnail(
media_id,
desired_width,
desired_height,
Expand All @@ -369,13 +374,18 @@ async def select_or_generate_local_thumbnail(
url_cache=bool(media_info.url_cache),
)

if file_path:
if thumbnail_result:
file_path, file_info = thumbnail_result
assert file_info.thumbnail is not None

if for_federation:
await respond_with_multipart_responder(
self.hs.get_clock(),
request,
FileResponder(self.hs, open(file_path, "rb")),
media_info,
file_info.thumbnail.type,
file_info.thumbnail.length,
None,
)
else:
await respond_with_file(self.hs, request, desired_type, file_path)
Expand Down Expand Up @@ -580,7 +590,12 @@ async def _select_and_respond_with_thumbnail(
if for_federation:
assert media_info is not None
await respond_with_multipart_responder(
self.hs.get_clock(), request, responder, media_info
self.hs.get_clock(),
request,
responder,
file_info.thumbnail.type,
file_info.thumbnail.length,
None,
)
return
else:
Expand Down Expand Up @@ -634,7 +649,12 @@ async def _select_and_respond_with_thumbnail(
if for_federation:
assert media_info is not None
await respond_with_multipart_responder(
self.hs.get_clock(), request, responder, media_info
self.hs.get_clock(),
request,
responder,
file_info.thumbnail.type,
file_info.thumbnail.length,
None,
)
else:
await respond_with_responder(
Expand Down
Loading