Merge 7d42a15965 into 72a461ba4a

litellm/integrations/s3_v2.py

@@ -10,6 +10,7 @@ import asyncio
 import time
 from datetime import datetime
 from typing import List, Optional, cast
+from urllib.parse import quote
 
 import litellm
 from litellm._logging import print_verbose, verbose_logger
@@ -347,8 +348,16 @@ class S3Logger(CustomBatchLogger, BaseAWSLLM):
             )
             verbose_logger.debug(f"s3_v2 logger - s3_verify setting: {self.s3_verify}")
 
+            # URL-encode the object key so that characters like '=' (from
+            # base64 padding) are percent-encoded.  This ensures the URL that
+            # httpx sends on the wire matches the canonical path that
+            # SigV4Auth uses when computing the request signature.
+            encoded_key = quote(
+                batch_logging_element.s3_object_key, safe="/"
+            )
+
             # Prepare the URL
-            url = f"https://{self.s3_bucket_name}.s3.{self.s3_region_name}.amazonaws.com/{batch_logging_element.s3_object_key}"
+            url = f"https://{self.s3_bucket_name}.s3.{self.s3_region_name}.amazonaws.com/{encoded_key}"
 
             if self.s3_endpoint_url and self.s3_bucket_name:
                 if self.s3_use_virtual_hosted_style:
@@ -361,7 +370,7 @@ class S3Logger(CustomBatchLogger, BaseAWSLLM):
                         if self.s3_endpoint_url.startswith("https://")
                         else "http://"
                     )
-                    url = f"{protocol}{self.s3_bucket_name}.{endpoint_host}/{batch_logging_element.s3_object_key}"
+                    url = f"{protocol}{self.s3_bucket_name}.{endpoint_host}/{encoded_key}"
                 else:
                     # Path-style: endpoint/bucket/key
                     url = (
@@ -369,7 +378,7 @@ class S3Logger(CustomBatchLogger, BaseAWSLLM):
                         + "/"
                         + self.s3_bucket_name
                         + "/"
-                        + batch_logging_element.s3_object_key
+                        + encoded_key
                     )
 
             # Convert JSON to string
@@ -536,8 +545,12 @@ class S3Logger(CustomBatchLogger, BaseAWSLLM):
                 aws_region_name=self.s3_region_name,
             )
 
+            encoded_key = quote(
+                batch_logging_element.s3_object_key, safe="/"
+            )
+
             # Prepare the URL
-            url = f"https://{self.s3_bucket_name}.s3.{self.s3_region_name}.amazonaws.com/{batch_logging_element.s3_object_key}"
+            url = f"https://{self.s3_bucket_name}.s3.{self.s3_region_name}.amazonaws.com/{encoded_key}"
 
             if self.s3_endpoint_url and self.s3_bucket_name:
                 if self.s3_use_virtual_hosted_style:
@@ -550,7 +563,7 @@ class S3Logger(CustomBatchLogger, BaseAWSLLM):
                         if self.s3_endpoint_url.startswith("https://")
                         else "http://"
                     )
-                    url = f"{protocol}{self.s3_bucket_name}.{endpoint_host}/{batch_logging_element.s3_object_key}"
+                    url = f"{protocol}{self.s3_bucket_name}.{endpoint_host}/{encoded_key}"
                 else:
                     # Path-style: endpoint/bucket/key
                     url = (
@@ -558,7 +571,7 @@ class S3Logger(CustomBatchLogger, BaseAWSLLM):
                         + "/"
                         + self.s3_bucket_name
                         + "/"
-                        + batch_logging_element.s3_object_key
+                        + encoded_key
                     )
 
             # Convert JSON to string
@@ -662,8 +675,10 @@ class S3Logger(CustomBatchLogger, BaseAWSLLM):
                 f"s3_v2 logger - downloading data from s3 - {s3_object_key}"
             )
 
+            encoded_key = quote(s3_object_key, safe="/")
+
             # Prepare the URL
-            url = f"https://{self.s3_bucket_name}.s3.{self.s3_region_name}.amazonaws.com/{s3_object_key}"
+            url = f"https://{self.s3_bucket_name}.s3.{self.s3_region_name}.amazonaws.com/{encoded_key}"
 
             if self.s3_endpoint_url and self.s3_bucket_name:
                 if self.s3_use_virtual_hosted_style:
@@ -676,7 +691,7 @@ class S3Logger(CustomBatchLogger, BaseAWSLLM):
                         if self.s3_endpoint_url.startswith("https://")
                         else "http://"
                     )
-                    url = f"{protocol}{self.s3_bucket_name}.{endpoint_host}/{s3_object_key}"
+                    url = f"{protocol}{self.s3_bucket_name}.{endpoint_host}/{encoded_key}"
                 else:
                     # Path-style: endpoint/bucket/key
                     url = (
@@ -684,7 +699,7 @@ class S3Logger(CustomBatchLogger, BaseAWSLLM):
                         + "/"
                         + self.s3_bucket_name
                         + "/"
-                        + s3_object_key
+                        + encoded_key
                     )
 
             # Prepare the request for GET operation

tests/test_litellm/integrations/test_s3_v2.py

@@ -25,6 +25,120 @@ class TestS3V2UnitTests:
             "json.dumps(" not in source_code
         ), "S3 v2 should not use json.dumps directly"
 
