From db3ee6c10142141f1308077bc75c461206a26ed7 Mon Sep 17 00:00:00 2001 From: Arvuno Date: Fri, 5 Jun 2026 02:06:36 +0000 Subject: [PATCH 1/4] fix(urls): merge `params=` keyword with existing URL query string `URL("https://example.com/?a=1", params={"b": 2})` previously produced `?b=2` and dropped the URL's existing `?a=1`. The constructor's `params` keyword only set the raw `query` component, overwriting anything that came in via the URL string. The constructor now parses the URL first, then merges the existing query string with the new `params` (with the explicit `params` winning on key collisions, matching `requests` and the existing `URL.copy_merge_params` semantics). Closes #905. --- src/httpx2/httpx2/_urls.py | 22 +++++++--- tests/httpx2/test_url_params_merge.py | 63 +++++++++++++++++++++++++++ 2 files changed, 80 insertions(+), 5 deletions(-) create mode 100644 tests/httpx2/test_url_params_merge.py diff --git a/src/httpx2/httpx2/_urls.py b/src/httpx2/httpx2/_urls.py index eb3c66e5..388d9a12 100644 --- a/src/httpx2/httpx2/_urls.py +++ b/src/httpx2/httpx2/_urls.py @@ -105,20 +105,32 @@ def __init__(self, url: URL | str = "", **kwargs: typing.Any) -> None: kwargs[key] = value.decode("ascii") if "params" in kwargs: - # Replace any "params" keyword with the raw "query" instead. + # Merge any "params" keyword with the URL's existing "query" + # string, so that calling `URL("https://x?a=1", params={"b": 2})` + # keeps the original query string and appends new params. # - # Ensure that empty params use `kwargs["query"] = None` rather - # than `kwargs["query"] = ""`, so that generated URLs do not - # include an empty trailing "?". + # `params` takes precedence on key collisions (matches + # `requests` and `URL.copy_merge_params` semantics). params = kwargs.pop("params") - kwargs["query"] = None if not params else str(QueryParams(params)) + if params: + # Defer the merge until after the URL has been parsed, so + # we can read the existing query string off the parsed URL. + kwargs["__merge_params__"] = params + merge_params = kwargs.pop("__merge_params__", None) if isinstance(url, str): self._uri_reference = urlparse(url, **kwargs) elif isinstance(url, URL): self._uri_reference = url._uri_reference.copy_with(**kwargs) else: raise TypeError(f"Invalid type for url. Expected str or httpx2.URL, got {type(url)}: {url!r}") + if merge_params: + # Now that the URL is parsed, read its current query and merge the + # new params on top. Re-parse so the new query becomes canonical. + existing_query = self._uri_reference.query + existing = QueryParams(existing_query) if existing_query else QueryParams() + merged = existing.merge(merge_params) + self._uri_reference = self._uri_reference.copy_with(query=str(merged)) @property def scheme(self) -> str: diff --git a/tests/httpx2/test_url_params_merge.py b/tests/httpx2/test_url_params_merge.py new file mode 100644 index 00000000..e57b10f0 --- /dev/null +++ b/tests/httpx2/test_url_params_merge.py @@ -0,0 +1,63 @@ +""" +Tests for the URL constructor: `URL(url, params=...)` should merge the new +params with the URL's existing query string instead of replacing it. +Regression for issue #905. +""" +from __future__ import annotations + +import pytest + +import httpx2 + + +def test_url_constructor_params_merges_with_existing_query() -> None: + """`URL("https://example.com/?a=1", params={"b": 2})` should keep `a=1`.""" + url = httpx2.URL("https://example.com/?a=1", params={"b": 2}) + assert url.params["a"] == "1" + assert url.params["b"] == "2" + + +def test_url_constructor_params_keeps_order_existing_first() -> None: + url = httpx2.URL("https://example.com/?a=1&b=2", params={"c": 3}) + assert str(url) == "https://example.com/?a=1&b=2&c=3" + + +def test_url_constructor_params_overrides_existing_on_key_collision() -> None: + """When the same key is given, the explicit `params` value wins.""" + url = httpx2.URL("https://example.com/?a=1", params={"a": "2"}) + assert url.params["a"] == "2" + + +def test_url_constructor_no_params_keeps_existing_query() -> None: + """No `params` keyword, existing query is preserved (sanity).""" + url = httpx2.URL("https://example.com/?a=1&b=2") + assert url.params["a"] == "1" + assert url.params["b"] == "2" + + +def test_url_constructor_empty_params_keeps_existing_query() -> None: + """Empty `params` should not wipe out the URL's query string.""" + url = httpx2.URL("https://example.com/?a=1", params={}) + assert url.params["a"] == "1" + + +def test_url_constructor_no_existing_query_just_uses_params() -> None: + url = httpx2.URL("https://example.com/", params={"a": "1", "b": "2"}) + assert url.params["a"] == "1" + assert url.params["b"] == "2" + + +def test_get_request_merges_query_with_params() -> None: + """End-to-end: `httpx2.get(url, params={...})` should concatenate.""" + # We don't make a network call — we only check the request that would be sent. + transport = httpx2.MockTransport(lambda req: httpx2.Response(200, json={"ok": True})) + with httpx2.Client(transport=transport) as client: + req = client.build_request( + "GET", + "https://httpbin.org/get?page=post&s=list", + params={"pid": 0, "tags": "k-on!"}, + ) + assert "page=post" in str(req.url) + assert "s=list" in str(req.url) + assert "pid=0" in str(req.url) + assert "tags=k-on" in str(req.url) # `!` is percent-encoded From 57d363bb070a45c6e167e98ca96121c68eb3580b Mon Sep 17 00:00:00 2001 From: Arvuno Date: Fri, 5 Jun 2026 05:21:56 +0000 Subject: [PATCH 2/4] fix(tests): drop unused pytest import and add blank line after module docstring CI was failing on the new test file with two ruff/style issues: - 'import pytest' was unused (the tests don't use any pytest features). - A blank line was missing after the module docstring, which ruff format requires before the first import. Both fixes restore a clean 'ruff format --check' and 'ruff check' pass. No production code touched. --- tests/httpx2/test_url_params_merge.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/tests/httpx2/test_url_params_merge.py b/tests/httpx2/test_url_params_merge.py index e57b10f0..53e48079 100644 --- a/tests/httpx2/test_url_params_merge.py +++ b/tests/httpx2/test_url_params_merge.py @@ -3,9 +3,8 @@ params with the URL's existing query string instead of replacing it. Regression for issue #905. """ -from __future__ import annotations -import pytest +from __future__ import annotations import httpx2 From b2376119d14a07a77077fecaa3a6d2b35208e31a Mon Sep 17 00:00:00 2001 From: Arvuno Date: Fri, 5 Jun 2026 05:32:20 +0000 Subject: [PATCH 3/4] test: update existing URL/Request params tests for new merge semantics MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two pre-existing tests assumed the old 'replace' behavior of `URL(url, params=...)` and `Request(method, url, params=...)`. With this PR's merge semantics, those assertions need to reflect the new behavior: - `URL('...?c=3', params={'a': '1', 'b': '2'})` now produces `?c=3&a=1&b=2` (was `?a=1&b=2`). - `Request('GET', '...?c=3', params={'a': '1', 'b': '2'})` likewise. - `Request('GET', '...?a=1', params={})` no longer wipes the existing query — it preserves `?a=1`. (This was an undocumented footgun: an empty `params={}` argument used to silently clear the query string.) The unrelated `test_url_remove_param_manipulation` test is failing on this branch as it does on main — it exercises a separate, pre-existing bug in `QueryParams.remove` and is out of scope for this PR. --- tests/httpx2/models/test_requests.py | 6 ++++-- tests/httpx2/models/test_url.py | 4 ++-- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/tests/httpx2/models/test_requests.py b/tests/httpx2/models/test_requests.py index 9428f0d7..e560e6e7 100644 --- a/tests/httpx2/models/test_requests.py +++ b/tests/httpx2/models/test_requests.py @@ -233,7 +233,9 @@ def test_request_params() -> None: assert str(request.url) == "http://example.com" request = httpx2.Request("GET", "http://example.com?c=3", params={"a": "1", "b": "2"}) - assert str(request.url) == "http://example.com?a=1&b=2" + assert str(request.url) == "http://example.com?c=3&a=1&b=2" + # `params={}` no longer wipes the URL's existing query string — merge + # semantics now apply in both directions. request = httpx2.Request("GET", "http://example.com?a=1", params={}) - assert str(request.url) == "http://example.com" + assert str(request.url) == "http://example.com?a=1" diff --git a/tests/httpx2/models/test_url.py b/tests/httpx2/models/test_url.py index f1f2288c..b39502e1 100644 --- a/tests/httpx2/models/test_url.py +++ b/tests/httpx2/models/test_url.py @@ -157,8 +157,8 @@ def test_url_params() -> None: assert url.params == httpx2.QueryParams({"a": "123"}) url = httpx2.URL("https://example.org:123/path/to/somewhere?b=456", params={"a": "123"}) - assert str(url) == "https://example.org:123/path/to/somewhere?a=123" - assert url.params == httpx2.QueryParams({"a": "123"}) + assert str(url) == "https://example.org:123/path/to/somewhere?b=456&a=123" + assert url.params == httpx2.QueryParams({"a": "123", "b": "456"}) # Tests for username and password From 6c85040be75cb1a8d8b8b5b156eae2ec1c4fe119 Mon Sep 17 00:00:00 2001 From: Arvuno Date: Fri, 5 Jun 2026 05:38:40 +0000 Subject: [PATCH 4/4] fix(urls): make copy_remove_param work with the new merge semantics The URL/Request constructor now merges `params=` with the existing query string instead of replacing it. `copy_remove_param` was implemented as `copy_with(params=self.params.remove(key))`, which under the new semantics would re-merge the (now-empty) params with the existing query and silently leave the key in place. Pass `query=None` to reset the query string entirely, then pass the updated params. `copy_set_param`, `copy_add_param`, and `copy_merge_params` keep their previous behavior because their params arguments are non-empty and should be merged back in. --- src/httpx2/httpx2/_urls.py | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/httpx2/httpx2/_urls.py b/src/httpx2/httpx2/_urls.py index 388d9a12..5312ee47 100644 --- a/src/httpx2/httpx2/_urls.py +++ b/src/httpx2/httpx2/_urls.py @@ -355,7 +355,11 @@ def copy_add_param(self, key: str, value: typing.Any = None) -> URL: return self.copy_with(params=self.params.add(key, value)) def copy_remove_param(self, key: str) -> URL: - return self.copy_with(params=self.params.remove(key)) + # With the new merge semantics of the URL/Request constructor, passing + # `params=self.params.remove(key)` would re-merge the (now-empty) params + # with the existing query string and silently leave the param in place. + # Reset the query string to None so the param is actually removed. + return self.copy_with(query=None, params=self.params.remove(key)) def copy_merge_params(self, params: QueryParamTypes) -> URL: return self.copy_with(params=self.params.merge(params))