From 9d473ac3f50cba2372dfd853eeb23e1c4504b1c0 Mon Sep 17 00:00:00 2001 From: SamRemis Date: Fri, 15 Nov 2024 14:35:46 -0500 Subject: [PATCH 1/8] Add support for tuples in get_attr --- botocore/endpoint_provider.py | 7 ++++--- tests/unit/test_endpoint_provider.py | 8 ++++++++ 2 files changed, 12 insertions(+), 3 deletions(-) diff --git a/botocore/endpoint_provider.py b/botocore/endpoint_provider.py index 38b0a5ffe6..75d0339824 100644 --- a/botocore/endpoint_provider.py +++ b/botocore/endpoint_provider.py @@ -42,7 +42,7 @@ logger = logging.getLogger(__name__) TEMPLATE_STRING_RE = re.compile(r"\{[a-zA-Z#]+\}") -GET_ATTR_RE = re.compile(r"(\w+)\[(\d+)\]") +GET_ATTR_RE = re.compile(r"(\w*)\[(\d+)\]") VALID_HOST_LABEL_RE = re.compile( r"^(?!-)[a-zA-Z\d-]{1,63}(?= len(value): return None + if name: + value = value.get(name) return value[index] else: value = value[part] diff --git a/tests/unit/test_endpoint_provider.py b/tests/unit/test_endpoint_provider.py index c1f82ace2e..0be04fc35e 100644 --- a/tests/unit/test_endpoint_provider.py +++ b/tests/unit/test_endpoint_provider.py @@ -510,3 +510,11 @@ def test_aws_is_virtual_hostable_s3_bucket_allow_subdomains( rule_lib.aws_is_virtual_hostable_s3_bucket(bucket, True) == expected_value ) + +def test_get_attr_can_get_dictionary_index(rule_lib): + result = rule_lib.get_attr({"foo": ['bar']}, 'foo[0]') + assert result == "bar" + +def test_get_attr_can_get_list_index(rule_lib): + result = rule_lib.get_attr(("foo"), '[0]') + assert result == "foo" From 31ed6dee44edd4f43018034b27afcc54c07512c0 Mon Sep 17 00:00:00 2001 From: SamRemis Date: Fri, 15 Nov 2024 15:01:35 -0500 Subject: [PATCH 2/8] Fix index ordering issue --- botocore/endpoint_provider.py | 6 +++--- tests/unit/test_endpoint_provider.py | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/botocore/endpoint_provider.py b/botocore/endpoint_provider.py index 75d0339824..40abc9cf6e 100644 --- a/botocore/endpoint_provider.py +++ b/botocore/endpoint_provider.py @@ -178,10 +178,10 @@ def get_attr(self, value, path): if match is not None: name, index = match.groups() index = int(index) - if value is None or index >= len(value): - return None - if name: + if name and value is not None: value = value.get(name) + if index >= len(value): + return None return value[index] else: value = value[part] diff --git a/tests/unit/test_endpoint_provider.py b/tests/unit/test_endpoint_provider.py index 0be04fc35e..b768792f35 100644 --- a/tests/unit/test_endpoint_provider.py +++ b/tests/unit/test_endpoint_provider.py @@ -516,5 +516,5 @@ def test_get_attr_can_get_dictionary_index(rule_lib): assert result == "bar" def test_get_attr_can_get_list_index(rule_lib): - result = rule_lib.get_attr(("foo"), '[0]') + result = rule_lib.get_attr(("foo",), '[0]') assert result == "foo" From 6fb6221c804ae51c8586ae4a18d8f97918916248 Mon Sep 17 00:00:00 2001 From: SamRemis Date: Fri, 15 Nov 2024 15:08:37 -0500 Subject: [PATCH 3/8] pre-commit and minor refactorings --- botocore/endpoint_provider.py | 4 +++- tests/unit/test_endpoint_provider.py | 2 ++ 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/botocore/endpoint_provider.py b/botocore/endpoint_provider.py index 40abc9cf6e..c0b19348e1 100644 --- a/botocore/endpoint_provider.py +++ b/botocore/endpoint_provider.py @@ -173,12 +173,14 @@ def get_attr(self, value, path): :type path: str :rtype: Any """ + if value is None: + return None for part in path.split("."): match = GET_ATTR_RE.search(part) if match is not None: name, index = match.groups() index = int(index) - if name and value is not None: + if name: value = value.get(name) if index >= len(value): return None diff --git a/tests/unit/test_endpoint_provider.py b/tests/unit/test_endpoint_provider.py index b768792f35..64cb84d4d9 100644 --- a/tests/unit/test_endpoint_provider.py +++ b/tests/unit/test_endpoint_provider.py @@ -511,10 +511,12 @@ def test_aws_is_virtual_hostable_s3_bucket_allow_subdomains( == expected_value ) + def test_get_attr_can_get_dictionary_index(rule_lib): result = rule_lib.get_attr({"foo": ['bar']}, 'foo[0]') assert result == "bar" + def test_get_attr_can_get_list_index(rule_lib): result = rule_lib.get_attr(("foo",), '[0]') assert result == "foo" From 73bcd875ea97974d25e1048c98bad72544c060a5 Mon Sep 17 00:00:00 2001 From: SamRemis Date: Wed, 20 Nov 2024 14:30:12 -0500 Subject: [PATCH 4/8] Move None check into loop --- botocore/endpoint_provider.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/botocore/endpoint_provider.py b/botocore/endpoint_provider.py index c0b19348e1..377564edfc 100644 --- a/botocore/endpoint_provider.py +++ b/botocore/endpoint_provider.py @@ -173,9 +173,9 @@ def get_attr(self, value, path): :type path: str :rtype: Any """ - if value is None: - return None for part in path.split("."): + if value is None: + return None match = GET_ATTR_RE.search(part) if match is not None: name, index = match.groups() From 762f499acd569b7b7916e5386e67c01fa5b2eb1e Mon Sep 17 00:00:00 2001 From: SamRemis Date: Thu, 21 Nov 2024 10:31:45 -0500 Subject: [PATCH 5/8] Update endpoint_provider.py --- botocore/endpoint_provider.py | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/botocore/endpoint_provider.py b/botocore/endpoint_provider.py index 377564edfc..0fbd802b8d 100644 --- a/botocore/endpoint_provider.py +++ b/botocore/endpoint_provider.py @@ -174,15 +174,13 @@ def get_attr(self, value, path): :rtype: Any """ for part in path.split("."): - if value is None: - return None match = GET_ATTR_RE.search(part) if match is not None: name, index = match.groups() index = int(index) if name: value = value.get(name) - if index >= len(value): + if value is None or index >= len(value): return None return value[index] else: From dec0fb978c4cb9acd4b22069a5a6b8940f918079 Mon Sep 17 00:00:00 2001 From: SamRemis Date: Thu, 21 Nov 2024 10:46:15 -0500 Subject: [PATCH 6/8] Update test_endpoint_provider.py --- tests/unit/test_endpoint_provider.py | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/tests/unit/test_endpoint_provider.py b/tests/unit/test_endpoint_provider.py index 64cb84d4d9..7dc906688b 100644 --- a/tests/unit/test_endpoint_provider.py +++ b/tests/unit/test_endpoint_provider.py @@ -517,6 +517,16 @@ def test_get_attr_can_get_dictionary_index(rule_lib): assert result == "bar" +def test_get_attr_returns_none_on_missing_key(rule_lib): + result = rule_lib.get_attr({"foo": ['bar']}, 'baz[0]') + assert result is None + + +def test_get_attr_returns_none_on_too_high_index(rule_lib): + result = rule_lib.get_attr({"foo": ['bar']}, 'foo[1]') + assert result is None + + def test_get_attr_can_get_list_index(rule_lib): result = rule_lib.get_attr(("foo",), '[0]') assert result == "foo" From 1188f330174bc129f93d4b07a48a9d08565af3f0 Mon Sep 17 00:00:00 2001 From: SamRemis Date: Thu, 21 Nov 2024 11:39:54 -0500 Subject: [PATCH 7/8] Parametrize tests --- tests/unit/test_endpoint_provider.py | 34 +++++++++++++--------------- 1 file changed, 16 insertions(+), 18 deletions(-) diff --git a/tests/unit/test_endpoint_provider.py b/tests/unit/test_endpoint_provider.py index 7dc906688b..b6428e5179 100644 --- a/tests/unit/test_endpoint_provider.py +++ b/tests/unit/test_endpoint_provider.py @@ -512,21 +512,19 @@ def test_aws_is_virtual_hostable_s3_bucket_allow_subdomains( ) -def test_get_attr_can_get_dictionary_index(rule_lib): - result = rule_lib.get_attr({"foo": ['bar']}, 'foo[0]') - assert result == "bar" - - -def test_get_attr_returns_none_on_missing_key(rule_lib): - result = rule_lib.get_attr({"foo": ['bar']}, 'baz[0]') - assert result is None - - -def test_get_attr_returns_none_on_too_high_index(rule_lib): - result = rule_lib.get_attr({"foo": ['bar']}, 'foo[1]') - assert result is None - - -def test_get_attr_can_get_list_index(rule_lib): - result = rule_lib.get_attr(("foo",), '[0]') - assert result == "foo" +@pytest.mark.parametrize( + "value, path, expected_value", + [ + ({"foo": ['bar']}, 'baz[0]', None), # Missing index + ({"foo": ['bar']}, 'foo[1]', None), # Out of range index + ({"foo": ['bar']}, 'foo[0]', "bar"), # Named index + (("foo",), '[0]', "foo"), # Bare index + ({"foo": {'bar': []}}, 'foo.baz[0]', None), # Missing subindex + ({"foo": {'bar': []}}, 'foo.bar[0]', None), # Out of range subindex + ({"foo": {"bar": "baz"}}, 'foo.bar', "baz"), # Subindex with named index + ({"foo": {"bar": ["baz"]}}, 'foo.bar[0]', "baz"), # Subindex with numeric index + ], +) +def test_get_attr(rule_lib, value, path, expected_value): + result = rule_lib.get_attr(value, path) + assert result == expected_value \ No newline at end of file From 88abe7fb1df2a409b04a41cec9bff94f0a7701a4 Mon Sep 17 00:00:00 2001 From: SamRemis Date: Thu, 21 Nov 2024 11:42:08 -0500 Subject: [PATCH 8/8] Update test_endpoint_provider.py --- tests/unit/test_endpoint_provider.py | 22 +++++++++++++++++----- 1 file changed, 17 insertions(+), 5 deletions(-) diff --git a/tests/unit/test_endpoint_provider.py b/tests/unit/test_endpoint_provider.py index b6428e5179..8bc8c429d0 100644 --- a/tests/unit/test_endpoint_provider.py +++ b/tests/unit/test_endpoint_provider.py @@ -519,12 +519,24 @@ def test_aws_is_virtual_hostable_s3_bucket_allow_subdomains( ({"foo": ['bar']}, 'foo[1]', None), # Out of range index ({"foo": ['bar']}, 'foo[0]', "bar"), # Named index (("foo",), '[0]', "foo"), # Bare index - ({"foo": {'bar': []}}, 'foo.baz[0]', None), # Missing subindex - ({"foo": {'bar': []}}, 'foo.bar[0]', None), # Out of range subindex - ({"foo": {"bar": "baz"}}, 'foo.bar', "baz"), # Subindex with named index - ({"foo": {"bar": ["baz"]}}, 'foo.bar[0]', "baz"), # Subindex with numeric index + ({"foo": {}}, 'foo.bar[0]', None), # Missing index from split path + ( + {"foo": {'bar': []}}, + 'foo.bar[0]', + None, + ), # Out of range from split path + ( + {"foo": {"bar": "baz"}}, + 'foo.bar', + "baz", + ), # Split path with named index + ( + {"foo": {"bar": ["baz"]}}, + 'foo.bar[0]', + "baz", + ), # Split path with numeric index ], ) def test_get_attr(rule_lib, value, path, expected_value): result = rule_lib.get_attr(value, path) - assert result == expected_value \ No newline at end of file + assert result == expected_value