Repository: boto/botocore
Pull request #2941 (2023-05-12 20:33:05)

Address virtual host style addressing bugs for short subdomains provided in ``endpoint_url``
This addresses https://github.com/boto/botocore/issues/2938. Currently `RuleSetStandardLibrary.aws_is_virtual_hostable_s3_bucket` is performing the exact same validations on the entire input value as it does with individual subdomains within the input value. This is incorrect because only the entire value needs to be at least 3 characters, not each individual component. As described in the issue, this caused a bug when enabling virtual style addressing and providing an `endpoint_url` to an S3 client that is insecure (http not https). For bucket names that look like URLs containing short subdomain fragments, path style is being used instead which is incorrect. 

Additionally, we only need to perform validation that the input value is not an IPV4 address, None and all lowercase at the top level so the recursive check has been removed.

codecov-commenter: ## [Codecov](https://app.codecov.io/gh/boto/botocore/pull/2941?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=boto) Report
Patch coverage: **`100.00`**% and project coverage change: **`-0.02`** :warning:
> Comparison is base [(`22a19ea`)](https://app.codecov.io/gh/boto/botocore/commit/22a19ea7c4c2c4dd7df4ab8c32733cba0c7597a4?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=boto) 93.29% compared to head [(`f975e1c`)](https://app.codecov.io/gh/boto/botocore/pull/2941?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=boto) 93.28%.

> :exclamation: Current head f975e1c differs from pull request most recent head 201f964. Consider uploading reports for the commit 201f964 to get more accurate results

:exclamation: Your organization is not using the GitHub App Integration. As a result you may experience degraded service beginning May 15th. Please [install the Github App Integration](https://github.com/apps/codecov) for your organization. [Read more](https://about.codecov.io/blog/codecov-is-updating-its-github-integration/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=boto).

<details><summary>Additional details and impacted files</summary>


```diff
@@             Coverage Diff             @@
##           develop    #2941      +/-   ##
===========================================
- Coverage    93.29%   93.28%   -0.02%     
===========================================
  Files           64       64              
  Lines        13590    13588       -2     
===========================================
- Hits         12679    12675       -4     
- Misses         911      913       +2     
```


| [Impacted Files](https://app.codecov.io/gh/boto/botocore/pull/2941?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=boto) | Coverage Δ | |
|---|---|---|
| [botocore/endpoint\_provider.py](https://app.codecov.io/gh/boto/botocore/pull/2941?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=boto#diff-Ym90b2NvcmUvZW5kcG9pbnRfcHJvdmlkZXIucHk=) | `99.01% <100.00%> (-0.01%)` | :arrow_down: |

... and [1 file with indirect coverage changes](https://app.codecov.io/gh/boto/botocore/pull/2941/indirect-changes?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=boto)


</details>

[:umbrella: View full report in Codecov by Sentry](https://app.codecov.io/gh/boto/botocore/pull/2941?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=boto).   
:loudspeaker: Do you have feedback about the report comment? [Let us know in this issue](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=boto).


Patch:
diff --git a/.changes/next-release/bugfix-EndpointProvider-38157.json b/.changes/next-release/bugfix-EndpointProvider-38157.json
new file mode 100644
index 000000000..67fb17c5f
--- /dev/null
+++ b/.changes/next-release/bugfix-EndpointProvider-38157.json
@@ -0,0 +1,5 @@
+{
+  "type": "bugfix",
+  "category": "EndpointProvider",
+  "description": "Fixed bug in virtual addressing for S3 Buckets `#2938 <https://github.com/boto/botocore/issues/2938>`__"
+}
diff --git a/botocore/endpoint_provider.py b/botocore/endpoint_provider.py
index 62bf087cc..1be5a25c8 100644
--- a/botocore/endpoint_provider.py
+++ b/botocore/endpoint_provider.py
@@ -398,13 +398,9 @@ class RuleSetStandardLibrary:
         ):
             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


Test patch:
diff --git a/tests/functional/test_s3.py b/tests/functional/test_s3.py
index f9a13b20b..b62b0b6b7 100644
--- a/tests/functional/test_s3.py
+++ b/tests/functional/test_s3.py
@@ -3487,6 +3487,24 @@ 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"},
+        customer_provided_endpoint="https://s3.us-west-2.amazonaws.com",
+        expected_url="https://s3.us-west-2.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 5f892a63c..51a07079b 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
+    )