Ensure we delete media if we reject due to spam check (#17246)

Fixes up #17239

We need to keep the spam check within the `try/except` block. Also makes
it so that we don't enter the top span twice.

Also also ensures that we get the right thumbnail length.
This commit is contained in:
Erik Johnston 2024-05-30 11:22:19 +01:00 committed by GitHub
parent 466f344547
commit 8bd9ff0783
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
3 changed files with 32 additions and 31 deletions

1
changelog.d/17246.misc Normal file
View File

@ -0,0 +1 @@
Fix errors in logs about closing incorrect logging contexts when media gets rejected by a module.

View File

@ -1049,6 +1049,11 @@ class MediaRepository:
finally: finally:
t_byte_source.close() t_byte_source.close()
# We flush and close the file to ensure that the bytes have
# been written before getting the size.
f.flush()
f.close()
t_len = os.path.getsize(fname) t_len = os.path.getsize(fname)
# Write to database # Write to database

View File

@ -137,24 +137,11 @@ class MediaStorage:
dirname = os.path.dirname(fname) dirname = os.path.dirname(fname)
os.makedirs(dirname, exist_ok=True) os.makedirs(dirname, exist_ok=True)
main_media_repo_write_trace_scope = start_active_span(
"writing to main media repo"
)
main_media_repo_write_trace_scope.__enter__()
with main_media_repo_write_trace_scope:
try: try:
with start_active_span("writing to main media repo"):
with open(fname, "wb") as f: with open(fname, "wb") as f:
yield f, fname yield f, fname
except Exception as e:
try:
os.remove(fname)
except Exception:
pass
raise e from None
with start_active_span("writing to other storage providers"): with start_active_span("writing to other storage providers"):
spam_check = ( spam_check = (
await self._spam_checker_module_callbacks.check_media_file_for_spam( await self._spam_checker_module_callbacks.check_media_file_for_spam(
@ -174,6 +161,14 @@ class MediaStorage:
with start_active_span(str(provider)): with start_active_span(str(provider)):
await provider.store_file(path, file_info) await provider.store_file(path, file_info)
except Exception as e:
try:
os.remove(fname)
except Exception:
pass
raise e from None
async def fetch_media(self, file_info: FileInfo) -> Optional[Responder]: async def fetch_media(self, file_info: FileInfo) -> Optional[Responder]:
"""Attempts to fetch media described by file_info from the local cache """Attempts to fetch media described by file_info from the local cache
and configured storage providers. and configured storage providers.