Skip to content

Commit b1768df

Browse files
jonpspriclaude
andcommitted
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>
1 parent 26a3047 commit b1768df

3 files changed

Lines changed: 92 additions & 24 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: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -4277,16 +4277,16 @@ async def invoke_tool(
42774277
rest_start_time = time.time()
42784278
try:
42794279
if method == "GET":
4280-
# For GET: merge query params from URL into payload and send as query string
4280+
# For GET: merge extracted URL query params into payload; everything sent as query string
42814281
if not tool_query_mapping:
42824282
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: query params stay in URL, payload goes to body
4286-
if not tool_query_mapping and query_params:
4287-
response = await asyncio.wait_for(self._http_client.request(method, final_url, json=payload, params=query_params, headers=headers), timeout=effective_timeout)
4288-
else:
4289-
response = await asyncio.wait_for(self._http_client.request(method, final_url, json=payload, headers=headers), timeout=effective_timeout)
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)
4289+
response = await asyncio.wait_for(self._http_client.request(method, final_url, json=payload, headers=headers), timeout=effective_timeout)
42904290
except (asyncio.TimeoutError, httpx.TimeoutException):
42914291
rest_elapsed_ms = (time.time() - rest_start_time) * 1000
42924292
structured_logger.log(

tests/unit/mcpgateway/services/test_tool_service.py

Lines changed: 78 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -2223,7 +2223,8 @@ async def test_invoke_tool_rest_post_with_path_query_and_body_params(self, tool_
22232223
- Path parameters (e.g., {user_id}) are substituted into the URL path
22242224
- Query parameters can also use templates (e.g., ?api_key={api_key})
22252225
- Static query parameters (e.g., ?version=v2) are preserved as-is
2226-
- Remaining payload goes to the JSON body
2226+
- Query parameters are merged into the JSON body for POST requests
2227+
- Remaining payload goes to the JSON body alongside merged query params
22272228
"""
22282229
mock_tool.integration_type = "REST"
22292230
mock_tool.request_type = "POST"
@@ -2234,10 +2235,10 @@ async def test_invoke_tool_rest_post_with_path_query_and_body_params(self, tool_
22342235

22352236
# Payload contains: path param (user_id), query param (api_key), and body params (title, content)
22362237
payload = {
2237-
"user_id": 456, # Will be substituted into URL path
2238-
"api_key": "secret123", # Will be substituted into query parameter
2239-
"title": "New Post", # Will go to JSON body
2240-
"content": "Hello World" # Will go to JSON body
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
22412242
}
22422243

22432244
setup_db_execute_mock(test_db, mock_tool, mock_global_config_obj)
@@ -2251,16 +2252,83 @@ async def test_invoke_tool_rest_post_with_path_query_and_body_params(self, tool_
22512252

22522253
await tool_service.invoke_tool(test_db, "test_tool", payload, request_headers=None)
22532254

2254-
# Verify the complete parameter separation:
2255+
# Verify parameter handling for POST:
22552256
# 1. Path parameter substituted: /users/456/posts
22562257
# 2. Query param template substituted: api_key=secret123
22572258
# 3. Static query param preserved: version=v2
2258-
# 4. Body params: title and content (user_id and api_key removed after substitution)
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)
22592261
tool_service._http_client.request.assert_called_once_with(
22602262
"POST",
2261-
"http://example.com/api/users/456/posts", # Path param substituted, query string removed
2262-
json={"title": "New Post", "content": "Hello World"}, # Body params only
2263-
params={"api_key": "secret123", "version": "v2"}, # Query params (both templated and static)
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"},
22642332
headers=mock_tool.headers,
22652333
)
22662334

0 commit comments

Comments
 (0)