Skip to content

Read & write JSON-RPC frames as bytes; require binary streams - #44

Open
ramfjord wants to merge 3 commits into
cxxxr:masterfrom
ramfjord:fix/byte-counted-content-length-binary
Open

ramfjord wants to merge 3 commits into
cxxxr:masterfrom
ramfjord:fix/byte-counted-content-length-binary

Conversation

@ramfjord

@ramfjord ramfjord commented May 4, 2026

Copy link
Copy Markdown

PR: Read & write JSON-RPC frames as bytes; require binary streams

Branch: ramfjord:fix/byte-counted-content-length-binarycxxxr:master

The bug

Content-Length in JSON-RPC's HTTP-style framing is bytes (LSP makes
this explicit). The previous read-message used
(make-string LENGTH) + read-sequence on a character stream — that
reads LENGTH characters. For pure-ASCII bodies the two are identical;
for any multi-byte UTF-8 in the body, the reader takes too few bytes
from the stream. The leftover bytes corrupt the next message's header
parse and the read loop hangs. write-message had the symmetric
character-vs-byte mismatch in Content-Length.

I hit this building a swank-backed LSP server: every Lisp source file
with a single em-dash in a comment desynced the wire after didOpen,
hanging gd and K until the LSP was restarted. Same root cause
would bite any LSP-shaped consumer of jsonrpc/transport/stdio.

The TCP transport never had this bug because it already operated on a
binary socket-stream and had its own byte-aware read-headers inline.
This PR consolidates around that working pattern.

The change (3 commits)

Three commits, each independently green and reviewable.

1. Read & write JSON-RPC frames as bytes; require binary streams

The actual bug fix.

  • request-response.lisp: read-headers, read-message, and
    write-message now operate on binary streams. Headers decode as
    ASCII (LSP headers are pure ASCII); body decodes as UTF-8 via
    trivial-utf-8.
  • transport/tcp.lisp: socket-stream is already binary, so the shared
    read-message / write-message are exactly what tcp needs. tcp's
    bespoke inline header parser (~70 lines) and body-reading deleted.
  • transport/stdio.lisp: :input / :output now default to binary
    stdio streams via sb-sys:make-fd-stream. Non-SBCL implementations
    must pass binary streams explicitly (errors helpfully on missing
    initform).
  • transport/local-domain-socket.lisp: noted that
    socket-make-stream there still defaults to a character stream.
    ASCII bodies behave as before; multi-byte safety needs
    :element-type '(unsigned-byte 8) at the call site (left as
    follow-up since this transport wasn't exercised in my report).

2. Hoist 1-line transport methods to defaults on the generic

Pure cleanup, no behavior change. stdio, tcp, and local-domain-socket
all had identical 1-line send-message-using-transport methods and
near-identical receive-message-using-transport methods. Move both
to default :method clauses on the generic in
transport/interface.lisp. Websocket keeps its own methods (different
framing). Three transports now inherit the default; -19 LoC.

3. Use flexi-streams to wrap the binary stream as bivalent

flexi-streams provides a stream wrap exposing both character and
binary I/O over one buffered binary stream. With it, read-message
creates one wrap and uses it for both: read-line for ASCII headers,
read-sequence of bytes for the UTF-8 body. The byte-loop helper and
custom eof condition go away; stdlib end-of-file replaces them.

Adds flexi-streams as a direct dep — widely used across the CL
ecosystem (drakma, hunchentoot, websocket-driver) but not currently a
transitive dep of jsonrpc.

Tests

tests/request-response.lisp gains read-message-test with three
cases (each frames bytes inline via flexi-streams
make-in-memory-input-stream + a local flet for the
Content-Length:-prefixed wire format):

  • ASCII body parses (regression guard).
  • Multi-byte UTF-8 body parses with byte-counted Content-Length.
  • Two back-to-back messages where the first has multi-byte content —
    the second's method comes through intact (proves no desync).

tests/transport/stdio.lisp updated to pass binary fd-streams in the
existing pipe-based stdio-server test.

All 4 upstream test suites pass on every commit:
request-response, tcp-server, stdio-server, websocket-server.

