Skip to content

fix: keep serving after malformed input instead of dying on decode errors - #2

Merged
jkyberneees merged 1 commit into
mainfrom
fix/malformed-input-kills-server
Aug 21, 2026
Merged

fix: keep serving after malformed input instead of dying on decode errors#2
jkyberneees merged 1 commit into
mainfrom
fix/malformed-input-kills-server

Conversation

@jkyberneees

Copy link
Copy Markdown
Contributor

Problem

RunWithIO decoded stdin with json.Decoder and returned a fatal decode error on any line it could not decode into JSONRPCRequest. That includes well-formed JSON whose field types do not match the struct:

{"jsonrpc":1,"id":2,"method":"tools/call","params":{"name":"ping"}}

A single such line — crafted by untrusted content or produced by a buggy host — kills the dispatch loop mid-session. Downstream, a host that frames requests through an io.Pipe (as odek-style adapters do) then wedges permanently: the offending request is never answered, every subsequent valid request is stranded behind a blocked writer, and the server never observes EOF.

Reproduced against v1.2.0: after the bad line, no response of any kind is emitted and RunWithIO never returns even after stdin closes.

Per JSON-RPC 2.0 this class of input must be answered in-band (-32700/-32600) and the connection kept alive.

Fix

The dispatch loop now reads newline-delimited messages directly (the MCP stdio framing: one message per line) over a reusable buffer:

  • broken JSON → -32700 Parse error, id null; loop keeps serving
  • well-formed JSON that is not a Request object → -32600 Invalid Request, id null; loop keeps serving
  • tolerates CRLF endings, blank separator lines, and a final unterminated line; still no message-size limit
  • RunWithIO returns only on clean EOF (nil) or a read/write failure

Framing note: previously multiple JSON values concatenated on one line were processed as separate decoder values. That is a protocol violation under the stdio transport ("messages MUST NOT contain embedded newlines", one per line); such lines are now rejected with -32700. All mainstream clients (Claude Desktop, Cursor, odek) already send strict NDJSON.

Testing

  • TestDecodeError asserted the old kill-the-server contract; replaced with five keep-serving regression tests: the wedge repro (type-mismatched line then valid request), non-object JSON, multiple bad lines + CRLF + blank lines, and final-line-without-newline EOF handling.
  • Full suite green including -race; E2E subprocess test passes.
  • Benchmarks unchanged in latency (~2 KB more buffered I/O for the reader buffer, 3–4 fewer allocs/op):
benchmark before after
Initialize 2005 ns/op 1993 ns/op
ToolsList 8068 ns/op 8153 ns/op
ToolsCall 3077 ns/op 3121 ns/op

Verified end-to-end with a downstream MCP adapter that previously deadlocked on the crafted line: it now receives -32600 for the bad line, a normal response for the follow-up request, and shuts down cleanly on EOF.

RunWithIO decoded stdin with json.Decoder and returned a fatal "decode
error" on any line that could not be decoded into JSONRPCRequest —
including well-formed JSON whose field types do not match, e.g.
{"jsonrpc":1,"id":2,"method":"ping"}. A single crafted or buggy line
could therefore kill the server mid-session: the offending request went
unanswered, and pipe-based hosts could wedge forever on a blocked
writer with no further responses of any kind.

The dispatch loop is now newline-delimited (the MCP stdio framing, one
message per line) over a reusable read buffer:

- broken JSON gets -32700 (Parse error); the loop keeps serving
- well-formed JSON that is not a Request object gets -32600 (Invalid
  Request) with id null; the loop keeps serving
- CRLF endings, blank separator lines, and a final unterminated line
  are tolerated; there is still no message-size limit
- RunWithIO returns only on clean EOF or a read/write failure

Note on framing strictness: multiple JSON values concatenated on one
line (or pretty-printed multi-line messages) were previously accepted
as separate decoder values; they are protocol violations under the
MCP stdio transport and are now rejected with -32700 per line.

TestDecodeError asserted the old kill-the-server contract; it is
replaced by keep-serving regression tests covering the wedge scenario,
non-object JSON, multiple bad lines, CRLF handling, and final-line EOF.
Latency is unchanged; allocations drop slightly (+2 KB buffered I/O
for the reusable reader).
@jkyberneees
jkyberneees merged commit c85672a into main Aug 21, 2026
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant