hanzhe-one opened a new pull request, #7926: URL: https://github.com/apache/shenyu/pull/7926
Fixes #7909. ## Problem A configured HTTP tool with a query argument reaches the upstream with the query twice: ```text http://127.0.0.1:18080/orders/status?id=A?id=A ``` `AgentHttpToolExecutor` put the whole route, query included, into `ShenyuContext.realUrl` (`context.setRealUrl(path)`), while the child request URI already carried the same encoded query. `RequestUrlUtils.buildRequestUri` appends the request query to `realUrl`, so the ordinary HTTP route concatenated both copies. ## Changes - `AgentHttpToolExecutor` now builds the child URI once and stores only `target.getRawPath()` in `realUrl` (and `path`), matching what the other context builders put there (`DefaultShenyuContextBuilder`, `DivideShenyuContextDecorator`). The encoded arguments stay on the child request URI, which is the single source of the query. - Added `shenyu-plugin-context-path` as a test-scope dependency so the context-path case can be exercised with the real plugin. ## Validation Regression tests in `AgentHttpToolExecutorTest` build the final upstream URI with the production `RequestUrlUtils.buildRequestUri`, the same call `URIPlugin` makes, instead of only asserting the child request URI: - `forwardsEncodedQueryArgumentsExactlyOnceThroughTheOrdinaryHttpRoute` — GET, `id=A&B` → `http://127.0.0.1:18080/orders/status?id=A%26B`. - `forwardsABodyArgumentWithoutInventingAQuery` — POST with `argsPosition=body` keeps the URI queryless. - `forwardsAQuerylessRouteWithoutAddingAQuestionMark` — no arguments, no trailing `?`. - `keepsTheContextPathRewriteWithoutDuplicatingTheQuery` — route `/api/orders/status` with a context-path rule `/api` still forwards `/orders/status?id=A%26B` after the real `ContextPathPlugin` rewrites `realUrl`. Failure before the fix (main code reverted, tests kept): ```text AgentHttpToolExecutorTest.forwardsEncodedQueryArgumentsExactlyOnceThroughTheOrdinaryHttpRoute java.lang.IllegalArgumentException: Invalid character '=' for QUERY_PARAM in "A%26B?id=A%26B" ``` Passes with the fix. Results: - `./mvnw test -pl shenyu-plugin/shenyu-plugin-agent-gateway`: 550 tests, 0 failures; 27 errors are all `java.io.IOException: Unable to establish loopback connection` / `SocketException: Invalid argument: connect` from tests that open a real local socket (`RequestScopedMcpClientTest`, `ManagedRemoteMcpCatalogTest`, one `AgentMcpOutputSchemaValidatorTest` case). The same failures reproduce on this machine with an unrelated 3-line `Selector.open()` program, so they are environmental and are left to CI. - `AgentHttpToolExecutorTest` 27/27, `AgentHttpToolRoutingTest` 13/13, `AgentGatewayPluginTest` 17/17. - `checkstyle:check`: 0 violations. `apache-rat:check`: 0 unapproved licenses. ## Blast radius `realUrl` is read by `RequestUrlUtils.buildRequestUri` and `getRewrittenRawPath`, `RewritePlugin`, and `AiRequestTransformerPlugin`. Tool calls set `path` and `realUrl` to the same value, so the "already rewritten" guard in the AI transformer and the selector/rule matching in `getRewrittenRawPath` keep seeing the route path, which is what they saw before the query was added to `realUrl` — the only observable difference is that the duplicated query is gone. ## Out of scope Remote MCP tools and the MCP Server plugin's own `ShenyuToolCallback` path already strip the query with `extractRawPath`; they are not touched here. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
