From 99e64ddfa0705f638f82b2b9bd2121bf31d453cb Mon Sep 17 00:00:00 2001 From: Hardik Kaurani Date: Thu, 29 Oct 2026 13:39:46 +0530 Subject: [PATCH 1/2] fix(abstractions): safely handle missing header values and normalize return in remove_value --- .../kiota_abstractions/headers_collection.py | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/packages/abstractions/kiota_abstractions/headers_collection.py b/packages/abstractions/kiota_abstractions/headers_collection.py index 8276bf96..94fa1c5b 100644 --- a/packages/abstractions/kiota_abstractions/headers_collection.py +++ b/packages/abstractions/kiota_abstractions/headers_collection.py @@ -146,7 +146,9 @@ def remove_value(self, header_name: str, header_value: str) -> Union[bool, set[s header_value (str): The value to remove from the header. Returns: - bool: _description_ + Union[bool, set[str]]: Remaining values set if header still has other values, + True if the value was removed and no values remain, or False if header or value + was not found. """ if not header_name: raise ValueError("Header name cannot be null") @@ -154,11 +156,14 @@ def remove_value(self, header_name: str, header_value: str) -> Union[bool, set[s raise ValueError("Header value cannot be null") header_name = header_name.lower() values = self.try_get(header_name) - if values: - values.remove(header_value) #type: ignore + if isinstance(values, set): + if header_value not in values: + return False + values.remove(header_value) if bool(values): return values - return self.remove(header_name) + self.remove(header_name) + return True return False @@ -169,7 +174,7 @@ def remove(self, header_name: str) -> Union[bool, set[str]]: header_name (str): The name of the header to remove. Returns: - bool: True if the header has been removed, False otherwise. + Union[bool, set[str]]: The removed set of values if present, False otherwise. """ if not header_name: raise ValueError("Header name cannot be null") From 838626db407851131a15d0a8a0eb44646059dcfa Mon Sep 17 00:00:00 2001 From: Hardik Kaurani Date: Thu, 29 Oct 2026 13:42:00 +0530 Subject: [PATCH 2/2] test(abstractions): add test coverage for nonexistent value removal and return semantics --- .../abstractions/tests/test_request_header.py | 36 +++++++++++++++++-- 1 file changed, 33 insertions(+), 3 deletions(-) diff --git a/packages/abstractions/tests/test_request_header.py b/packages/abstractions/tests/test_request_header.py index 98ee3cd5..be7696e4 100644 --- a/packages/abstractions/tests/test_request_header.py +++ b/packages/abstractions/tests/test_request_header.py @@ -118,14 +118,15 @@ def test_removes_value_from_existing_header(): """Tests removing a value from an existing header """ headers = HeadersCollection() - headers.remove_value("header1", "value1") + assert headers.remove_value("header1", "value1") is False headers.add("header1", "value1") headers.add("header1", "value2") assert headers.contains("header1") assert headers.count() == 1 - headers.remove_value("header1", "value1") + remaining = headers.remove_value("header1", "value1") + assert remaining == {"value2"} assert {"value2"} <= headers.try_get("header1") - headers.remove_value("header1", "value2") + assert headers.remove_value("header1", "value2") is True assert not headers.contains("header1") assert headers.count() == 0 @@ -303,3 +304,32 @@ def test_subscript_assignment_single_value_header(): headers["Content-Type"] = ["application/json", "text/plain"] assert headers["content-type"] == {"application/json"} + +def test_remove_value_nonexistent_value_returns_false(): + """Tests removing a nonexistent value from an existing header returns False without raising KeyError""" + headers = HeadersCollection() + headers.add("header1", ["value1", "value2"]) + assert headers.count() == 1 + assert headers.remove_value("header1", "nonexistent") is False + assert headers.get("header1") == {"value1", "value2"} + assert headers.count() == 1 + + +def test_remove_value_case_insensitive_header_name(): + """Tests remove_value normalizes header name casing""" + headers = HeadersCollection() + headers.add("Content-Type", "application/json") + assert headers.remove_value("content-type", "application/json") is True + assert not headers.contains("content-type") + assert headers.count() == 0 + + +def test_remove_header_return_semantics(): + """Tests that remove returns the popped set when present, and False when not found""" + headers = HeadersCollection() + assert headers.remove("nonexistent") is False + headers.add("header1", ["value1", "value2"]) + removed = headers.remove("header1") + assert removed == {"value1", "value2"} + assert not headers.contains("header1") + assert headers.remove("header1") is False