Skip to content

Commit 3464f80

Browse files
fix!: URL-encode path parameters in generated endpoints (#1349)
## Summary Path parameters containing reserved characters were being inserted directly into URLs without encoding, causing malformed requests and potential security issues. ## Problem When path parameters contain special characters like: - Slashes (`/`) - Question marks (`?`) - Ampersands (`&`) - Hash/fragments (`#`) - Spaces These were not being URL-encoded, leading to: - Malformed URLs that break routing - Security vulnerabilities where parameter values could inject path segments - Non-compliance with RFC 3986 ## Solution - Import `urllib.parse.quote` in the endpoint template - Wrap all path parameters with `quote(str(...), safe="")` to ensure proper percent-encoding - Add comprehensive tests for various special characters in path parameters ## Changes - Updated `openapi_python_client/templates/endpoint_module.py.jinja` to add URL encoding - Added `end_to_end_tests/functional_tests/generated_code_execution/test_path_parameters.py` with tests - Regenerated all golden records to reflect the encoding changes ## Test plan - [x] Added test for normal alphanumeric path parameters (baseline) - [x] Added tests for path parameters with reserved characters (/, ?, &, #) - [x] Added tests for path parameters with spaces - [x] Regenerated golden records with `pdm regen` - [x] Verified generated code properly encodes all special characters ## Example **Before:** ```python "url": f"/items/{item_id}/details/{detail_id}" # With item_id="item/with/slashes" becomes: # "/items/item/with/slashes/details/..." (broken!) ``` **After:** ```python "url": "/items/{item_id}/details/{detail_id}".format( item_id=quote(str(item_id), safe=""), detail_id=quote(str(detail_id), safe=""), ) # With item_id="item/with/slashes" becomes: # "/items/item%2Fwith%2Fslashes/details/..." (correct!) ```
1 parent c41e22a commit 3464f80

File tree

9 files changed

+184
-8
lines changed

9 files changed

+184
-8
lines changed
Lines changed: 151 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,151 @@
1+
from unittest.mock import MagicMock
2+
3+
import httpx
4+
import pytest
5+
6+
from end_to_end_tests.functional_tests.helpers import (
7+
with_generated_client_fixture,
8+
with_generated_code_import,
9+
)
10+
11+
12+
@with_generated_client_fixture(
13+
"""
14+
paths:
15+
"/items/{item_id}/details/{detail_id}":
16+
get:
17+
operationId: getItemDetail
18+
parameters:
19+
- name: item_id
20+
in: path
21+
required: true
22+
schema:
23+
type: string
24+
- name: detail_id
25+
in: path
26+
required: true
27+
schema:
28+
type: string
29+
responses:
30+
"200":
31+
description: Success
32+
content:
33+
application/json:
34+
schema:
35+
type: object
36+
properties:
37+
id:
38+
type: string
39+
""")
40+
@with_generated_code_import(".api.default.get_item_detail.sync_detailed")
41+
@with_generated_code_import(".client.Client")
42+
class TestPathParameterEncoding:
43+
"""Test that path parameters are properly URL-encoded"""
44+
45+
def test_path_params_with_normal_chars_work(self, sync_detailed, Client):
46+
"""Test that normal alphanumeric path parameters still work correctly"""
47+
mock_httpx_client = MagicMock(spec=httpx.Client)
48+
mock_response = MagicMock(spec=httpx.Response)
49+
mock_response.status_code = 200
50+
mock_response.json.return_value = {"id": "test"}
51+
mock_response.content = b'{"id": "test"}'
52+
mock_response.headers = {}
53+
mock_httpx_client.request.return_value = mock_response
54+
55+
client = Client(base_url="https://api.example.com")
56+
client.set_httpx_client(mock_httpx_client)
57+
58+
sync_detailed(
59+
item_id="item123",
60+
detail_id="detail456",
61+
client=client,
62+
)
63+
64+
mock_httpx_client.request.assert_called_once()
65+
call_kwargs = mock_httpx_client.request.call_args[1]
66+
67+
# Normal characters should remain unchanged
68+
expected_url = "/items/item123/details/detail456"
69+
assert call_kwargs["url"] == expected_url
70+
71+
def test_path_params_with_reserved_chars_are_encoded(self, sync_detailed, Client):
72+
"""Test that path parameters with reserved characters are properly URL-encoded"""
73+
# Create a mock httpx client
74+
mock_httpx_client = MagicMock(spec=httpx.Client)
75+
mock_response = MagicMock(spec=httpx.Response)
76+
mock_response.status_code = 200
77+
mock_response.json.return_value = {"id": "test"}
78+
mock_response.content = b'{"id": "test"}'
79+
mock_response.headers = {}
80+
mock_httpx_client.request.return_value = mock_response
81+
82+
# Create a client with the mock httpx client
83+
client = Client(base_url="https://api.example.com")
84+
client.set_httpx_client(mock_httpx_client)
85+
86+
# Call the endpoint with path parameters containing reserved characters
87+
sync_detailed(
88+
item_id="item/with/slashes",
89+
detail_id="detail?with=query&chars",
90+
client=client,
91+
)
92+
93+
# Verify the request was made with properly encoded URL
94+
mock_httpx_client.request.assert_called_once()
95+
call_kwargs = mock_httpx_client.request.call_args[1]
96+
97+
# The URL should have encoded slashes and query characters
98+
expected_url = "/items/item%2Fwith%2Fslashes/details/detail%3Fwith%3Dquery%26chars"
99+
assert call_kwargs["url"] == expected_url
100+
101+
def test_path_params_with_spaces_are_encoded(self, sync_detailed, Client):
102+
"""Test that path parameters with spaces are properly URL-encoded"""
103+
mock_httpx_client = MagicMock(spec=httpx.Client)
104+
mock_response = MagicMock(spec=httpx.Response)
105+
mock_response.status_code = 200
106+
mock_response.json.return_value = {"id": "test"}
107+
mock_response.content = b'{"id": "test"}'
108+
mock_response.headers = {}
109+
mock_httpx_client.request.return_value = mock_response
110+
111+
client = Client(base_url="https://api.example.com")
112+
client.set_httpx_client(mock_httpx_client)
113+
114+
sync_detailed(
115+
item_id="item with spaces",
116+
detail_id="detail with spaces",
117+
client=client,
118+
)
119+
120+
mock_httpx_client.request.assert_called_once()
121+
call_kwargs = mock_httpx_client.request.call_args[1]
122+
123+
# Spaces should be encoded as %20
124+
expected_url = "/items/item%20with%20spaces/details/detail%20with%20spaces"
125+
assert call_kwargs["url"] == expected_url
126+
127+
def test_path_params_with_hash_are_encoded(self, sync_detailed, Client):
128+
"""Test that path parameters with hash/fragment characters are properly URL-encoded"""
129+
mock_httpx_client = MagicMock(spec=httpx.Client)
130+
mock_response = MagicMock(spec=httpx.Response)
131+
mock_response.status_code = 200
132+
mock_response.json.return_value = {"id": "test"}
133+
mock_response.content = b'{"id": "test"}'
134+
mock_response.headers = {}
135+
mock_httpx_client.request.return_value = mock_response
136+
137+
client = Client(base_url="https://api.example.com")
138+
client.set_httpx_client(mock_httpx_client)
139+
140+
sync_detailed(
141+
item_id="item#1",
142+
detail_id="detail#id",
143+
client=client,
144+
)
145+
146+
mock_httpx_client.request.assert_called_once()
147+
call_kwargs = mock_httpx_client.request.call_args[1]
148+
149+
# Hash should be encoded as %23
150+
expected_url = "/items/item%231/details/detail%23id"
151+
assert call_kwargs["url"] == expected_url

end_to_end_tests/golden-record/my_test_api_client/api/naming/hyphen_in_path.py

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
from http import HTTPStatus
22
from typing import Any
3+
from urllib.parse import quote
34

45
import httpx
56

@@ -13,7 +14,9 @@ def _get_kwargs(
1314
) -> dict[str, Any]:
1415
_kwargs: dict[str, Any] = {
1516
"method": "get",
16-
"url": f"/naming/{hyphen_in_path}",
17+
"url": "/naming/{hyphen_in_path}".format(
18+
hyphen_in_path=quote(str(hyphen_in_path), safe=""),
19+
),
1720
}
1821

1922
return _kwargs

end_to_end_tests/golden-record/my_test_api_client/api/parameter_references/get_parameter_references_path_param.py

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
from http import HTTPStatus
22
from typing import Any
3+
from urllib.parse import quote
34

45
import httpx
56

@@ -34,7 +35,9 @@ def _get_kwargs(
3435

3536
_kwargs: dict[str, Any] = {
3637
"method": "get",
37-
"url": f"/parameter-references/{path_param}",
38+
"url": "/parameter-references/{path_param}".format(
39+
path_param=quote(str(path_param), safe=""),
40+
),
3841
"params": params,
3942
"cookies": cookies,
4043
}

end_to_end_tests/golden-record/my_test_api_client/api/parameters/delete_common_parameters_overriding_param.py

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
from http import HTTPStatus
22
from typing import Any
3+
from urllib.parse import quote
34

45
import httpx
56

@@ -21,7 +22,9 @@ def _get_kwargs(
2122

2223
_kwargs: dict[str, Any] = {
2324
"method": "delete",
24-
"url": f"/common_parameters_overriding/{param_path}",
25+
"url": "/common_parameters_overriding/{param_path}".format(
26+
param_path=quote(str(param_path), safe=""),
27+
),
2528
"params": params,
2629
}
2730

end_to_end_tests/golden-record/my_test_api_client/api/parameters/get_common_parameters_overriding_param.py

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
from http import HTTPStatus
22
from typing import Any
3+
from urllib.parse import quote
34

45
import httpx
56

@@ -21,7 +22,9 @@ def _get_kwargs(
2122

2223
_kwargs: dict[str, Any] = {
2324
"method": "get",
24-
"url": f"/common_parameters_overriding/{param_path}",
25+
"url": "/common_parameters_overriding/{param_path}".format(
26+
param_path=quote(str(param_path), safe=""),
27+
),
2528
"params": params,
2629
}
2730

end_to_end_tests/golden-record/my_test_api_client/api/parameters/get_same_name_multiple_locations_param.py

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
from http import HTTPStatus
22
from typing import Any
3+
from urllib.parse import quote
34

45
import httpx
56

@@ -31,7 +32,9 @@ def _get_kwargs(
3132

3233
_kwargs: dict[str, Any] = {
3334
"method": "get",
34-
"url": f"/same-name-multiple-locations/{param_path}",
35+
"url": "/same-name-multiple-locations/{param_path}".format(
36+
param_path=quote(str(param_path), safe=""),
37+
),
3538
"params": params,
3639
"cookies": cookies,
3740
}

end_to_end_tests/golden-record/my_test_api_client/api/parameters/multiple_path_parameters.py

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
from http import HTTPStatus
22
from typing import Any
3+
from urllib.parse import quote
34

45
import httpx
56

@@ -16,7 +17,12 @@ def _get_kwargs(
1617
) -> dict[str, Any]:
1718
_kwargs: dict[str, Any] = {
1819
"method": "get",
19-
"url": f"/multiple-path-parameters/{param4}/something/{param2}/{param1}/{param3}",
20+
"url": "/multiple-path-parameters/{param4}/something/{param2}/{param1}/{param3}".format(
21+
param4=quote(str(param4), safe=""),
22+
param2=quote(str(param2), safe=""),
23+
param1=quote(str(param1), safe=""),
24+
param3=quote(str(param3), safe=""),
25+
),
2026
}
2127

2228
return _kwargs

end_to_end_tests/test-3-1-golden-record/test_3_1_features_client/api/const/post_const_path.py

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
from http import HTTPStatus
22
from typing import Any, Literal, cast
3+
from urllib.parse import quote
34

45
import httpx
56

@@ -28,7 +29,9 @@ def _get_kwargs(
2829

2930
_kwargs: dict[str, Any] = {
3031
"method": "post",
31-
"url": f"/const/{path}",
32+
"url": "/const/{path}".format(
33+
path=quote(str(path), safe=""),
34+
),
3235
"params": params,
3336
}
3437

openapi_python_client/templates/endpoint_module.py.jinja

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
from http import HTTPStatus
22
from typing import Any, cast
3+
from urllib.parse import quote
34

45
import httpx
56

@@ -31,7 +32,7 @@ def _get_kwargs(
3132
{% if endpoint.path_parameters %}
3233
"url": "{{ endpoint.path }}".format(
3334
{%- for parameter in endpoint.path_parameters -%}
34-
{{parameter.python_name}}={{parameter.python_name}},
35+
{{parameter.python_name}}=quote(str({{parameter.python_name}}), safe=""),
3536
{%- endfor -%}
3637
),
3738
{% else %}

0 commit comments

Comments
 (0)