Skip to content

Commit a93c8e9

Browse files
TissandierEmmanuel Tissandierjonpspriclaude
authored
fix(api): separate query params from body payload in REST tool POST requests (#3720)
* improve post for rest tools Signed-off-by: Emmanuel Tissandier <Emmanuel.Tissandier@fr.ibm.com> * test on new may to handle query parameters for POST Signed-off-by: Emmanuel Tissandier <Emmanuel.Tissandier@fr.ibm.com> * clean up Signed-off-by: Emmanuel Tissandier <Emmanuel.Tissandier@fr.ibm.com> * fix(tools): preserve backward-compatible query param handling for REST tools Move query param merging into method-specific branches (GET vs non-GET) while keeping the same runtime behavior as main. Add tests for GET with static query params, PUT with query params, and POST with combined path/query/body params. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Signed-off-by: Jonathan Springer <jps@s390x.com> --------- Signed-off-by: Emmanuel Tissandier <Emmanuel.Tissandier@fr.ibm.com> Signed-off-by: Jonathan Springer <jps@s390x.com> Co-authored-by: Emmanuel Tissandier <Emmanuel.Tissandier@fr.ibm.com> Co-authored-by: Jonathan Springer <jps@s390x.com> Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1 parent a2aa82a commit a93c8e9

3 files changed

Lines changed: 135 additions & 14 deletions

File tree

‎.secrets.baseline‎

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@
33
"files": "^.secrets.baseline|package-lock.json|Cargo.lock|scripts/sign_image.sh|scripts/zap|sonar-project.properties|uv.lock|^.secrets.baseline$",
44
"lines": null
55
},
6-
"generated_at": "2026-04-13T06:40:07Z",
6+
"generated_at": "2026-04-13T09:59:07Z",
77
"plugins_used": [
88
{
99
"name": "AWSKeyDetector"
@@ -9714,55 +9714,55 @@
97149714
"hashed_secret": "9fb7fe1217aed442b04c0f5e43b5d5a7d3287097",
97159715
"is_secret": false,
97169716
"is_verified": false,
9717-
"line_number": 2877,
9717+
"line_number": 2994,
97189718
"type": "Secret Keyword",
97199719
"verified_result": null
97209720
},
97219721
{
97229722
"hashed_secret": "72cb70dbbafe97e5ea13ad88acd65d08389439b0",
97239723
"is_secret": false,
97249724
"is_verified": false,
9725-
"line_number": 3505,
9725+
"line_number": 3622,
97269726
"type": "Secret Keyword",
97279727
"verified_result": null
97289728
},
97299729
{
97309730
"hashed_secret": "ee977806d7286510da8b9a7492ba58e2484c0ecc",
97319731
"is_secret": false,
97329732
"is_verified": false,
9733-
"line_number": 6259,
9733+
"line_number": 6376,
97349734
"type": "Secret Keyword",
97359735
"verified_result": null
97369736
},
97379737
{
97389738
"hashed_secret": "f2e7745f43b0ef0e2c2faf61d6c6a28be2965750",
97399739
"is_secret": false,
97409740
"is_verified": false,
9741-
"line_number": 6751,
9741+
"line_number": 6868,
97429742
"type": "Secret Keyword",
97439743
"verified_result": null
97449744
},
97459745
{
97469746
"hashed_secret": "4a249743d4d2241bd2ae085b4fe654d089488295",
97479747
"is_secret": false,
97489748
"is_verified": false,
9749-
"line_number": 8098,
9749+
"line_number": 8215,
97509750
"type": "Secret Keyword",
97519751
"verified_result": null
97529752
},
97539753
{
97549754
"hashed_secret": "0c8d051d3c7eada5d31b53d9936fce6bcc232ae2",
97559755
"is_secret": false,
97569756
"is_verified": false,
9757-
"line_number": 8240,
9757+
"line_number": 8357,
97589758
"type": "Secret Keyword",
97599759
"verified_result": null
97609760
},
97619761
{
97629762
"hashed_secret": "f2b14f68eb995facb3a1c35287b778d5bd785511",
97639763
"is_secret": false,
97649764
"is_verified": false,
9765-
"line_number": 8616,
9765+
"line_number": 8733,
97669766
"type": "Secret Keyword",
97679767
"verified_result": null
97689768
}

‎mcpgateway/services/tool_service.py‎

Lines changed: 10 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -4232,10 +4232,10 @@ async def invoke_tool(
42324232
# Build the payload based on integration type
42334233
payload = arguments.copy()
42344234

4235-
# Handle URL path parameter substitution (using local variable)
4235+
# Handle URL path and query parameter substitution (using local variable)
42364236
final_url = tool_url
42374237
if "{" in tool_url and "}" in tool_url:
4238-
# Extract path parameters from URL template and arguments
4238+
# Extract ALL parameters (path and query) from URL template
42394239
url_params = re.findall(r"\{(\w+)\}", tool_url)
42404240
url_substitutions = {}
42414241

@@ -4246,7 +4246,7 @@ async def invoke_tool(
42464246
else:
42474247
raise ToolInvocationError(f"Required URL parameter '{param}' not found in arguments")
42484248

4249-
# --- Extract query params from URL ---
4249+
# --- Extract query params from URL (after substitution) ---
42504250
parsed = urlparse(final_url)
42514251
final_url = f"{parsed.scheme}://{parsed.netloc}{parsed.path}"
42524252

@@ -4260,9 +4260,6 @@ async def invoke_tool(
42604260
for qk, qv in payload.items():
42614261
if isinstance(qv, (dict, list)):
42624262
raise ToolInvocationError(f"Tool '{name}': query_mapping produced non-scalar value for parameter '{qk}'")
4263-
else:
4264-
# No query mapping — merge URL query params into the payload (query params override on collision)
4265-
payload.update(query_params)
42664263

42674264
# Headers are mapped from the original arguments (not the path-param-reduced payload)
42684265
# to preserve all available data for header injection.
@@ -4280,8 +4277,15 @@ async def invoke_tool(
42804277
rest_start_time = time.time()
42814278
try:
42824279
if method == "GET":
4280+
# For GET: merge extracted URL query params into payload; everything sent as query string
4281+
if not tool_query_mapping:
4282+
payload.update(query_params)
42834283
response = await asyncio.wait_for(self._http_client.get(final_url, params=payload, headers=headers), timeout=effective_timeout)
42844284
else:
4285+
# For POST/PUT/PATCH/DELETE: merge query params into the JSON body
4286+
# (preserves backward compatibility with existing tool configurations)
4287+
if not tool_query_mapping:
4288+
payload.update(query_params)
42854289
response = await asyncio.wait_for(self._http_client.request(method, final_url, json=payload, headers=headers), timeout=effective_timeout)
42864290
except (asyncio.TimeoutError, httpx.TimeoutException):
42874291
rest_elapsed_ms = (time.time() - rest_start_time) * 1000

‎tests/unit/mcpgateway/services/test_tool_service.py‎

Lines changed: 117 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2215,6 +2215,123 @@ async def test_invoke_tool_rest_parameter_substitution(self, tool_service, mock_
22152215
headers=mock_tool.headers,
22162216
)
22172217

2218+
@pytest.mark.asyncio
2219+
async def test_invoke_tool_rest_post_with_path_query_and_body_params(self, tool_service, mock_tool, mock_global_config_obj, test_db):
2220+
"""Test POST request with path parameters, query parameters (with templates), and body parameters.
2221+
2222+
This test demonstrates the complete parameter handling:
2223+
- Path parameters (e.g., {user_id}) are substituted into the URL path
2224+
- Query parameters can also use templates (e.g., ?api_key={api_key})
2225+
- Static query parameters (e.g., ?version=v2) are preserved as-is
2226+
- Query parameters are merged into the JSON body for POST requests
2227+
- Remaining payload goes to the JSON body alongside merged query params
2228+
"""
2229+
mock_tool.integration_type = "REST"
2230+
mock_tool.request_type = "POST"
2231+
mock_tool.jsonpath_filter = ""
2232+
mock_tool.auth_value = None
2233+
# URL with path parameters AND templated query parameters
2234+
mock_tool.url = "http://example.com/api/users/{user_id}/posts?api_key={api_key}&version=v2"
2235+
2236+
# Payload contains: path param (user_id), query param (api_key), and body params (title, content)
2237+
payload = {
2238+
"user_id": 456, # Will be substituted into URL path
2239+
"api_key": "secret123", # Template in query string portion of URL; substituted then extracted as query param
2240+
"title": "New Post", # Will go to JSON body
2241+
"content": "Hello World", # Will go to JSON body
2242+
}
2243+
2244+
setup_db_execute_mock(test_db, mock_tool, mock_global_config_obj)
2245+
2246+
mock_response = Mock()
2247+
mock_response.raise_for_status = Mock()
2248+
mock_response.status_code = 200
2249+
mock_response.json = Mock(return_value={"id": 789, "status": "created"})
2250+
2251+
tool_service._http_client.request = AsyncMock(return_value=mock_response)
2252+
2253+
await tool_service.invoke_tool(test_db, "test_tool", payload, request_headers=None)
2254+
2255+
# Verify parameter handling for POST:
2256+
# 1. Path parameter substituted: /users/456/posts
2257+
# 2. Query param template substituted: api_key=secret123
2258+
# 3. Static query param preserved: version=v2
2259+
# 4. Query params merged into JSON body (backward-compatible behavior for POST)
2260+
# 5. Body params: title and content (user_id and api_key removed after path/query substitution)
2261+
tool_service._http_client.request.assert_called_once_with(
2262+
"POST",
2263+
"http://example.com/api/users/456/posts", # Path param substituted, query string stripped
2264+
json={"title": "New Post", "content": "Hello World", "api_key": "secret123", "version": "v2"}, # Body + merged query params
2265+
headers=mock_tool.headers,
2266+
)
2267+
2268+
@pytest.mark.asyncio
2269+
async def test_invoke_tool_rest_get_with_static_query_params_no_mapping(self, tool_service, mock_tool, mock_global_config_obj, test_db):
2270+
"""Test GET request with static URL query params and no query_mapping.
2271+
2272+
Verifies that query params extracted from the URL are merged into the
2273+
payload and sent together via params= on the GET request.
2274+
"""
2275+
mock_tool.integration_type = "REST"
2276+
mock_tool.request_type = "GET"
2277+
mock_tool.jsonpath_filter = ""
2278+
mock_tool.auth_value = None
2279+
mock_tool.url = "http://example.com/api/search?version=v2&format=json"
2280+
2281+
payload = {"q": "hello"}
2282+
2283+
setup_db_execute_mock(test_db, mock_tool, mock_global_config_obj)
2284+
2285+
mock_response = Mock()
2286+
mock_response.raise_for_status = Mock()
2287+
mock_response.status_code = 200
2288+
mock_response.json = Mock(return_value={"results": []})
2289+
2290+
tool_service._http_client.get = AsyncMock(return_value=mock_response)
2291+
2292+
await tool_service.invoke_tool(test_db, "test_tool", payload, request_headers=None)
2293+
2294+
# URL query params (version, format) are merged into payload alongside the user-provided "q"
2295+
tool_service._http_client.get.assert_called_once_with(
2296+
"http://example.com/api/search",
2297+
params={"q": "hello", "version": "v2", "format": "json"},
2298+
headers=mock_tool.headers,
2299+
)
2300+
2301+
@pytest.mark.asyncio
2302+
async def test_invoke_tool_rest_put_with_query_params(self, tool_service, mock_tool, mock_global_config_obj, test_db):
2303+
"""Test PUT request with URL query params merges them into the JSON body.
2304+
2305+
Verifies that non-GET methods other than POST (e.g. PUT) also merge
2306+
URL query params into the JSON body for backward compatibility.
2307+
"""
2308+
mock_tool.integration_type = "REST"
2309+
mock_tool.request_type = "PUT"
2310+
mock_tool.jsonpath_filter = ""
2311+
mock_tool.auth_value = None
2312+
mock_tool.url = "http://example.com/api/items/1?version=v2"
2313+
2314+
payload = {"name": "updated"}
2315+
2316+
setup_db_execute_mock(test_db, mock_tool, mock_global_config_obj)
2317+
2318+
mock_response = Mock()
2319+
mock_response.raise_for_status = Mock()
2320+
mock_response.status_code = 200
2321+
mock_response.json = Mock(return_value={"status": "ok"})
2322+
2323+
tool_service._http_client.request = AsyncMock(return_value=mock_response)
2324+
2325+
await tool_service.invoke_tool(test_db, "test_tool", payload, request_headers=None)
2326+
2327+
# Query params merged into JSON body, same as POST
2328+
tool_service._http_client.request.assert_called_once_with(
2329+
"PUT",
2330+
"http://example.com/api/items/1",
2331+
json={"name": "updated", "version": "v2"},
2332+
headers=mock_tool.headers,
2333+
)
2334+
22182335
@pytest.mark.asyncio
22192336
async def test_invoke_tool_rest_jq_filter_error_returns_error(self, tool_service, mock_tool, mock_global_config_obj, test_db):
22202337
"""Test REST tool invocation marks result as error when jq filter returns TextContent error."""

0 commit comments

Comments
 (0)