fix: FIT-720: Fix binary octet processing in image caching (#9378)

makseq · web-flow · commit 6daf144393ea · 2026-02-11T22:12:22.000+03:00
diff --git a/.pre-commit-dev.yaml b/.pre-commit-dev.yaml
@@ -14,7 +14,7 @@ repos:
     rev: "v0.1.0"
     hooks:
       - id: biome-check
-        args: [ --config-path, ./web ]
+        args: [ --config-path, ./web, --diagnostic-level=error ]
         additional_dependencies: [ "@biomejs/biome@1.9.4" ]
         files: ^web/.*
   - repo: local
diff --git a/label_studio/data_manager/actions/experimental.py b/label_studio/data_manager/actions/experimental.py
@@ -165,6 +165,10 @@ def add_data_field(project, queryset, **kwargs):
     value = request.data.get('value')
     size = queryset.count()
 
+    # Save a task ID before the update, because queryset filters may no longer
+    # match after we modify the data field (e.g. changing the filtered column value).
+    first_task_id = queryset.values_list('id', flat=True).first()
+
     cast = {'String': str, 'Number': float, 'Expression': str}
     assert value_type in cast.keys()
     value = cast[value_type](value)
@@ -192,7 +196,13 @@ def add_data_field(project, queryset, **kwargs):
                 )
             )
 
-    project.summary.update_data_columns([queryset.first()])
+    # Fetch the task by saved ID since the original queryset filters
+    # may no longer match after the data field was modified.
+    if first_task_id is not None:
+        first_task = Task.objects.filter(id=first_task_id).first()
+        if first_task is not None:
+            project.summary.update_data_columns([first_task])
+
     return {'response_code': 200, 'detail': f'Updated {size} tasks'}
 
 
diff --git a/label_studio/io_storages/proxy_api.py b/label_studio/io_storages/proxy_api.py
@@ -1,5 +1,6 @@
 import base64
 import logging
+import mimetypes
 import time
 from typing import Union
 from urllib.parse import unquote
@@ -246,6 +247,13 @@ def proxy_data_from_storage(self, request, uri, project, storage):
                     status=status.HTTP_424_FAILED_DEPENDENCY,
                 )
 
+            # Detect content type from URI file extension as a safety net for all storage types
+            # when the storage returns a generic type like binary/octet-stream or application/octet-stream
+            if not content_type or 'octet-stream' in content_type:
+                guessed_type, _ = mimetypes.guess_type(uri)
+                if guessed_type:
+                    content_type = guessed_type
+
             # Create time-limited stream
             time_limited_stream = self.time_limited_chunker(stream)
 
