Skip to content

Commit c3ecaa9

Browse files
authored
fix: Normalize query params in dataset create_items_public_url (#963)
`DatasetClient.create_items_public_url` (and its async twin) passed `_build_params` output straight to `urlencode`, bypassing the `_parse_params` normalization that the real HTTP request path applies. As a result, boolean and list query params ended up as Python reprs in the signed URL — e.g. `create_items_public_url(clean=True, fields=['title', 'url'])` produced `clean=True&fields=%5B%27title%27%2C+%27url%27%5D` instead of `clean=true&fields=title,url`. Consumers of the shared URL then got unclean/unfiltered items or an API error. The fix routes the params through `self._http_client._parse_params(...)` before `urlencode`, so the public URL matches exactly what an actual API request would send (bool→`true`/`false`, list→comma-joined, `None` dropped). Added regression tests for both the sync and async clients. *✍️ Drafted by Claude Code*
1 parent 55e88ef commit c3ecaa9

4 files changed

Lines changed: 66 additions & 47 deletions

File tree

src/apify_client/_resource_clients/_resource_client.py

Lines changed: 25 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
from datetime import UTC, datetime, timedelta
66
from functools import cached_property
77
from typing import TYPE_CHECKING, Any, Literal, get_args
8+
from urllib.parse import urlencode, urlparse, urlunparse
89

910
from apify_client._consts import DEFAULT_WAIT_FOR_FINISH, DEFAULT_WAIT_WHEN_JOB_NOT_EXIST
1011
from apify_client._docs import docs_group
@@ -115,6 +116,25 @@ def _build_url(
115116

116117
return url
117118

119+
def _build_public_url(self, path: str, params: dict[str, Any]) -> str:
120+
"""Build a public resource URL with API-normalized query params.
121+
122+
Normalizes `params` the same way the HTTP request path does (bool -> true/false, list -> comma-joined,
123+
datetime -> ISO 8601 Zulu) so the shareable URL matches what the client would send over the wire.
124+
125+
Args:
126+
path: Path segment appended to the resource URL (e.g. 'items', 'keys').
127+
params: Query parameters to normalize and append.
128+
129+
Returns:
130+
The public URL with a normalized query string.
131+
"""
132+
public_url = urlparse(self._build_url(path, public=True))
133+
filtered_params = self._http_client._parse_params(params) # noqa: SLF001
134+
if filtered_params:
135+
public_url = public_url._replace(query=urlencode(filtered_params))
136+
return urlunparse(public_url)
137+
118138
def _build_params(self, **kwargs: Any) -> dict:
119139
"""Merge default params with method params, filtering out None values.
120140
@@ -133,7 +153,7 @@ def _clean_json_payload(data: dict) -> dict:
133153
134154
The Apify API ignores missing fields but may reject fields explicitly set to None.
135155
Nested sub-models serialized by Pydantic may produce empty dicts when all their
136-
fields are None these are also removed.
156+
fields are None - these are also removed.
137157
138158
Uses an iterative stack-based approach, analogous to _build_params for query params.
139159
"""
@@ -203,7 +223,7 @@ def _get(self, *, timeout: Timeout) -> dict | None:
203223
"""Perform a GET request for this resource, returning the parsed response or None if not found.
204224
205225
404s collapse to `None` only for ID-identified clients. Chained clients without a `resource_id`
206-
(e.g. `run.dataset()`) propagate `NotFoundError` see `catch_not_found_for_resource_or_throw`.
226+
(e.g. `run.dataset()`) propagate `NotFoundError` - see `catch_not_found_for_resource_or_throw`.
207227
"""
208228
try:
209229
response = self._http_client.call(
@@ -232,7 +252,7 @@ def _delete(self, *, timeout: Timeout) -> None:
232252
"""Perform a DELETE request to delete this resource.
233253
234254
404s are swallowed (idempotent DELETE) only for ID-identified clients. Chained clients without a
235-
`resource_id` propagate `NotFoundError` see `catch_not_found_for_resource_or_throw`.
255+
`resource_id` propagate `NotFoundError` - see `catch_not_found_for_resource_or_throw`.
236256
"""
237257
try:
238258
self._http_client.call(
@@ -395,7 +415,7 @@ async def _get(self, *, timeout: Timeout) -> dict | None:
395415
"""Perform a GET request for this resource, returning the parsed response or None if not found.
396416
397417
404s collapse to `None` only for ID-identified clients. Chained clients without a `resource_id`
398-
(e.g. `run.dataset()`) propagate `NotFoundError` see `catch_not_found_for_resource_or_throw`.
418+
(e.g. `run.dataset()`) propagate `NotFoundError` - see `catch_not_found_for_resource_or_throw`.
399419
"""
400420
try:
401421
response = await self._http_client.call(
@@ -424,7 +444,7 @@ async def _delete(self, *, timeout: Timeout) -> None:
424444
"""Perform a DELETE request to delete this resource.
425445
426446
404s are swallowed (idempotent DELETE) only for ID-identified clients. Chained clients without a
427-
`resource_id` propagate `NotFoundError` see `catch_not_found_for_resource_or_throw`.
447+
`resource_id` propagate `NotFoundError` - see `catch_not_found_for_resource_or_throw`.
428448
"""
429449
try:
430450
await self._http_client.call(

src/apify_client/_resource_clients/dataset.py

Lines changed: 2 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,6 @@
33
from contextlib import asynccontextmanager, contextmanager
44
from dataclasses import dataclass
55
from typing import TYPE_CHECKING, Any
6-
from urllib.parse import urlencode, urlparse, urlunparse
76

87
from apify_client._docs import docs_group
98
from apify_client._models import Dataset, DatasetResponse, DatasetStatistics, DatasetStatisticsResponse
@@ -606,12 +605,7 @@ def create_items_public_url(
606605
)
607606
request_params['signature'] = signature
608607

609-
items_public_url = urlparse(self._build_url('items', public=True))
610-
filtered_params = {k: v for k, v in request_params.items() if v is not None}
611-
if filtered_params:
612-
items_public_url = items_public_url._replace(query=urlencode(filtered_params))
613-
614-
return urlunparse(items_public_url)
608+
return self._build_public_url('items', request_params)
615609

616610

617611
@docs_group('Resource clients')
@@ -1172,9 +1166,4 @@ async def create_items_public_url(
11721166
)
11731167
request_params['signature'] = signature
11741168

1175-
items_public_url = urlparse(self._build_url('items', public=True))
1176-
filtered_params = {k: v for k, v in request_params.items() if v is not None}
1177-
if filtered_params:
1178-
items_public_url = items_public_url._replace(query=urlencode(filtered_params))
1179-
1180-
return urlunparse(items_public_url)
1169+
return self._build_public_url('items', request_params)

src/apify_client/_resource_clients/key_value_store.py

Lines changed: 4 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,6 @@
44
from contextlib import asynccontextmanager, contextmanager
55
from http import HTTPStatus
66
from typing import TYPE_CHECKING, Any
7-
from urllib.parse import urlencode, urlparse, urlunparse
87

98
from apify_client._docs import docs_group
109
from apify_client._models import (
@@ -425,13 +424,7 @@ def get_record_public_url(self, key: str, *, timeout: Timeout = 'long') -> str:
425424
if metadata and metadata.url_signing_secret_key:
426425
request_params['signature'] = create_hmac_signature(metadata.url_signing_secret_key, key)
427426

428-
key_public_url = urlparse(self._build_url(f'records/{key}', public=True))
429-
filtered_params = {k: v for k, v in request_params.items() if v is not None}
430-
431-
if filtered_params:
432-
key_public_url = key_public_url._replace(query=urlencode(filtered_params))
433-
434-
return urlunparse(key_public_url)
427+
return self._build_public_url(f'records/{key}', request_params)
435428

436429
def create_keys_public_url(
437430
self,
@@ -482,13 +475,7 @@ def create_keys_public_url(
482475
)
483476
request_params['signature'] = signature
484477

485-
keys_public_url = urlparse(self._build_url('keys', public=True))
486-
487-
filtered_params = {k: v for k, v in request_params.items() if v is not None}
488-
if filtered_params:
489-
keys_public_url = keys_public_url._replace(query=urlencode(filtered_params))
490-
491-
return urlunparse(keys_public_url)
478+
return self._build_public_url('keys', request_params)
492479

493480

494481
@docs_group('Resource clients')
@@ -853,13 +840,7 @@ async def get_record_public_url(self, key: str, *, timeout: Timeout = 'long') ->
853840
if metadata and metadata.url_signing_secret_key:
854841
request_params['signature'] = create_hmac_signature(metadata.url_signing_secret_key, key)
855842

856-
key_public_url = urlparse(self._build_url(f'records/{key}', public=True))
857-
filtered_params = {k: v for k, v in request_params.items() if v is not None}
858-
859-
if filtered_params:
860-
key_public_url = key_public_url._replace(query=urlencode(filtered_params))
861-
862-
return urlunparse(key_public_url)
843+
return self._build_public_url(f'records/{key}', request_params)
863844

864845
async def create_keys_public_url(
865846
self,
@@ -910,10 +891,4 @@ async def create_keys_public_url(
910891
)
911892
request_params['signature'] = signature
912893

913-
keys_public_url = urlparse(self._build_url('keys', public=True))
914-
915-
filtered_params = {k: v for k, v in request_params.items() if v is not None}
916-
if filtered_params:
917-
keys_public_url = keys_public_url._replace(query=urlencode(filtered_params))
918-
919-
return urlunparse(keys_public_url)
894+
return self._build_public_url('keys', request_params)

tests/unit/test_url_generation.py

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
import json
44
from unittest import mock
55
from unittest.mock import Mock
6+
from urllib.parse import parse_qs, urlparse
67

78
import pytest
89

@@ -128,6 +129,40 @@ async def test_dataset_public_url_async(api_url: str, api_public_url: str | None
128129
)
129130

130131

132+
def test_dataset_public_url_normalizes_params_sync() -> None:
133+
"""Bool and list query params must be API-normalized (bool→true/false, list→comma-joined), not Python reprs."""
134+
client = ApifyClient(token='dummy-token', api_url='https://api.apify.com')
135+
dataset = client.dataset('someID')
136+
137+
mock_response = Mock()
138+
mock_response.json.return_value = json.loads(MOCKED_DATASET_RESPONSE)
139+
140+
with mock.patch.object(client._http_client, 'call', return_value=mock_response):
141+
public_url = dataset.create_items_public_url(clean=True, desc=False, fields=['title', 'url'])
142+
143+
query = parse_qs(urlparse(public_url).query)
144+
assert query['clean'] == ['true']
145+
assert query['desc'] == ['false']
146+
assert query['fields'] == ['title,url']
147+
148+
149+
async def test_dataset_public_url_normalizes_params_async() -> None:
150+
"""Bool and list query params must be API-normalized (bool→true/false, list→comma-joined), not Python reprs."""
151+
client = ApifyClientAsync(token='dummy-token', api_url='https://api.apify.com')
152+
dataset = client.dataset('someID')
153+
154+
mock_response = Mock()
155+
mock_response.json.return_value = json.loads(MOCKED_DATASET_RESPONSE)
156+
157+
with mock.patch.object(client._http_client, 'call', return_value=mock_response):
158+
public_url = await dataset.create_items_public_url(clean=True, desc=False, fields=['title', 'url'])
159+
160+
query = parse_qs(urlparse(public_url).query)
161+
assert query['clean'] == ['true']
162+
assert query['desc'] == ['false']
163+
assert query['fields'] == ['title,url']
164+
165+
131166
# ============================================================================
132167
# Key-value store URL generation tests
133168
# ============================================================================

0 commit comments

Comments
 (0)