8000 improve S3 vhost matching, fix S3 routing in moto to allow upload of … · localstack/localstack@080d53e · GitHub
[go: up one dir, main page]
Skip to content

Commit 080d53e

Browse files
whummerbentsku
whummer
and
bentsku
authored
improve S3 vhost matching, fix S3 routing in moto to allow upload of favicon.ico files (#7868)
Co-authored-by: Ben Simon Hartung <42031100+bentsku@users.noreply.github.com>
1 parent 3cb5069 commit 080d53e

File tree

3 files changed

+79
-9
lines changed

3 files changed

+79
-9
lines changed

‎localstack/services/motoserver.py

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
from localstack import constants
77
from localstack.utils.net import get_free_tcp_port
88
from localstack.utils.objects import singleton_factory
9+
from localstack.utils.patch import patch
910
from localstack.utils.serving import Server
1011

1112
LOG = logging.getLogger(__name__)
@@ -41,3 +42,19 @@ def get_moto_server() -> MotoServer:
4142
raise TimeoutError("gave up waiting for moto server on %s" % server.url)
4243

4344
return server
45+
46+
47+
@patch(DomainDispatcherApplication.get_application)
48+
def get_application(fn, self, environ, *args, **kwargs):
49+
"""
50+
Patch to fix an upstream issue where moto treats "/favicon.ico" as a special path, which
51+
can break clients attempting to upload favicon.ico files to S3 buckets.
52+
"""
53+
if environ.get("PATH_INFO") == "/favicon.ico":
54+
environ["PATH_INFO"] = "/"
55+
try:
56+
return fn(self, environ, *args, **kwargs)
57+
finally:
58+
environ["PATH_INFO"] = "/favicon.ico"
59+
60+
return fn(self, environ, *args, **kwargs)

‎localstack/services/s3/virtual_host.py

Lines changed: 21 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22
import logging
33
from urllib.parse import urlsplit, urlunsplit
44

5-
from localstack.config import LEGACY_S3_PROVIDER
5+
from localstack import config
66
from localstack.constants import LOCALHOST_HOSTNAME
77
from localstack.http import Request, Response
88
from localstack.http.proxy import Proxy
@@ -13,13 +13,15 @@
1313

1414
LOG = logging.getLogger(__name__)
1515

16-
# virtual-host style: https://{bucket-name}.s3.{region}.localhost.localstack.cloud.com/{key-name}
17-
VHOST_REGEX_PATTERN = f"<regex('.*'):bucket>.s3.<regex('({AWS_REGION_REGEX}\\.)?'):region>{LOCALHOST_HOSTNAME}<regex('(?::\\d+)?'):port>"
16+
# virtual-host style: https://{bucket-name}.s3.{region?}.{domain}:{port?}/{key-name}
17+
# ex: https://{bucket-name}.s3.{region}.localhost.localstack.cloud.com:4566/{key-name}
18+
# ex: https://{bucket-name}.s3.{region}.amazonaws.com/{key-name}
19+
VHOST_REGEX_PATTERN = f"<regex('.*'):bucket>.s3.<regex('({AWS_REGION_REGEX}\\.)?'):region><regex('.*'):domain><regex('(?::\\d+)?'):port>"
1820

1921
# path addressed request with the region in the hostname
2022
# https://s3.{region}.localhost.localstack.cloud.com/{bucket-name}/{key-name}
2123
PATH_WITH_REGION_PATTERN = (
22-
f"s3.<regex('({AWS_REGION_REGEX}\\.)'):region>{LOCALHOST_HOSTNAME}<regex('(?::\\d+)?'):port>"
24+
f"s3.<regex('({AWS_REGION_REGEX}\\.)'):region><regex('.*'):domain><regex('(?::\\d+)?'):port>"
2325
)
2426

2527

@@ -31,7 +33,7 @@ class S3VirtualHostProxyHandler:
3133

3234
def __call__(self, request: Request, **kwargs) -> Response:
3335
# TODO region pattern currently not working -> removing it from url
34-
rewritten_url = self._rewrite_url(request.url, kwargs.get("bucket"), kwargs.get("region"))
36+
rewritten_url = self._rewrite_url(url=request.url, **kwargs)
3537

3638
LOG.debug(f"Rewritten original host url: {request.url} to path-style url: {rewritten_url}")
3739

@@ -41,7 +43,7 @@ def __call__(self, request: Request, **kwargs) -> Response:
4143
copied_headers[S3_VIRTUAL_HOST_FORWARDED_HEADER] = request.headers["host"]
4244
# do not preserve the Host when forwarding (to avoid an endless loop)
4345
with Proxy(
44-
forward_base_url=f"{forward_to_url.scheme}://{forward_to_url.netloc}",
46+
forward_base_url=config.get_edge_url(),
4547
preserve_host=False,
4648
) as proxy:
4749
forwarded = proxy.forward(
@@ -53,18 +55,21 @@ def __call__(self, request: Request, **kwargs) -> Response:
5355
return forwarded
5456

5557
@staticmethod
56-
def _rewrite_url(url: str, bucket: str, region: str) -> str:
58+
def _rewrite_url(url: str, domain: str, bucket: str, region: str, port: str, **kwargs) -> str:
5759
"""
5860
Rewrites the url so that it can be forwarded to moto. Used for vhost-style and for any url that contains the region.
5961
6062
For vhost style: removes the bucket-name from the host-name and adds it as path
61-
E.g. http://my-bucket.s3.localhost.localstack.cloud:4566 -> http://s3.localhost.localstack.cloud:4566/my-bucket
63+
E.g. https://bucket.s3.localhost.localstack.cloud:4566 -> https://s3.localhost.localstack.cloud:4566/bucket
64+
E.g. https://bucket.s3.amazonaws.com -> https://s3.localhost.localstack.cloud:4566/bucket
6265
6366
If the region is contained in the host-name we remove it (for now) as moto cannot handle the region correctly
6467
6568
:param url: the original url
69+
:param domain: the domain name
6670
:param bucket: the bucket name
6771
:param region: the region name
72+
:param port: the port number (if specified in the original request URL), or an empty string
6873
:return: re-written url as string
6974
"""
7075
splitted = urlsplit(url)
@@ -79,10 +84,17 @@ def _rewrite_url(url: str, bucket: str, region: str) -> str:
7984
if region:
8085
netloc = netloc.replace(f"{region}", "")
8186

87+
# the user can specify whatever domain & port he wants in the Host header
88+
# we need to make sure we're redirecting the request to our edge URL, possibly s3.localhost.localstack.cloud
89+
host = f"{domain}:{port}" if port else domain
90+
edge_host = f"{LOCALHOST_HOSTNAME}:{config.get_edge_port_http()}"
91+
if host != edge_host:
92+
netloc = netloc.replace(host, edge_host)
93+
8294
return urlunsplit((splitted.scheme, netloc, path, splitted.query, splitted.fragment))
8395

8496

85-
@hooks.on_infra_ready(should_load=not LEGACY_S3_PROVIDER)
97+
@hooks.on_infra_ready(should_load=not config.LEGACY_S3_PROVIDER)
8698
def register_virtual_host_routes():
8799
"""
88100
Registers the S3 virtual host handler into the edge router.

‎tests/integration/s3/test_s3.py

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6033,6 +6033,47 @@ def _get_static_hosting_transformers(snapshot):
60336033
]
60346034

60356035

6036+
class TestS3Routing:
6037+
@pytest.mark.only_localstack
6038+
@pytest.mark.parametrize(
6039+
"domain, use_virtual_address",
6040+
[
6041+
("s3.amazonaws.com", False),
6042+
("s3.amazonaws.com", True),
6043+
("s3.us-west-2.amazonaws.com", False),
6044+
("s3.us-west-2.amazonaws.com", True),
6045+
],
6046+
)
6047+
def test_access_favicon_via_aws_endpoints(
6048+
self, s3_bucket, s3_client, domain, use_virtual_address
6049+
):
6050+
"""Assert that /favicon.ico objects can be created/accessed/deleted using amazonaws host headers"""
6051+
6052+
s3_key = "favicon.ico"
6053+
content = b"test 123"
6054+
s3_client.put_object(Bucket=s3_bucket, Key=s3_key, Body=content)
6055+
s3_client.head_object(Bucket=s3_bucket, Key=s3_key)
6056+
6057+
path = s3_key if use_virtual_address else f"{s3_bucket}/{s3_key}"
6058+
url = f"{config.get_edge_url()}/{path}"
6059+
headers = aws_stack.mock_aws_request_headers("s3")
6060+
headers["host"] = f"{s3_bucket}.{domain}" if use_virtual_address else domain
6061+
6062+
# get object via *.amazonaws.com host header
6063+
result = requests.get(url, headers=headers)
6064+
assert result.ok
6065+
assert result.content == content
6066+
6067+
# delete object via *.amazonaws.com host header
6068+
result = requests.delete(url, headers=headers)
6069+
assert result.ok
6070+
6071+
# assert that object has been deleted
6072+
with pytest.raises(ClientError) as exc:
6073+
s3_client.head_object(Bucket=s3_bucket, Key=s3_key)
6074+
assert exc.value.response["Error"]["Message"] == "Not Found"
6075+
6076+
60366077
def _anon_client(service: str):
60376078
conf = Config(signature_version=UNSIGNED)
60386079
if os.environ.get("TEST_TARGET") == "AWS_CLOUD":

0 commit comments

Comments
 (0)
0