diff --git a/label_studio/io_storages/s3/tests/__init__.py b/label_studio/io_storages/s3/tests/__init__.py
diff --git a/label_studio/io_storages/s3/tests/test_resolve_s3_url.py b/label_studio/io_storages/s3/tests/test_resolve_s3_url.py
@@ -0,0 +1,110 @@
+"""Tests for resolve_s3_url content type handling.
+
+Verifies that presigned URLs include ResponseContentType based on file extension,
+so S3 returns the correct MIME type even if objects were uploaded without one.
+"""
+import unittest
+from unittest.mock import MagicMock
+
+from io_storages.s3.utils import resolve_s3_url
+
+
+class TestResolveS3UrlContentType(unittest.TestCase):
+    """Test that resolve_s3_url adds ResponseContentType to presigned URL params."""
+
+    def setUp(self):
+        self.mock_client = MagicMock()
+        self.mock_client.generate_presigned_url.return_value = 'https://bucket.s3.amazonaws.com/presigned'
+
+    def test_jpeg_gets_response_content_type(self):
+        """Presigned URL for .jpg should include ResponseContentType=image/jpeg"""
+        resolve_s3_url('s3://bucket/path/image.jpg', self.mock_client, presign=True)
+
+        call_args = self.mock_client.generate_presigned_url.call_args
+        params = (
+            call_args[1]['Params']
+            if 'Params' in call_args[1]
+            else call_args[0][1]
+            if len(call_args[0]) > 1
+            else call_args[1].get('Params')
+        )
+        assert params['ResponseContentType'] == 'image/jpeg'
+
+    def test_png_gets_response_content_type(self):
+        """Presigned URL for .png should include ResponseContentType=image/png"""
+        resolve_s3_url('s3://bucket/path/image.png', self.mock_client, presign=True)
+
+        call_args = self.mock_client.generate_presigned_url.call_args
+        params = call_args[1]['Params']
+        assert params['ResponseContentType'] == 'image/png'
+
+    def test_mp4_gets_response_content_type(self):
+        """Presigned URL for .mp4 should include ResponseContentType=video/mp4"""
+        resolve_s3_url('s3://bucket/videos/clip.mp4', self.mock_client, presign=True)
+
+        call_args = self.mock_client.generate_presigned_url.call_args
+        params = call_args[1]['Params']
+        assert params['ResponseContentType'] == 'video/mp4'
+
+    def test_unknown_extension_no_response_content_type(self):
+        """Presigned URL for unknown extension should NOT include ResponseContentType"""
+        resolve_s3_url('s3://bucket/data/file.xyz123', self.mock_client, presign=True)
+
+        call_args = self.mock_client.generate_presigned_url.call_args
+        params = call_args[1]['Params']
+        assert 'ResponseContentType' not in params
+
+    def test_no_extension_no_response_content_type(self):
+        """Presigned URL for file without extension should NOT include ResponseContentType"""
+        resolve_s3_url('s3://bucket/data/README', self.mock_client, presign=True)
+
+        call_args = self.mock_client.generate_presigned_url.call_args
+        params = call_args[1]['Params']
+        assert 'ResponseContentType' not in params
+
+    def test_json_gets_response_content_type(self):
+        """Presigned URL for .json should include ResponseContentType=application/json"""
+        resolve_s3_url('s3://bucket/data/tasks.json', self.mock_client, presign=True)
+
+        call_args = self.mock_client.generate_presigned_url.call_args
+        params = call_args[1]['Params']
+        assert params['ResponseContentType'] == 'application/json'
+
+    def test_tiff_gets_response_content_type(self):
+        """Presigned URL for .tiff should include ResponseContentType=image/tiff"""
+        resolve_s3_url('s3://bucket/scans/scan.tiff', self.mock_client, presign=True)
+
+        call_args = self.mock_client.generate_presigned_url.call_args
+        params = call_args[1]['Params']
+        assert params['ResponseContentType'] == 'image/tiff'
+
+    def test_presign_false_does_not_use_response_content_type(self):
+        """When presign=False, blob is returned directly - no presigned URL generated"""
+        mock_body = MagicMock()
+        mock_body.read.return_value = b'fake image data'
+        self.mock_client.get_object.return_value = {
+            'ResponseMetadata': {'HTTPHeaders': {'content-type': 'binary/octet-stream'}},
+            'Body': mock_body,
+        }
+
+        result = resolve_s3_url('s3://bucket/image.jpg', self.mock_client, presign=False)
+
+        # Should return base64 data, not call generate_presigned_url
+        self.mock_client.generate_presigned_url.assert_not_called()
+        assert result.startswith('data:binary/octet-stream;base64,')
+
+    def test_bucket_and_key_always_present(self):
+        """Bucket and Key should always be in params regardless of content type detection"""
+        resolve_s3_url('s3://my-bucket/path/to/file.jpg', self.mock_client, presign=True)
+
+        call_args = self.mock_client.generate_presigned_url.call_args
+        params = call_args[1]['Params']
+        assert params['Bucket'] == 'my-bucket'
+        assert params['Key'] == 'path/to/file.jpg'
+
+    def test_expires_in_passed_through(self):
+        """ExpiresIn parameter should be passed through to generate_presigned_url"""
+        resolve_s3_url('s3://bucket/file.jpg', self.mock_client, presign=True, expires_in=120)
+
+        call_args = self.mock_client.generate_presigned_url.call_args
+        assert call_args[1]['ExpiresIn'] == 120
diff --git a/label_studio/io_storages/s3/utils.py b/label_studio/io_storages/s3/utils.py
@@ -3,6 +3,7 @@
 import base64
 import fnmatch
 import logging
