stash
This commit is contained in:
parent
8d96bc97c4
commit
ed04d98bf1
54 changed files with 661 additions and 1214 deletions
|
|
@ -24,9 +24,8 @@ THUMBNAIL_SIZES = (32, 64, 256)
|
|||
|
||||
|
||||
def _accessible_files(request):
|
||||
# Shared by media_urls and thumbnail_urls so both endpoints always agree on who can see
|
||||
# what - a file is visible if the requester is friends-or-self with whatever currently
|
||||
# references it (an inventory item, a profile picture) or it's their own staged photo.
|
||||
# Shared by media_urls and thumbnail_urls: a file is visible if the requester is
|
||||
# friends-or-self with whatever references it (item, profile picture), or it's their own staged photo.
|
||||
return File.objects.filter(
|
||||
Q(connected_items__owner__in=request.user.friends_or_self()) |
|
||||
Q(profile_picture_users__in=request.user.friends_or_self()) |
|
||||
|
|
@ -35,8 +34,7 @@ def _accessible_files(request):
|
|||
|
||||
|
||||
def _cache_headers(etag):
|
||||
# Content is addressed by its own hash and can never change under a given URL, so caches
|
||||
# (and the conditional-GET checks in both views below) can treat it as immutable forever.
|
||||
# Content is hash-addressed and can never change under a given URL, so it's cacheable forever.
|
||||
return {
|
||||
'ETag': etag,
|
||||
'Cache-Control': 'max-age=31536000, private, immutable',
|
||||
|
|
@ -49,23 +47,16 @@ def _cache_headers(etag):
|
|||
@permission_classes([IsAuthenticated])
|
||||
@authentication_classes([SignatureAuthentication])
|
||||
def media_urls(request, hash_path):
|
||||
# Note: CORS headers are NOT set here - django-cors-headers (CorsMiddleware,
|
||||
# configured in settings.py) adds them to every Django response automatically, so
|
||||
# setting them manually on these responses would just be redundant. The one exception
|
||||
# is the SERVE_X_ACCEL_REDIRECT path: nginx replaces this response entirely when it
|
||||
# follows the X-Accel-Redirect and serves the file itself, so the CORS header for that
|
||||
# case has to be configured in nginx's `location /redirect_media/` block instead.
|
||||
# CORS is added automatically by middleware, except via X-Accel-Redirect, where nginx's
|
||||
# /redirect_media/ block must set it instead.
|
||||
#
|
||||
# Looked up by the derived storage path (not by raw hash) because FileSerializer.name
|
||||
# (files/serializers.py) - used everywhere a file URL is handed to the frontend, e.g.
|
||||
# AuthenticatedImage's `src` - already returns this path via Django's FileField.url, and
|
||||
# the existing test suite (files/tests.py MediaUrlTestCase) exercises it this way too.
|
||||
# Looked up by the derived storage path, not the raw hash, to match FileSerializer.name
|
||||
# (used for AuthenticatedImage's `src`) and the existing test suite (MediaUrlTestCase).
|
||||
try:
|
||||
file = _accessible_files(request).get(file=hash_path)
|
||||
|
||||
# The access-control lookup above must happen before this check - otherwise a bare
|
||||
# hash + If-None-Match would let anyone probe "does a file with this hash exist" for
|
||||
# files they can't actually see.
|
||||
# Must run before this check, else a bare hash + If-None-Match would let anyone probe
|
||||
# file existence for files they can't see.
|
||||
if request.META.get('HTTP_IF_NONE_MATCH') == file.hash:
|
||||
return HttpResponse(status=status.HTTP_304_NOT_MODIFIED)
|
||||
|
||||
|
|
@ -79,9 +70,7 @@ def media_urls(request, hash_path):
|
|||
**cache_headers,
|
||||
})
|
||||
else:
|
||||
# Read via the FieldFile itself (works against whatever storage backend is
|
||||
# actually configured) rather than assuming file.file.path is a real filesystem
|
||||
# path - the test suite swaps in an in-memory backend where that isn't true.
|
||||
# Reads via FieldFile.open() (not file.file.path) since tests swap in an in-memory storage backend.
|
||||
with file.file.open('rb') as fh:
|
||||
content = fh.read()
|
||||
return HttpResponse(status=status.HTTP_200_OK,
|
||||
|
|
@ -94,9 +83,7 @@ def media_urls(request, hash_path):
|
|||
|
||||
|
||||
def _thumbnail_rel_path(file_hash, size):
|
||||
# Mirrors files/models.py's hash_upload() sharding, under its own `thumbnails/<size>/`
|
||||
# subtree - reachable through the same nginx `/redirect_media/` alias as originals when
|
||||
# served from real disk, no separate nginx location needed.
|
||||
# Mirrors hash_upload()'s sharding under thumbnails/<size>/, reachable via the same nginx alias as originals.
|
||||
return os.path.join('thumbnails', str(size), file_hash[:2], file_hash[2:4], file_hash[4:6],
|
||||
file_hash[6:] + '.jpg')
|
||||
|
||||
|
|
@ -116,22 +103,15 @@ def thumbnail_urls(request, size, hash_path):
|
|||
if request.META.get('HTTP_IF_NONE_MATCH') == etag:
|
||||
return HttpResponse(status=status.HTTP_304_NOT_MODIFIED)
|
||||
|
||||
# Read/write through the default storage backend, same as File.file itself, rather
|
||||
# than a hand-rolled filesystem path - correct regardless of storage backend (real
|
||||
# disk in production, in-memory under the test runner) and keeps the cache in the
|
||||
# same place originals live.
|
||||
# Read/write via default_storage, not a hand-rolled path, to work with both real-disk
|
||||
# and in-memory test storage.
|
||||
rel_path = _thumbnail_rel_path(file.hash, size)
|
||||
if not default_storage.exists(rel_path):
|
||||
# Thumbnails are always re-encoded as JPEG regardless of the original format -
|
||||
# smaller and simpler than preserving e.g. PNG transparency at this scale.
|
||||
# Always re-encoded as JPEG regardless of original format - simpler than preserving transparency at this scale.
|
||||
with file.file.open('rb') as fh:
|
||||
image = Image.open(fh)
|
||||
image.thumbnail((size, size))
|
||||
# Flatten through RGBA before dropping to RGB - some modes (grayscale+alpha,
|
||||
# palette-with-transparency, RGBA) store meaningless color/luminance data under
|
||||
# fully transparent pixels (often zeroed out, i.e. black). Converting straight
|
||||
# to RGB reveals that instead of "nothing there"; compositing onto an opaque
|
||||
# background first shows what the image is actually supposed to look like.
|
||||
# Flatten through RGBA before dropping to RGB. See docs/implementation.md#rgba-flattening-avoids-revealing-black-under-transparent-pixels.
|
||||
rgba = image.convert('RGBA')
|
||||
flattened = Image.new('RGB', rgba.size, (255, 255, 255))
|
||||
flattened.paste(rgba, mask=rgba.getchannel('A'))
|
||||
|
|
|
|||
|
|
@ -43,15 +43,7 @@ class FileManager(models.Manager):
|
|||
else:
|
||||
raise ValueError('data must be a base64 encoded string or file and hash must be provided')
|
||||
if not self.filter(hash=kwargs['hash']).exists():
|
||||
# The upload path is derived entirely from the hash (hash_upload, above), and hash
|
||||
# is DB-unique - so if no File row owns this hash yet, anything already sitting at
|
||||
# its computed path is necessarily a stale orphan (e.g. left behind by a bug in
|
||||
# cleanup code that deleted a File row without removing its stored bytes, or a
|
||||
# crashed upload). Clear it before saving instead of letting Django's storage layer
|
||||
# invent an alternate filename to avoid the "collision" - a suffixed name would
|
||||
# silently break every part of the app that derives this file's URL purely from its
|
||||
# hash (media serving, thumbnail generation, item/avatar attachment), and no future
|
||||
# caller could ever discover it again.
|
||||
# Clears a stale orphan already at this hash's canonical path before saving. See docs/implementation.md#stale-orphan-cleanup-at-the-canonical-hash-path.
|
||||
expected_path = hash_upload(SimpleNamespace(hash=kwargs['hash']), '')
|
||||
if default_storage.exists(expected_path):
|
||||
default_storage.delete(expected_path)
|
||||
|
|
|
|||
|
|
@ -113,12 +113,7 @@ class FilesTestCase(FilesTestMixin, ToolshedTestCase):
|
|||
self.assertEqual(countdir(DefaultStorage(), ''), 3)
|
||||
|
||||
def test_file_upload_reclaims_stale_orphan_at_canonical_path(self):
|
||||
# Reproduces a real incident: a File row gets deleted without its underlying stored
|
||||
# bytes being removed (e.g. a bug in some cleanup call site), leaving an orphan sitting
|
||||
# at the exact path hash_upload() would compute for that content. A later upload of the
|
||||
# same content must land back on that canonical path - not get silently suffixed by
|
||||
# Django's default collision-avoidance, which would make it unreachable to every part
|
||||
# of the app that derives a file's URL purely from its hash.
|
||||
# Regression test for a stale orphan at the canonical hash path. See docs/implementation.md#stale-orphan-cleanup-at-the-canonical-hash-path.
|
||||
expected_path = f"{self.f['hash4'][:2]}/{self.f['hash4'][2:4]}/{self.f['hash4'][4:6]}/{self.f['hash4'][6:]}"
|
||||
default_storage.save(expected_path, ContentFile(self.f['test_content4']))
|
||||
self.assertTrue(default_storage.exists(expected_path))
|
||||
|
|
@ -231,11 +226,8 @@ class ThumbnailUrlTestCase(FilesTestMixin, UserTestMixin, InventoryTestMixin, To
|
|||
self.prepare_properties()
|
||||
self.prepare_inventory()
|
||||
|
||||
# Each test method gets its own distinct image content (and therefore its own content
|
||||
# hash / thumbnail cache path) - InMemoryStorage isn't reset between test methods within
|
||||
# a run, so sharing one fixed image across methods risks one test's cached (or, in
|
||||
# test_thumbnail_served_from_cache_on_second_request's case, deliberately corrupted)
|
||||
# thumbnail leaking into another test's assertions.
|
||||
# Each test uses a distinct seeded image (own hash/cache path) since InMemoryStorage
|
||||
# isn't reset between test methods, so a shared image risks one test's cached thumbnail leaking into another's assertions.
|
||||
seed = zlib.crc32(self._testMethodName.encode()) % 256
|
||||
buffer = io.BytesIO()
|
||||
Image.new('RGB', (800, 600), (seed, 255 - seed, 128)).save(buffer, 'PNG')
|
||||
|
|
@ -254,8 +246,7 @@ class ThumbnailUrlTestCase(FilesTestMixin, UserTestMixin, InventoryTestMixin, To
|
|||
return os.path.join('thumbnails', str(size), h[:2], h[2:4], h[4:6], h[6:] + '.jpg')
|
||||
|
||||
def test_thumbnail_sizes_available(self):
|
||||
# Documents the fixed size allow-list this test suite exercises against - update both
|
||||
# if files/media_urls.py's THUMBNAIL_SIZES ever changes.
|
||||
# Fixed size allow-list this suite exercises - update both if media_urls.py's THUMBNAIL_SIZES changes.
|
||||
self.assertEqual(THUMBNAIL_SIZES, (32, 64, 256))
|
||||
|
||||
@override_settings(SERVE_X_ACCEL_REDIRECT=False)
|
||||
|
|
@ -274,10 +265,7 @@ class ThumbnailUrlTestCase(FilesTestMixin, UserTestMixin, InventoryTestMixin, To
|
|||
|
||||
@override_settings(SERVE_X_ACCEL_REDIRECT=False)
|
||||
def test_thumbnail_flattens_transparency_instead_of_going_black(self):
|
||||
# Reproduces a real incident: an 'LA' (grayscale + alpha) source whose fully-transparent
|
||||
# region has zeroed-out luminance underneath, as many image tools produce. Converting
|
||||
# straight to RGB (dropping alpha without compositing) reveals that zeroed data - the
|
||||
# whole thumbnail comes out solid black even though the visible (opaque) content isn't.
|
||||
# Regression test for an 'LA' source with zeroed transparent-region luminance. See docs/implementation.md#rgba-flattening-avoids-revealing-black-under-transparent-pixels.
|
||||
half_transparent = Image.new('LA', (200, 200))
|
||||
pixels = half_transparent.load()
|
||||
for x in range(200):
|
||||
|
|
@ -310,8 +298,7 @@ class ThumbnailUrlTestCase(FilesTestMixin, UserTestMixin, InventoryTestMixin, To
|
|||
with default_storage.open(rel_path, 'rb') as f:
|
||||
cached_bytes = f.read()
|
||||
|
||||
# Overwrite the cached file with a marker so a correct implementation must serve this
|
||||
# exact content back rather than regenerating it from the original.
|
||||
# Overwrites the cache with a marker so a correct implementation must serve it back, not regenerate.
|
||||
default_storage.delete(rel_path)
|
||||
default_storage.save(rel_path, ContentFile(cached_bytes + b'MARKER'))
|
||||
|
||||
|
|
@ -333,8 +320,7 @@ class ThumbnailUrlTestCase(FilesTestMixin, UserTestMixin, InventoryTestMixin, To
|
|||
self.assertEqual(reply.status_code, 403)
|
||||
|
||||
def test_thumbnail_not_friend(self):
|
||||
# local_user1/local_user2 are friends in these fixtures (see prepare_inventory) - the
|
||||
# denied case needs a stranger to that friendship instead.
|
||||
# local_user1/local_user2 are friends here (see prepare_inventory), so the denied case needs a stranger instead.
|
||||
reply = client.get(self._thumb_url(64), self.f['ext_user1'])
|
||||
self.assertEqual(reply.status_code, 404)
|
||||
self.assertFalse(default_storage.exists(self._thumb_rel_path(64)))
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue