Repository navigation
Streamable HTTP: pre-existing hardening items from the #2547 review #2562
Description
Three pre-existing Streamable HTTP transport items surfaced during the #2547 review that are independent of the keep-alive and delivery work there. Collecting them here so they don't get lost; each is small enough to fix individually.
-
Protocol-version gate accepts unvalidated strings on initialize. The priming/resumability gate compares
protocolVersion >= '2025-11-25'lexicographically, and for initialize requests the string comes verbatim from the request body without being checked againstSUPPORTED_PROTOCOL_VERSIONS(non-initialize requests are header-validated). Unsupported strings that sort above the threshold (draft,2026-01-01) enable priming events and the polling callbacks for the initialize stream even though the negotiated version in the response may be lower. Gate on the negotiated version instead. -
Concurrent DELETE requests can fire
onsessionclosedtwice. While the first DELETE is parked on the callback await,_closedis still false, so a second DELETE passes the guards and fires the callback again for the same session. A flag set before invoking the callback closes the window. -
The
EventStorecontract has no session scoping for the standalone stream. Every transport stores standalone server-initiated messages under the fixed id_GET_stream, so a store instance shared across sessions (module-level store inssePollingExample, or any multi-node shared store) mixes sessions on standalone resume: one session's resume can replay another session's notifications. The contract needs a session dimension, or documented single-session store usage.
Applies to both main and v1.x.
Activity
axits-lab commentedon Jul 30, 2026
@felixweinberger I'd like to take these — starting with item 2 (concurrent DELETE double-firing
onsessionclosed), then item 1 (gate on the negotiated version rather than the unvalidatedprotocolVersionstring from the initialize body), as separate PRs since you noted each stands alone.Item 3 reads like it needs a decision before code: adding a session dimension to the
EventStorecontract is a breaking change for implementors, whereas documenting single-session store usage isn't. Happy to take it once you've called which way it should go.Are you already on any of these? Don't want to duplicate your own follow-up work.
axits-lab commentedon Jul 30, 2026
On item 1 — I think this one is mostly overtaken by #2280, and the remainder isn't local. Flagging before writing code rather than guessing at intent.
The described failure (
draft,2026-01-01sorting above the threshold and enabling priming) is already blocked onmain: #2280 bounded the gate to supported versions on 2026-06-12, sosupportsEmptySSEDataisAn unsupported string fails
includes()and never reaches the comparison, for both the priming event and thecloseSSEStream/closeStandaloneSSEStreamcallbacks.What's left is the last sentence — gating on the negotiated version — and that isn't a small change, because the negotiated version doesn't exist yet at the point the gate runs.
writePrimingEventis called atstreamableHttp.ts:938, whileinitializeisn't handed to the server until theonmessagedispatch at :957. So the transport only ever hasinitRequest.params.protocolVersion(what the client asked for) to work with. Getting the negotiated value would mean deferring the priming event until after theInitializeResultis produced, which reshapes the initialize stream lifecycle rather than tightening a condition.There is still a real gap, but it's the inverse of the one described: when a client requests a version this transport doesn't support,
includes()is false and priming stays off even though the negotiated version may well be >=2025-11-25and support it. That's a lost capability, not an unsafe enable.So — do you want (a) item 1 closed as covered by #2280, (b) the false-negative above fixed by gating on the version the transport would negotiate (computable pre-dispatch from
_supportedProtocolVersions), or (c) the full deferral so the gate sees the true negotiated version? Happy to take (b) or (c); (c) is big enough that I'd want your call on the shape first.Item 2 is up as #2583 (draft — the one-open-PR cap for non-write-access users, will mark ready once #2582 lands).