spec: Fix receiver algorithm to account for FD batching#10
spec: Fix receiver algorithm to account for FD batching#10cgwalters merged 1 commit intobootc-dev:mainfrom
Conversation
Looking at bootc-dev#9 basically the spec had an internal contradiction one part mandating returning an error with fewer than expected FDs, but a whole other section explaining how to handle more FDs than fit in a single sendmsg clal. Fix this by clarifying and cross referencing how to handle the latter case. Assisted-by: OpenCode (Claude Opus 4) Signed-off-by: Colin Walters <walters@verbum.org>
There was a problem hiding this comment.
Code Review
This pull request updates the README to clarify file descriptor ordering and handling, specifically requiring receivers to buffer parsed messages when descriptors are batched across multiple calls. The review feedback highlights a potential logic flaw in the processing steps where the byte buffer needs to be cleared before checking for the next message to prevent incorrect error triggers. There is also a recommendation to use consistent terminology for non-whitespace bytes to align with RFC 8259.
| 2. **Handle Parse Result:** If parsing succeeds, record the number of bytes consumed. If parsing fails with a syntax error (not EOF), this is a fatal Framing Error (see Section 7), and the connection MUST be closed. | ||
| 3. **Read FD Count:** Read the `fds` field from the parsed JSON message to determine the number of file descriptors (N) associated with this message. If the field is absent, N is 0. | ||
| 4. **Check FD Queue:** Check if the file descriptor queue contains at least N FDs. If it contains fewer than N FDs, this is a fatal Mismatched Count error (see Section 7). The protocol state is desynchronized, and the connection MUST be closed. | ||
| 4. **Check FD Queue:** Check if the file descriptor queue contains at least N FDs. If it contains fewer than N FDs, the receiver MUST buffer the parsed message and return to reading from the socket (outer Step 2) until enough FDs have been received. Between messages, the only bytes the sender may transmit are whitespace continuation bytes carrying additional FDs (see Section 4.1). After each read, if the byte buffer contains any non-whitespace byte (i.e. the start of the next JSON message) and the FD queue still has fewer than N FDs, this is a fatal Mismatched Count error (see Section 7) — the sender has violated the ordering rule in Section 2.3 by beginning the next message before delivering all FDs for the current one. If the connection is closed before all N FDs have arrived, this is also a fatal Mismatched Count error. |
There was a problem hiding this comment.
The logic for detecting a Mismatched Count error is currently ambiguous. Since the consumed bytes of the current message are not removed from the buffer until Step 7, the buffer will still contain the non-whitespace JSON of the current message when the receiver returns to Step 2. Consequently, the check for 'any non-whitespace byte' will trigger a fatal error immediately after the next read, even if the sender is correctly sending whitespace. I suggest explicitly stating that the bytes should be removed before waiting, which also aligns with how the current Rust implementation is structured (draining the buffer immediately after a successful parse).
| 4. **Check FD Queue:** Check if the file descriptor queue contains at least N FDs. If it contains fewer than N FDs, the receiver MUST buffer the parsed message and return to reading from the socket (outer Step 2) until enough FDs have been received. Between messages, the only bytes the sender may transmit are whitespace continuation bytes carrying additional FDs (see Section 4.1). After each read, if the byte buffer contains any non-whitespace byte (i.e. the start of the next JSON message) and the FD queue still has fewer than N FDs, this is a fatal Mismatched Count error (see Section 7) — the sender has violated the ordering rule in Section 2.3 by beginning the next message before delivering all FDs for the current one. If the connection is closed before all N FDs have arrived, this is also a fatal Mismatched Count error. | |
| 4. **Check FD Queue:** Check if the file descriptor queue contains at least N FDs. If it contains fewer than N FDs, the receiver MUST remove the consumed bytes from the front of the byte buffer (as in Step 7), buffer the parsed message, and return to reading from the socket (outer Step 2) until enough FDs have been received. Between messages, the only bytes the sender may transmit are whitespace continuation bytes carrying additional FDs (see Section 4.1). After each read, if the byte buffer contains any non-whitespace byte (i.e. the start of the next JSON message) and the FD queue still has fewer than N FDs, this is a fatal Mismatched Count error (see Section 7) — the sender has violated the ordering rule in Section 2.3 by beginning the next message before delivering all FDs for the current one. If the connection is closed before all N FDs have arrived, this is also a fatal Mismatched Count error. |
|
|
||
| * **Framing Error:** The byte stream cannot be parsed as valid JSON (syntax error, not incomplete data). | ||
| * **Mismatched Count:** A parsed message's `fds` field specifies N file descriptors, but the receiver's file descriptor queue contains fewer than N available FDs at the time of processing. | ||
| * **Mismatched Count:** A parsed message's `fds` field specifies N file descriptors, but either the connection was closed or a non-continuation character was received before N FDs were received. This indicates that the sender failed to deliver the promised file descriptors. |
There was a problem hiding this comment.
For consistency with Section 5, Step 4, consider using the term 'non-whitespace byte' instead of 'non-continuation character'. This clarifies that any character not permitted by RFC 8259 as whitespace (and thus potentially the start of a new message) triggers the error.
| * **Mismatched Count:** A parsed message's `fds` field specifies N file descriptors, but either the connection was closed or a non-continuation character was received before N FDs were received. This indicates that the sender failed to deliver the promised file descriptors. | |
| * **Mismatched Count:** A parsed message's `fds` field specifies N file descriptors, but either the connection was closed or a non-whitespace byte was received before N FDs were received. This indicates that the sender failed to deliver the promised file descriptors. |
Looking at #9 basically the spec had an internal contradiction one part mandating returning an error with fewer than expected FDs, but a whole other section explaining how to handle more FDs than fit in a single sendmsg clal.
Fix this by clarifying and cross referencing how to handle the latter case.
Assisted-by: OpenCode (Claude Opus 4)