From f975e1c97ba01952771ac3ac8f93ac7f1b2efa01 Mon Sep 17 00:00:00 2001 From: davidlm Date: Fri, 12 May 2023 16:31:12 -0400 Subject: [PATCH 1/3] Address virtual style addressing bugs for short subdomains provided in ``endpoint_url`` --- botocore/endpoint_provider.py | 10 +++------ tests/functional/test_s3.py | 32 ++++++++++++++++++++++++++++ tests/unit/test_endpoint_provider.py | 20 +++++++++++++++++ 3 files changed, 55 insertions(+), 7 deletions(-) diff --git a/botocore/endpoint_provider.py b/botocore/endpoint_provider.py index 62bf087ccd..1be5a25c8d 100644 --- a/botocore/endpoint_provider.py +++ b/botocore/endpoint_provider.py @@ -398,13 +398,9 @@ def aws_is_virtual_hostable_s3_bucket(self, value, allow_subdomains): ): return False - if allow_subdomains is True: - return all( - self.aws_is_virtual_hostable_s3_bucket(label, False) - for label in value.split(".") - ) - - return self.is_valid_host_label(value, allow_subdomains=False) + return self.is_valid_host_label( + value, allow_subdomains=allow_subdomains + ) # maintains backwards compatibility as `Library` was misspelled diff --git a/tests/functional/test_s3.py b/tests/functional/test_s3.py index f9a13b20b0..e82a6ded80 100644 --- a/tests/functional/test_s3.py +++ b/tests/functional/test_s3.py @@ -3487,6 +3487,38 @@ def _addressing_for_presigned_url_test_cases(): signature_version="s3", expected_url="https://s3.us-west-1.amazonaws.com/foo.bar.biz/key", ) + # Bucket names that contain dots and subcomponents that are less than + # 3 characters should still use virtual host style addressing if + # configured by the customer and they provide their own ``endpoint_url`` + # that is insecure. https://github.com/boto/botocore/issues/2938 + yield dict( + bucket="foo.b.biz", + key="key", + s3_config={"addressing_style": "virtual"}, + customer_provided_endpoint="http://s3.us-west-2.amazonaws.com", + expected_url="http://foo.b.biz.s3.us-west-2.amazonaws.com/key", + ) + yield dict( + bucket="foo.b.biz", + key="key", + s3_config={"addressing_style": "virtual"}, + is_secure=False, + expected_url="http://s3.amazonaws.com/foo.b.biz/key", + ) + yield dict( + bucket="foo.b.biz", + key="key", + s3_config={"addressing_style": "virtual"}, + customer_provided_endpoint="https://s3.us-west-2.amazonaws.com", + expected_url="https://s3.us-west-2.amazonaws.com/foo.b.biz/key", + ) + yield dict( + region="us-west-1", + bucket="foo.b.biz", + key="key", + s3_config={"addressing_style": "virtual"}, + expected_url="https://s3.us-west-1.amazonaws.com/foo.b.biz/key", + ) @pytest.mark.parametrize( diff --git a/tests/unit/test_endpoint_provider.py b/tests/unit/test_endpoint_provider.py index 5f892a63c7..51a07079bc 100644 --- a/tests/unit/test_endpoint_provider.py +++ b/tests/unit/test_endpoint_provider.py @@ -491,3 +491,23 @@ def test_endpoint_reevaluates_result(endpoint_provider, monkeypatch): for region in regions: endpoint_provider.resolve_endpoint(Region=region) assert mock_evaluate.call_count == 2 + + +@pytest.mark.parametrize( + "bucket, expected_value", + [ + ("mybucket", True), + ("ab", False), + ("a.b", True), + ("my.great.bucket.aws.com", True), + ("mY.GREAT.bucket.aws.com", False), + ("192.168.1.1", False), + ], +) +def test_aws_is_virtual_hostable_s3_bucket_allow_subdomains( + rule_lib, bucket, expected_value +): + assert ( + rule_lib.aws_is_virtual_hostable_s3_bucket(bucket, True) + == expected_value + ) From 201f964c2a3cb3eab35a335fbbcf6c221abe6954 Mon Sep 17 00:00:00 2001 From: davidlm Date: Fri, 12 May 2023 16:40:09 -0400 Subject: [PATCH 2/3] changelog --- .changes/next-release/bugfix-EndpointProvider-38157.json | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 .changes/next-release/bugfix-EndpointProvider-38157.json diff --git a/.changes/next-release/bugfix-EndpointProvider-38157.json b/.changes/next-release/bugfix-EndpointProvider-38157.json new file mode 100644 index 0000000000..b27d413488 --- /dev/null +++ b/.changes/next-release/bugfix-EndpointProvider-38157.json @@ -0,0 +1,5 @@ +{ + "type": "bugfix", + "category": "EndpointProvider", + "description": "Addresses issue `#2938 `__" +} From 3dbb49d9bae8bda753ceaee2abd2f60a9d757102 Mon Sep 17 00:00:00 2001 From: davidlm Date: Fri, 12 May 2023 17:02:59 -0400 Subject: [PATCH 3/3] pr feedback --- .../bugfix-EndpointProvider-38157.json | 2 +- tests/functional/test_s3.py | 14 -------------- 2 files changed, 1 insertion(+), 15 deletions(-) diff --git a/.changes/next-release/bugfix-EndpointProvider-38157.json b/.changes/next-release/bugfix-EndpointProvider-38157.json index b27d413488..67fb17c5fb 100644 --- a/.changes/next-release/bugfix-EndpointProvider-38157.json +++ b/.changes/next-release/bugfix-EndpointProvider-38157.json @@ -1,5 +1,5 @@ { "type": "bugfix", "category": "EndpointProvider", - "description": "Addresses issue `#2938 `__" + "description": "Fixed bug in virtual addressing for S3 Buckets `#2938 `__" } diff --git a/tests/functional/test_s3.py b/tests/functional/test_s3.py index e82a6ded80..b62b0b6b78 100644 --- a/tests/functional/test_s3.py +++ b/tests/functional/test_s3.py @@ -3498,13 +3498,6 @@ def _addressing_for_presigned_url_test_cases(): customer_provided_endpoint="http://s3.us-west-2.amazonaws.com", expected_url="http://foo.b.biz.s3.us-west-2.amazonaws.com/key", ) - yield dict( - bucket="foo.b.biz", - key="key", - s3_config={"addressing_style": "virtual"}, - is_secure=False, - expected_url="http://s3.amazonaws.com/foo.b.biz/key", - ) yield dict( bucket="foo.b.biz", key="key", @@ -3512,13 +3505,6 @@ def _addressing_for_presigned_url_test_cases(): customer_provided_endpoint="https://s3.us-west-2.amazonaws.com", expected_url="https://s3.us-west-2.amazonaws.com/foo.b.biz/key", ) - yield dict( - region="us-west-1", - bucket="foo.b.biz", - key="key", - s3_config={"addressing_style": "virtual"}, - expected_url="https://s3.us-west-1.amazonaws.com/foo.b.biz/key", - ) @pytest.mark.parametrize(