From 41e054c7983222d4df201b3a23f6ede89bee64ee Mon Sep 17 00:00:00 2001 From: yryzhan Date: Wed, 20 May 2026 18:06:50 +0200 Subject: [PATCH] fix(s3_v2): fix test patch target and prevent double-encoding - Fix test: use CustomBatchLogger.periodic_flush (not S3Logger._periodic_flush) - Prevent double-encoding of pre-encoded keys by applying unquote() before quote(), making encoding idempotent (Greptile P2 feedback) --- litellm/integrations/s3_v2.py | 8 ++++---- tests/test_litellm/integrations/test_s3_v2.py | 2 +- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/litellm/integrations/s3_v2.py b/litellm/integrations/s3_v2.py index 79060bedf19..64fc10ec8f6 100644 --- a/litellm/integrations/s3_v2.py +++ b/litellm/integrations/s3_v2.py @@ -10,7 +10,7 @@ import asyncio import time from datetime import datetime from typing import List, Optional, cast -from urllib.parse import quote +from urllib.parse import quote, unquote import litellm from litellm._logging import print_verbose, verbose_logger @@ -350,7 +350,7 @@ class S3Logger(CustomBatchLogger, BaseAWSLLM): verbose_logger.debug(f"s3_v2 logger - s3_verify setting: {self.s3_verify}") # Prepare the URL with percent-encoded object key - encoded_key = quote(batch_logging_element.s3_object_key, safe="/") + encoded_key = quote(unquote(batch_logging_element.s3_object_key), safe="/") 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: @@ -536,7 +536,7 @@ class S3Logger(CustomBatchLogger, BaseAWSLLM): ) # Prepare the URL with percent-encoded object key - encoded_key = quote(batch_logging_element.s3_object_key, safe="/") + encoded_key = quote(unquote(batch_logging_element.s3_object_key), safe="/") 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: @@ -661,7 +661,7 @@ class S3Logger(CustomBatchLogger, BaseAWSLLM): ) # Prepare the URL with percent-encoded object key - encoded_key = quote(s3_object_key, safe="/") + encoded_key = quote(unquote(s3_object_key), safe="/") 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: diff --git a/tests/test_litellm/integrations/test_s3_v2.py b/tests/test_litellm/integrations/test_s3_v2.py index 65505de28ac..2cfa0b0612b 100644 --- a/tests/test_litellm/integrations/test_s3_v2.py +++ b/tests/test_litellm/integrations/test_s3_v2.py @@ -1198,7 +1198,7 @@ def test_s3_callback_params_override_empty_dict_is_opt_in(): @pytest.mark.asyncio @patch("asyncio.create_task") -@patch.object(S3Logger, "_periodic_flush") +@patch("litellm.integrations.s3_v2.CustomBatchLogger.periodic_flush") async def test_s3_v2_put_url_encodes_special_chars( mock_periodic_flush, mock_create_task ):