Use urljoin, not string cat, for urls (#746)

* fix: use urljoin, not string cat, for urls

* chore: update changelog

Author: gadomski

CHANGELOG.md

@@ -7,6 +7,10 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0.
 
 ## [Unreleased]
 
+### Fixed
+
+- Use urljoin to build hrefs [#746](https://github.com/stac-utils/pystac-client/pull/746)
+
 ## [v0.8.4] - 2024-10-16
 
 ### Added

pystac_client/_utils.py

@@ -1,3 +1,4 @@
+import urllib
 import warnings
 from typing import Any, Callable, Dict, Optional, Union
 
@@ -25,3 +26,12 @@ def call_modifier(
             "a function that returns 'None' or silence this warning.",
             IgnoredResultWarning,
         )
+
+
+def urljoin(href: str, name: str) -> str:
+    """Joins a path onto an existing href, respecting query strings, etc."""
+    url = urllib.parse.urlparse(href)
+    path = url.path
+    if not path.endswith("/"):
+        path += "/"
+    return urllib.parse.urlunparse(url._replace(path=path + name))

pystac_client/client.py

@@ -20,7 +20,7 @@
 from pystac.layout import APILayoutStrategy, HrefLayoutStrategy
 from requests import Request
 
-from pystac_client._utils import Modifiable, call_modifier
+from pystac_client._utils import Modifiable, call_modifier, urljoin
 from pystac_client.collection_client import CollectionClient
 from pystac_client.collection_search import CollectionSearch
 from pystac_client.conformance import ConformanceClasses
@@ -786,11 +786,11 @@ def _collections_href(self, collection_id: Optional[str] = None) -> str:
         data_link = self.get_single_link("data")
         href = self._get_href("data", data_link, "collections")
         if collection_id is not None:
-            return f"{href.rstrip('/')}/{collection_id}"
+            return urljoin(href, collection_id)
         return href
 
     def _get_collection_queryables_href(
         self, collection_id: Optional[str] = None
     ) -> str:
         href = self._collections_href(collection_id)
-        return f"{href.rstrip('/')}/{QUERYABLES_ENDPOINT}"
+        return urljoin(href, QUERYABLES_ENDPOINT)

pystac_client/mixins.py

@@ -3,6 +3,7 @@
 
 import pystac
 
+from pystac_client._utils import urljoin
 from pystac_client.conformance import ConformanceClasses
 from pystac_client.exceptions import APIError
 from pystac_client.stac_api_io import StacApiIO
@@ -25,7 +26,7 @@ def _get_href(self, rel: str, link: Optional[pystac.Link], endpoint: str) -> str
             href = link.absolute_href
         else:
             warnings.warn(MissingLink(rel, self.__class__.__name__), stacklevel=2)
-            href = f"{self.self_href.rstrip('/')}/{endpoint}"
+            href = urljoin(self.self_href, endpoint)
         return href
 
 

tests/cassettes/test_client/test_query_string_in_collections_url.yaml

@@ -791,3 +791,10 @@ def test_fallback_strategy() -> None:
 
     assert (item_root := item.get_single_link("root"))
     assert item_root.href == root_href
+
+
+@pytest.mark.vcr
+def test_query_string_in_collections_url() -> None:
+    # https://github.com/stac-utils/pystac-client/issues/745
+    client = Client.open("https://paituli.csc.fi/geoserver/ogc/stac/v1")
+    client.get_collection("sentinel2-l2a")