+    @patch('asyncio.create_task')
+    @patch('litellm.integrations.s3_v2.CustomBatchLogger.periodic_flush')
+    def test_s3_v2_url_encodes_object_key_with_special_chars(self, mock_periodic_flush, mock_create_task):
+        """Test that S3 object keys with special characters (e.g. base64 padding '=')
+        are URL-encoded in the request URL.
+
+        Without URL-encoding, SigV4Auth computes the signature over the URL-encoded
+        canonical path (%3D) while httpx sends the literal '=' on the wire. S3-compatible
+        servers (Garage, MinIO) then reject the request with 403 Invalid signature.
+        """
+        from unittest.mock import AsyncMock, MagicMock
+
+        from litellm.types.integrations.s3_v2 import s3BatchLoggingElement
+
+        mock_periodic_flush.return_value = None
+        mock_create_task.return_value = None
+
+        mock_response = MagicMock()
+        mock_response.status_code = 200
+        mock_response.raise_for_status = MagicMock()
+
+        # Object key with base64 padding (== at the end), as produced by
+        # LiteLLM's Responses API composite response IDs
+        test_element = s3BatchLoggingElement(
+            s3_object_key="2025-09-14/time-18-07-17_resp_bGl0ZWxsbTpPUT==.json",
+            payload={"test": "data"},
+            s3_object_download_filename="test-file.json"
+        )
+
+        # Test: path-style with custom endpoint
+        s3_logger = S3Logger(
+            s3_bucket_name="test-bucket",
+            s3_endpoint_url="https://garage.example.com",
+            s3_aws_access_key_id="test-key",
+            s3_aws_secret_access_key="test-secret",
+            s3_region_name="us-east-1"
+        )
+
+        s3_logger.async_httpx_client = AsyncMock()
+        s3_logger.async_httpx_client.put.return_value = mock_response
+
+        asyncio.run(s3_logger.async_upload_data_to_s3(test_element))
+
+        call_args = s3_logger.async_httpx_client.put.call_args
+        assert call_args is not None
+        url = call_args[0][0]
+        # The '=' characters must be percent-encoded as %3D
+        assert "%3D" in url, f"Expected URL-encoded '=' (%3D) in URL, got {url}"
+        assert "==" not in url, f"Literal '==' should not appear in URL path, got {url}"
+
+        # Test: virtual-hosted-style
+        s3_logger_virtual = S3Logger(
+            s3_bucket_name="test-bucket",
+            s3_endpoint_url="https://garage.example.com",
+            s3_aws_access_key_id="test-key",
+            s3_aws_secret_access_key="test-secret",
+            s3_region_name="us-east-1",
+            s3_use_virtual_hosted_style=True,
+        )
+
+        s3_logger_virtual.async_httpx_client = AsyncMock()
+        s3_logger_virtual.async_httpx_client.put.return_value = mock_response
+
+        asyncio.run(s3_logger_virtual.async_upload_data_to_s3(test_element))
+
+        call_args_virtual = s3_logger_virtual.async_httpx_client.put.call_args
+        url_virtual = call_args_virtual[0][0]
+        assert "%3D" in url_virtual, f"Expected URL-encoded '=' in virtual-hosted URL, got {url_virtual}"
+        assert "==" not in url_virtual, f"Literal '==' should not appear in virtual-hosted URL, got {url_virtual}"
+
+        # Test: sync upload method
+        s3_logger_sync = S3Logger(
+            s3_bucket_name="test-bucket",
+            s3_endpoint_url="https://garage.example.com",
+            s3_aws_access_key_id="test-key",
+            s3_aws_secret_access_key="test-secret",
+            s3_region_name="us-east-1"
+        )
+
+        mock_sync_client = MagicMock()
+        mock_sync_client.put.return_value = mock_response
+
+        with patch('litellm.integrations.s3_v2._get_httpx_client', return_value=mock_sync_client):
+            s3_logger_sync.upload_data_to_s3(test_element)
+
+            call_args_sync = mock_sync_client.put.call_args
+            url_sync = call_args_sync[0][0]
+            assert "%3D" in url_sync, f"Expected URL-encoded '=' in sync URL, got {url_sync}"
+            assert "==" not in url_sync, f"Literal '==' should not appear in sync URL, got {url_sync}"
+
+        # Test: download method
+        s3_logger_download = S3Logger(
+            s3_bucket_name="test-bucket",
+            s3_endpoint_url="https://garage.example.com",
+            s3_aws_access_key_id="test-key",
+            s3_aws_secret_access_key="test-secret",
+            s3_region_name="us-east-1"
+        )
+
+        mock_download_response = MagicMock()
+        mock_download_response.status_code = 200
+        mock_download_response.json = MagicMock(return_value={"downloaded": "data"})
+        s3_logger_download.async_httpx_client = AsyncMock()
+        s3_logger_download.async_httpx_client.get.return_value = mock_download_response
+
+        asyncio.run(s3_logger_download._download_object_from_s3(
+            "2025-09-14/time-18-07-17_resp_bGl0ZWxsbTpPUT==.json"
+        ))
+
+        call_args_dl = s3_logger_download.async_httpx_client.get.call_args
+        url_dl = call_args_dl[0][0]
+        assert "%3D" in url_dl, f"Expected URL-encoded '=' in download URL, got {url_dl}"
+        assert "==" not in url_dl, f"Literal '==' should not appear in download URL, got {url_dl}"
+
     @patch('asyncio.create_task')
     @patch('litellm.integrations.s3_v2.CustomBatchLogger.periodic_flush')
     def test_s3_v2_endpoint_url(self, mock_periodic_flush, mock_create_task):