+import mimetypes
 import re
 from urllib.parse import urlparse
 
@@ -55,9 +56,17 @@ def resolve_s3_url(url, client, presign=True, expires_in=3600):
 
     # Otherwise try to generate presigned url
     try:
-        presigned_url = client.generate_presigned_url(
-            ClientMethod='get_object', Params={'Bucket': bucket_name, 'Key': key}, ExpiresIn=expires_in
-        )
+        params = {'Bucket': bucket_name, 'Key': key}
+
+        # Override response Content-Type based on file extension so that S3 returns
+        # the correct MIME type even if the object was uploaded without one.
+        # Without this, S3 may return binary/octet-stream which causes browsers
+        # to download files instead of displaying them inline (e.g. images, audio, video).
+        content_type, _ = mimetypes.guess_type(key)
+        if content_type:
+            params['ResponseContentType'] = content_type
+
+        presigned_url = client.generate_presigned_url(ClientMethod='get_object', Params=params, ExpiresIn=expires_in)
     except ClientError as exc:
         logger.warning(f"Can't generate presigned URL. Reason: {exc}")
         return url
diff --git a/label_studio/io_storages/tests/test_proxy_api.py b/label_studio/io_storages/tests/test_proxy_api.py
@@ -112,6 +112,117 @@ def test_redirect_to_presign_url_exception(self):
         result = self.mixin.redirect_to_presign_url('fileuri', self.task, 'Task')
         assert result.status_code == status.HTTP_404_NOT_FOUND
 