Stats

  • 7 files changed, +92 / -141 (net -49 LoC)
  • One new direct dep: flexi-streams (commit 3).

Happy to split things differently or rework anything — let me know
what you'd prefer.

Thomas Ramfjord added 3 commits May 3, 2026 22:35
Content-Length in JSON-RPC's HTTP-style framing is bytes (LSP makes
this explicit). The previous read-message used (make-string LENGTH)
+ read-sequence on a character stream, undercounting whenever the
body had multi-byte UTF-8 -- the leftover bytes corrupted the next
header parse and the read loop hung. write-message had the
symmetric bug: (length json) is character count, not byte count.

This change consolidates around binary streams throughout:

- request-response.lisp: read-message / write-message and the
  supporting read-headers now operate on binary streams. Headers
  are decoded as ASCII (LSP headers are pure ASCII); bodies as
  UTF-8 via trivial-utf-8. A small read-ascii-line helper does
  scan-until-LF on a binary stream so read-headers stays
  structurally identical to the previous character-stream version
  (loop until empty line, parse each line) instead of becoming a
  state machine.

- transport/tcp.lisp: socket-stream is already binary, so the
  shared read-message / write-message are exactly what tcp needs.
  The previous bespoke read-headers and inline body-reading logic
  (~70 lines) deleted; tcp's send/receive methods now delegate.

- transport/stdio.lisp: stdio-transport's :input/:output now
  default to binary stdio streams (SBCL via sb-sys:make-fd-stream;
  non-SBCL must pass :input/:output binary streams explicitly).

- transport/local-domain-socket.lisp: unchanged behavior. Note
  added: socket-make-stream there still defaults to a character
  stream; for multi-byte safety pass :element-type
  '(unsigned-byte 8) at the call site (this transport wasn't
  exercised in the original bug report).

Tests: tests/request-response.lisp gains read-message-test with
three cases plus a small frame-stream helper (uses flexi-streams,
already a transitive dep, for in-memory binary input streams):
- ASCII body parses (regression guard).
- Multi-byte UTF-8 body parses with byte-counted Content-Length.
- Two back-to-back messages where the first has multi-byte content
  -- second's method comes through intact (proves no desync).
- tests/transport/stdio.lisp updated to pass binary fd-streams in
  the existing pipe-based stdio-server test.

All upstream test suites (request-response, tcp-server,
stdio-server, websocket-server) pass.

Real-world impact: this hung an LSP server built on
jsonrpc/transport/stdio -- every Lisp source file with one em-dash
in a comment desynced the wire after didOpen.
stdio, tcp, and local-domain-socket all had identical 1-line
SEND-MESSAGE-USING-TRANSPORT methods that called WRITE-MESSAGE on the
connection's stream, and near-identical RECEIVE-MESSAGE-USING-TRANSPORT
methods (tcp's only addition was an EOF handler that returns NIL).

Move both to default :method clauses on the generic. WEBSOCKET-TRANSPORT
keeps its own methods (different framing).

No behavioral change. All 4 upstream test suites pass.
Optional follow-up to the binary-framing fix. Discard this commit if
adding flexi-streams as a direct dep is unwelcome -- the previous
two commits stand on their own.

flexi-streams provides a stream wrap that exposes both character and
binary I/O over a single underlying binary stream, with one shared
buffer. With it, READ-MESSAGE creates one wrap and uses it for both:
READ-LINE for ASCII headers, READ-SEQUENCE of bytes for the UTF-8
body. The local READ-ASCII-LINE helper and the custom EOF condition
go away; READ-LINE signals stdlib END-OF-FILE on stream end and the
default RECEIVE-MESSAGE-USING-TRANSPORT method catches that.

Trade-off: flexi-streams becomes a direct dep. Widely depended on
across the CL ecosystem (drakma, hunchentoot, websocket-driver) but
not currently a transitive dep of jsonrpc itself.

All 4 upstream test suites pass.
@ramfjord

Copy link
Copy Markdown
Author

The TL;DR on this is that it fixes a bug for UTF characters (specifics can be found in the individual test additions), by relying a bit more directly on the flexi-stream library which is already a dependency here

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