Copilot commented on code in PR #13549:
URL: https://github.com/apache/apisix/pull/13549#discussion_r3410954540
##########
t/plugin/mcp-bridge.t:
##########
@@ -57,3 +57,40 @@ property "command" is required
property "command" validation failed: wrong type: expected string, got number
done
property "args" validation failed: wrong type: expected array, got string
+
+
+
+=== TEST 2: stderr content with JSON special characters keeps the notification
well-formed
+--- config
+ location /t {
+ content_by_lua_block {
+ local core = require("apisix.core")
+ local pipe = require("ngx.pipe")
+
+ -- a subprocess line that contains double quotes, a backslash and
the
+ -- "}}" sequence, all of which break naive string concatenation
+ local payload = [[Error: invalid input "x\y" }}]]
+
+ local proc = assert(pipe.spawn({"sh", "-c", "printf '%s\\n' '" ..
payload .. "' 1>&2"}))
+ proc:set_timeouts(nil, 1000, 1000)
+ local line = assert(proc:stderr_read_line())
Review Comment:
The test builds a `sh -c` command by concatenating `payload` directly into
the shell script. This is brittle (e.g., payload containing a single quote
would break the command) and makes the test harder to extend. Prefer passing
the payload as a positional argument to `sh -c` (via `$1`) so no shell escaping
is needed.
##########
apisix/plugins/mcp-bridge.lua:
##########
@@ -109,9 +109,13 @@ local function on_connect(conf, ctx)
local line, _
line, _, stderr_partial = proc:stderr_read_line()
if line then
- local ok, err = server.transport:send(
-
'{"jsonrpc":"2.0","method":"notifications/stderr","params":{"content":"'
- .. (stderr_partial and stderr_partial .. line or
line) .. '"}}')
+ local ok, err =
server.transport:send(core.json.encode({
+ jsonrpc = "2.0",
+ method = "notifications/stderr",
+ params = {
+ content = stderr_partial and stderr_partial ..
line or line,
+ },
+ }))
Review Comment:
`core.json.encode(...)` can return `nil, err` (e.g., if `content` contains
invalid UTF-8). In that case `server.transport:send(nil)` will eventually hit
`ngx_print(... .. data .. ...)` in the SSE transport and throw a runtime error
due to concatenating `nil`. Consider handling JSON encoding failure explicitly
and falling back to a safe placeholder string so the session doesn’t crash on
unencodable stderr output.
--
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]
