Skip to content
This repository was archived by the owner on Mar 23, 2026. It is now read-only.

S3: clean up leftover legacy code #13271

Merged
bentsku merged 1 commit into from
Oct 16, 2025
Merged

S3: clean up leftover legacy code #13271

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 0 additions & 2 deletions localstack-core/localstack/services/s3/constants.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,6 @@
)
from localstack.aws.api.s3 import Type as GranteeType

S3_VIRTUAL_HOST_FORWARDED_HEADER = "x-s3-vhost-forwarded-for"

S3_UPLOAD_PART_MIN_SIZE = 5242880
"""
This is minimum size allowed by S3 when uploading more than one part for a Multipart Upload, except for the last part
Expand Down
2 changes: 1 addition & 1 deletion localstack-core/localstack/services/s3/notifications.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,7 @@ class S3EventNotificationContext:
key_storage_class: StorageClass | None

@classmethod
def from_request_context_native(
def from_request_context(
cls,
request_context: RequestContext,
s3_bucket: S3Bucket,
Expand Down
41 changes: 13 additions & 28 deletions localstack-core/localstack/services/s3/presigned_url.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,10 +45,8 @@
SIGNATURE_V4_PARAMS,
)
from localstack.services.s3.utils import (
S3_VIRTUAL_HOST_FORWARDED_HEADER,
capitalize_header_name_from_snake_case,
extract_bucket_name_and_key_from_headers_and_path,
forwarded_from_virtual_host_addressed_request,
is_bucket_name_valid,
is_presigned_url_request,
uses_host_addressing,
Expand Down Expand Up @@ -567,34 +565,21 @@ def __init__(self, context: RequestContext):
self._query_parameters
)

if forwarded_from_virtual_host_addressed_request(self._headers):
# FIXME: maybe move this so it happens earlier in the chain when using virtual host?
if not is_bucket_name_valid(self._bucket):
raise InvalidBucketName(BucketName=self._bucket)
netloc = self._headers.get(S3_VIRTUAL_HOST_FORWARDED_HEADER)
self.host = netloc
self._original_host = netloc
self.signed_headers["host"] = netloc
# the request comes from the Virtual Host router, we need to remove the bucket from the path
netloc = urlparse.urlparse(self.request.url).netloc
self.host = netloc
self._original_host = netloc
if (host_addressed := uses_host_addressing(self._headers)) and not is_bucket_name_valid(
self._bucket
):
raise InvalidBucketName(BucketName=self._bucket)

if not host_addressed and not self.request.path.startswith(f"/{self._bucket}"):
# if in path style, check that the path starts with the bucket
# our path has been sanitized, we should use the un-sanitized one
splitted_path = self.request.path.split("/", maxsplit=2)
self.path = f"/{splitted_path[-1]}"

self.path = f"/{self._bucket}/{splitted_path[-1]}"
else:
netloc = urlparse.urlparse(self.request.url).netloc
self.host = netloc
self._original_host = netloc
if (host_addressed := uses_host_addressing(self._headers)) and not is_bucket_name_valid(
self._bucket
):
raise InvalidBucketName(BucketName=self._bucket)

if not host_addressed and not self.request.path.startswith(f"/{self._bucket}"):
# if in path style, check that the path starts with the bucket
# our path has been sanitized, we should use the un-sanitized one
splitted_path = self.request.path.split("/", maxsplit=2)
self.path = f"/{self._bucket}/{splitted_path[-1]}"
else:
self.path = self.request.path
self.path = self.request.path

# we need to URL encode the path, as the key needs to be urlencoded for the signature to match
self.path = urlparse.quote(self.path)
Expand Down
8 changes: 4 additions & 4 deletions localstack-core/localstack/services/s3/provider.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -384,7 +384,7 @@ def _notify(
"""
if s3_bucket.notification_configuration:
if not s3_notif_ctx:
s3_notif_ctx = S3EventNotificationContext.from_request_context_native(
s3_notif_ctx = S3EventNotificationContext.from_request_context(
context,
s3_bucket=s3_bucket,
s3_object=s3_object,
Expand Down Expand Up @@ -1271,7 +1271,7 @@ def delete_object(
delete_marker_id = generate_version_id(s3_bucket.versioning_status)
delete_marker = S3DeleteMarker(key=key, version_id=delete_marker_id)
s3_bucket.objects.set(key, delete_marker)
s3_notif_ctx = S3EventNotificationContext.from_request_context_native(
s3_notif_ctx = S3EventNotificationContext.from_request_context(
context,
s3_bucket=s3_bucket,
s3_object=delete_marker,
Expand Down Expand Up @@ -1374,7 +1374,7 @@ def delete_objects(
delete_marker_id = generate_version_id(s3_bucket.versioning_status)
delete_marker = S3DeleteMarker(key=object_key, version_id=delete_marker_id)
s3_bucket.objects.set(object_key, delete_marker)
s3_notif_ctx = S3EventNotificationContext.from_request_context_native(
s3_notif_ctx = S3EventNotificationContext.from_request_context(
context,
s3_bucket=s3_bucket,
s3_object=delete_marker,
Expand Down Expand Up @@ -2202,7 +2202,7 @@ def restore_object(
# TODO: add a way to transition from ongoing-request=true to false? for now it is instant
s3_object.restore = f'ongoing-request="false", expiry-date="{restore_expiration_date}"'

s3_notif_ctx_initiated = S3EventNotificationContext.from_request_context_native(
s3_notif_ctx_initiated = S3EventNotificationContext.from_request_context(
context,
s3_bucket=s3_bucket,
s3_object=s3_object,
Expand Down
13 changes: 2 additions & 11 deletions localstack-core/localstack/services/s3/utils.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
import re
import time
import zlib
from collections.abc import Mapping
from enum import StrEnum
from secrets import token_bytes
from typing import Any, Literal, NamedTuple, Protocol
Expand Down Expand Up @@ -63,7 +64,6 @@
AUTHENTICATED_USERS_ACL_GRANTEE,
CHECKSUM_ALGORITHMS,
LOG_DELIVERY_ACL_GRANTEE,
S3_VIRTUAL_HOST_FORWARDED_HEADER,
SIGNATURE_V2_PARAMS,
SIGNATURE_V4_PARAMS,
SYSTEM_METADATA_SETTABLE_HEADERS,
Expand Down Expand Up @@ -522,7 +522,7 @@ def is_valid_canonical_id(canonical_id: str) -> bool:
return False


def uses_host_addressing(headers: dict[str, str]) -> str | None:
def uses_host_addressing(headers: Mapping[str, str]) -> str | None:
"""
Determines if the request is targeting S3 with virtual host addressing
:param headers: the request headers
Expand Down Expand Up @@ -551,15 +551,6 @@ def get_system_metadata_from_request(request: dict) -> Metadata:
return metadata


def forwarded_from_virtual_host_addressed_request(headers: dict[str, str]) -> bool:
"""
Determines if the request was forwarded from a v-host addressing style into a path one
"""
# we can assume that the host header we are receiving here is actually the header we originally received
# from the client (because the edge service is forwarding the request in memory)
return S3_VIRTUAL_HOST_FORWARDED_HEADER in headers


def extract_bucket_name_and_key_from_headers_and_path(
headers: dict[str, str], path: str
) -> tuple[str | None, str | None]:
Expand Down
Loading