Repository navigation
fix(net): write IETF data stream headers in one write (backport #6504) - #6769
Conversation
imquic drops a SUBGROUP_HEADER or FETCH_HEADER that spans more than one stream read, and @moq/net on release writes the header one field per write. Write the stream type and header as one chunk, as main does since #6504. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
kixelated
left a comment
There was a problem hiding this comment.
Automated review by review (OpenAI)
Reviewed commit: d29a1aa
No actionable bugs found. This is a focused backport: buffering preserves the header encoding, copies reusable scratch buffers safely, and moves the FETCH stream type into the header encoder without duplicating it at the publisher. The regression checks both varint formats, write count and decoding. Coalescing writes is a reasonable interoperability mitigation, though it cannot guarantee transport read boundaries.
Verification: static review of the patch, writer lifecycle, publisher call path and original in-repository change. I did not run tests or independently verify the external imquic/runner results.
@moq/net 0.4.7 shipped from release with #6769, and englishm/moq-interop-runner#179 moves the runner's moq-dev-js client to it. Rewrite the quest to wait for that PR to merge and the client image to be rebuilt. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
imquic drops a SUBGROUP_HEADER or FETCH_HEADER that arrives across more than one stream read (meetecho/imquic#43), and @moq/net 0.4.6 and earlier write those headers one field per write, so imquic discards every object and data-subgroup-basic fails. 0.4.7 carries moq-dev/moq#6769 and writes each header in one write. Against the imquic relay image (draft-22, WebTransport), the client built from this commit passes data-subgroup-basic 3/3 (0.4.6: 0/3). announce-subscribe times out on both versions. Also corrects the TAP banner, which still said 0.4.4. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Backports the
js/netchange from moq-dev/moq#6504 torelease, so it ships in the next@moq/netpatch release and the interop runner's moq-dev-js client can bump to it without takingmain's breaking API.Problem
imquic drops a SUBGROUP_HEADER or FETCH_HEADER that arrives across more than one stream read (meetecho/imquic#43, acknowledged upstream, still open).
@moq/net0.4.6 onreleasewrites the header one field perwrite(), which the@moq/web-transportpolyfill sends as separate STREAM frames, so the runner's moq-dev-jsdata-subgroup-basiccase gets no objects through imquic (draft-22, WebTransport).Changes
Group.encodeandFetchHeader.encodewrite the stream type and header in one write, as onmain. The fetch path no longer writes its stream type separately.Writer.collectandconcatcome over frommain(moq-dev/moq#6255) unchanged, so the release-into-main merge stays clean.stream.tsis not exported from the package.ietf/object.test.tscarries fix(net): write IETF data stream headers in one write #6504's test; it fails onreleasewithout the fix (6 writes instead of 2).Public API and wire impact
Verification
The runner's moq-dev-js client (
builds/moq-dev-js/src/main.tsat upstream HEAD) againstghcr.io/englishm/moq-interop-runner-imquic-moq-relay:latestlocally:@moq/netdata-subgroup-basicThe other cases are unchanged between the two:
announce-subscribetimes out waiting for the broadcast with both, so it is unrelated.Checks
just check origin/release: pass (@moq/net1282 tests).just test interop --all: not run. The harness runs moq-lite, which this does not touch; the live imquic runs above are the coverage.Choices
CONTRIBUTING.md(broken interop), JS-only like moq-dev/moq#6518. The Rust publisher already buffers the header.Tracked by the quest PR moq-dev/moq#6764.
(Written by Claude Opus 5.5)
🤖 Generated with Claude Code