+    def test_proxy_data_from_storage_content_type_fallback_for_octet_stream(self):
+        """Test that proxy detects correct content type from URI when storage returns octet-stream.
+
+        S3 objects uploaded without explicit Content-Type often have binary/octet-stream.
+        The proxy should detect the correct type from the URI file extension.
+        """
+        mock_storage = MagicMock()
+        mock_stream = MagicMock()
+        mock_metadata = {
+            'StatusCode': 200,
+            'ContentLength': 1000,
+            'LastModified': datetime.now(),
+            'ETag': '"abcdef123456"',
+        }
+        # Storage returns binary/octet-stream (common S3 default for missing Content-Type)
+        mock_storage.get_bytes_stream.return_value = (mock_stream, 'binary/octet-stream', mock_metadata)
+        mock_project = MagicMock()
+
+        with patch('io_storages.proxy_api.StreamingHttpResponse') as mock_response_class, patch(
+            'io_storages.proxy_api.settings'
+        ) as mock_settings:
+            mock_settings.RESOLVER_PROXY_MAX_RANGE_SIZE = 1024 * 1024
+            mock_settings.RESOLVER_PROXY_BUFFER_SIZE = 8192
+            mock_settings.RESOLVER_PROXY_CACHE_TIMEOUT = 3600
+            mock_settings.RESOLVER_PROXY_TIMEOUT = 20
+            mock_settings.RESOLVER_PROXY_ENABLE_ETAG_CACHE = False
+
+            mock_response = MagicMock()
+            mock_response.headers = {}
+            mock_response_class.return_value = mock_response
+            self.request.headers = {}
+
+            # URI with .jpg extension - should be detected as image/jpeg
+            self.mixin.proxy_data_from_storage(self.request, 's3://bucket/photo.jpg', mock_project, mock_storage)
+
+            # Verify StreamingHttpResponse was called with image/jpeg, not binary/octet-stream
+            call_args, call_kwargs = mock_response_class.call_args
+            assert call_kwargs.get('content_type') == 'image/jpeg' or (
+                len(call_args) > 1 and call_args[1] == 'image/jpeg'
+            ), f'Expected content_type=image/jpeg, got: args={call_args}, kwargs={call_kwargs}'
+
+    def test_proxy_data_from_storage_content_type_fallback_for_application_octet_stream(self):
+        """Test fallback for application/octet-stream (another generic type)."""
+        mock_storage = MagicMock()
+        mock_stream = MagicMock()
+        mock_metadata = {
+            'StatusCode': 200,
+            'ContentLength': 5000,
+            'LastModified': datetime.now(),
+            'ETag': '"xyz789"',
+        }
+        mock_storage.get_bytes_stream.return_value = (mock_stream, 'application/octet-stream', mock_metadata)
+        mock_project = MagicMock()
+
+        with patch('io_storages.proxy_api.StreamingHttpResponse') as mock_response_class, patch(
+            'io_storages.proxy_api.settings'
+        ) as mock_settings:
+            mock_settings.RESOLVER_PROXY_MAX_RANGE_SIZE = 1024 * 1024
+            mock_settings.RESOLVER_PROXY_BUFFER_SIZE = 8192
+            mock_settings.RESOLVER_PROXY_CACHE_TIMEOUT = 3600
+            mock_settings.RESOLVER_PROXY_TIMEOUT = 20
+            mock_settings.RESOLVER_PROXY_ENABLE_ETAG_CACHE = False
+
+            mock_response = MagicMock()
+            mock_response.headers = {}
+            mock_response_class.return_value = mock_response
+            self.request.headers = {}
+
+            self.mixin.proxy_data_from_storage(self.request, 's3://bucket/video.mp4', mock_project, mock_storage)
+
+            call_args, call_kwargs = mock_response_class.call_args
+            assert call_kwargs.get('content_type') == 'video/mp4' or (
+                len(call_args) > 1 and call_args[1] == 'video/mp4'
+            ), f'Expected content_type=video/mp4, got: args={call_args}, kwargs={call_kwargs}'
+
+    def test_proxy_data_from_storage_preserves_correct_content_type(self):
+        """When storage returns a proper content type, it should not be overridden."""
+        mock_storage = MagicMock()
+        mock_stream = MagicMock()
+        mock_metadata = {
+            'StatusCode': 200,
+            'ContentLength': 1000,
+            'LastModified': datetime.now(),
+            'ETag': '"test"',
+        }
+        # Storage returns correct content type
+        mock_storage.get_bytes_stream.return_value = (mock_stream, 'image/webp', mock_metadata)
+        mock_project = MagicMock()
+
+        with patch('io_storages.proxy_api.StreamingHttpResponse') as mock_response_class, patch(
+            'io_storages.proxy_api.settings'
+        ) as mock_settings:
+            mock_settings.RESOLVER_PROXY_MAX_RANGE_SIZE = 1024 * 1024
+            mock_settings.RESOLVER_PROXY_BUFFER_SIZE = 8192
+            mock_settings.RESOLVER_PROXY_CACHE_TIMEOUT = 3600
+            mock_settings.RESOLVER_PROXY_TIMEOUT = 20
+            mock_settings.RESOLVER_PROXY_ENABLE_ETAG_CACHE = False
+
+            mock_response = MagicMock()
+            mock_response.headers = {}
+            mock_response_class.return_value = mock_response
+            self.request.headers = {}
+
+            self.mixin.proxy_data_from_storage(self.request, 's3://bucket/photo.jpg', mock_project, mock_storage)
+
+            # Should preserve the original image/webp, not override with image/jpeg
+            call_args, call_kwargs = mock_response_class.call_args
+            assert call_kwargs.get('content_type') == 'image/webp' or (
+                len(call_args) > 1 and call_args[1] == 'image/webp'
+            ), f'Expected content_type=image/webp, got: args={call_args}, kwargs={call_kwargs}'
+
     def test_proxy_data_from_storage_success(self):
         mock_storage = MagicMock()
         # Ensure get_bytes_stream returns a three-tuple, metadata can be empty initially
diff --git a/web/libs/core/src/lib/utils/ImageCache.test.ts b/web/libs/core/src/lib/utils/ImageCache.test.ts
diff --git a/web/libs/core/src/lib/utils/ImageCache.ts b/web/libs/core/src/lib/utils/ImageCache.ts
