Repository navigation
Save media content with random names #699
Description
Refactoring Proposal
Sometimes users upload content with special characters on the filename, which has and could cause problems while searching for the file using django admin or S3, leading to "file not found" errors when special characters are automatically escaped by S3 or Django.
Describe refactoring alternatives to redesign the code
- Create a new field on models called "original_filename" to save the original filename in case its needed
- Create a UUID name that is based on timestamp to avoid collisions and use that to save the media
- Create a migration to rename all the objects in the current database with the same pattern of UUID.
- Be careful to keep the extension on the end of the name, its used in multiple places
Activity
vjpixel commentedon Apr 25, 2026
Análise — minha sugestão é fechar como
not plannedou parar de priorizar.O que motivou a issue (jul/2025):
photo.jpg).Estado hoje:
os.path.basename()em todo upload, entãosubdir/foo.gifvirafoo.gifantes de chegar no Storage.Storage.get_valid_name()(default emS3Boto3Storage) escapa caracteres não--\w.no basename —meu arquivo!.gifvirameu_arquivo.gif.PublicMediaStoragetemfile_overwrite = False(emsrc/config/storage_backends.py), então colisões viramphoto_xxx.jpgautomaticamente.O que ainda fica fora:
_, espaços viram_).original_filename— é alto-risco (URLs públicas mudam, cache/CDN podem servir 404 durante a janela, links externos no Pulse e em redes sociais quebram).Recomendação: fechar como
not planned. O problema agudo está resolvido via #720 + comportamento default do Django. A reescrita para UUIDs traz risco real e ganho cosmético.Se quiser manter aberta como tracking pra eventual 2.0.0, deixar assignada ao Pablo.
Generated by Claude Code
pablodiegoss commentedon May 12, 2026
Using user defined names is not recommended for files, The #720 solves another problem that we were using the base folder on edits, creating recursive folders. Using UUID names or not, including the folder would still recursively create folders.
However, we still should not use user defined names for files. This is a privacy issue, that could allow users to upload NSFW, political, religious name to files that would be later made public via URLs.
I'll keep this issue as is
vjpixel commentedon Sep 17, 2026
Agreed, and the privacy framing is the stronger argument — a user-supplied name becoming part of a public URL is a different and worse problem than the S3 escaping this issue opens with. #982 is built on that reasoning now.
Scoping it, most of the surface is already covered: objects land in
objects/<pk>/source.<ext>and markers inmarkers/<pk>/original.<ext>, neither of which carries a user-supplied name.Sound.filewas the one still writingsounds/<the user's filename>, so that's what #982 changes — tosounds/<uuid>.<ext>, keeping the original infile_name_original, which already existed.One related gap I found and did not fix there, since it's a different code path: markers are only eventually safe. The upload first lands under the user's own filename, and
marker_utils.py:97-101deletes it afterwards inside a bareexcept Exception: pass. If that delete ever fails, the user-named object stays in public storage and nobody is told. Same exposure you're describing — worth making that failure loud.I also left the backfill out on purpose: renaming live S3 objects wants to be a resumable management command rather than a migration, since a half-applied run leaves rows pointing at files that no longer exist. Want me to open that separately?
Generated by Claude Code
vjpixel commentedon Sep 17, 2026
Answering the two loose ends I left here, both now open as their own work rather than widening #982.
The backfill → #991, with #992 implementing it as a resumable management command rather than the data migration this issue originally asked for. The reasoning: a migration renames live S3 objects inside a deploy, cannot be resumed after a partial run, and gives no way to see what would change first. The command is idempotent (so resuming is just running it again), writes the new object before repointing the row and only then deletes the old one (so an interruption leaves an orphan, never a row pointing at a missing key), and takes
--dry-runand--limitso the first batch can be small and inspected. A failing row is logged and skipped instead of aborting the batch.#992 is based on #982's branch rather than
develop, because it reuses thesound_file_pathhelper so backfilled names match new ones instead of reimplementing the pattern.The silent marker cleanup → #989, fixed in #990. The bare
except Exception: passis now narrowed toOSError/SuspiciousFileOperationand logs the path left behind, so the case you describe stops being invisible. The upload still succeeds either way.One detail that surfaced while testing it, worth knowing for this issue generally: Django's
get_valid_namesanitises but does not anonymise.Minha imagem #1.pngreaches storage asMinha_imagem_1.png— spaces to underscores,#dropped — still recognisably the uploader's chosen name. So the default storage behaviour does not mitigate the exposure, it just tidies the characters.Generated by Claude Code
Merge pull request #982 from memeLab/feature/699-uuid-media-filenames