Add IRCv3 labeled-response and batch capabilities - #110
Open
MrIron-no wants to merge 7 commits into
Open
Conversation
Implements labeled-response (dependent on batch) via a per-connection capture in parse.c/send.c: a labeled command's output is deferred and released as a bare ACK, a single labeled line, or a labeled BATCH depending on how many lines it produced. LIST is a special case: rather than deferring output for an end-of- command decision, a labeled LIST commits to BATCH immediately (label_capture_stream_active(), send.c) and streams each line to the wire tagged batch=ref as list_next_channels() (hash.c) produces it, one event-loop tick at a time. Streamed output goes through the real send_buffer()/cli_sendQ() path, so a labeled LIST now genuinely pauses and resumes across ticks exactly like an unlabeled one, and can never overflow an in-memory buffer. A second LIST/STOP superseding a still- parked one closes the old batch cleanly (RPL_LISTEND folded in, then BATCH -ref via reopen()+finish(), not abort()) and opens a fresh BATCH for the new command -- fixes a parse.c bug where any already-parked LIST continuation caused an unrelated labeled command's capture to be misrouted into it. Propagates labels across S2S: hunt_server_cmd()-routed commands (the "WHOIS nick nick" trick, remote STATS) carry @Label= over the wire (sendcmdto_one_hunted(), gated on FEAT_NETWORK_FEATURES like other federated tags) to whichever server actually answers on the original requester's behalf (parse_server()'s labeled-response wrapper), with BATCH/ACK relayed back hop by hop by address (ms_batch()/ms_ack(), m_batch.c) the same way do_numeric() already relays numerics. Confirmed safe across a partially-upgraded network (nf_compat topology): a peer with NETWORK_FEATURES off never sees an @Label=/@Batch= tag, and the worst case (a hop in the middle doesn't propagate) degrades to a plain, unlabeled reply -- the spec's own sanctioned fallback for a response a server can't honestly label, not a hang or a crash. Adds test coverage in tests/labeled_response/: core capture/ACK/BATCH shapes, CAP dependency enforcement, LIST at scale and under interruption (channels seeded via a fake P10 server-link burst rather than real client JOINs, which trip ircu's unrelated target-change flood limit at scale), the async-parking regression above, remote WHOIS/STATS over a real S2S round trip at one and two hops, and (tests/pr_network_features_ compat/) the same over a mixed-version network. Claude-Session: https://claude.ai/code/session_01SyPrvPCSmSY7hxx3QL3cQn
- parse_server(): re-verify `from` is still alive after the handler runs via a safe (non-dereferencing) hash lookup instead of trusting rc == CPTR_KILLED, which only fires when cptr == victim. A server-origin QUIT frees `from` (the user) while cptr (the link) survives, so the old check missed it and label_capture_finish() dereferenced freed memory. Servers verify via FindNServer()/cli_yxx() (numeric), matching the lookup style already used earlier in the same function, rather than by name. - exit_one_client() (s_misc.c): properly finish outstanding label captures for a remote client before it's freed, instead of leaving parse_server() to discover the free too late. - sendcmdto_one_hunted(): use label_capture_abort() on handoff so any already-buffered lines are flushed unlabeled instead of discarded. - m_list.c / send.c: harden label_capture_reopen() to report whether it actually found the capture, closing the window where a stale label_ref could misdirect a superseding LIST's RPL_LISTEND into an unrelated capture. - test_remote_labeled_quit_for_own_user_does_not_crash_hub: add cleanup and a bounded retry on the trailing WHOIS check. Diagnosed a flaky timeout in the full labeled_response suite (never in isolation); confirmed via an ASan rebuild (0 violations, 26/26 pass) that this is session-level connection-accounting noise (IPcheck.c, shared across tests on 127.0.0.1), not a real defect.
do_numeric()'s pattern of rewriting the sender to &me at every relay hop is fine when every server agrees on FEAT_HIS_REWRITE, but on a mixed-config network an earlier hop's rewrite permanently overwrites sptr before a later hop -- one that might have HIS configured differently -- ever gets a say, silently discarding the true origin. Relaying hops now forward sptr untouched; only the hop that actually MyConnect()s the target makes the rewrite call, in both ms_batch() and ms_ack(). Also gate local delivery on the target actually having CAP_BATCH and CAP_LABELED_RESPONSE active (matching the exact check parse.c already uses before starting a capture), so a client that dropped or never negotiated these caps doesn't get a raw BATCH/ACK line sprung on it by a remote peer's relay.
parse.c decided "this handler just started a new listing" by comparing the cli_listing() pointer before and after the handler ran. m_list.c's superseding path frees the old ListingArgs and immediately allocates a new one of the same size, which the allocator routinely returns at the same address, so a labeled LIST that replaced a parked one looked "unchanged" and parse.c finished its brand-new streaming capture on the spot: BATCH -ref went out after the first tick and every later RPL_LIST plus the final RPL_LISTEND left unlabeled, outside any batch. m_list() already stamps the capture ref into ListingArgs.label_ref (via label_capture_stream_active()) when it starts a paginated listing, so parse.c now simply checks whether the current listing carries this dispatch's ref. The pointer snapshot and the redundant re-stamp are gone. Regression test: two labeled LISTs where the second (LIST >0, so it starts a real listing rather than acting as LIST STOP) arrives while the first is genuinely parked. Getting a LIST to park needs the hub's kernel write to block: the test connects to the container IP (docker-proxy buffers on its own), throttles the client's receive side, and stops reading. The pre-existing test on the TinySendQ port never parked anything because send_buffer() flushes every 1KB. The test asserts on distinct channel names: list_next_channels() breaks before args->bucket++ and so re-sends the bucket it paused on -- a pre-existing upstream bug (UndernetIRC#109), not addressed here. Also: correct a stale "500-line/64KB" comment (the limits are 5000 lines and 1MB), a misleading comment in hash.c about when label_ref is set, list CAP_BATCH / CAP_LABELED_RESPONSE in the example config, and fix the copyright line in m_batch.c.
send.c had grown by 550 lines of capture machinery that only touched it at two points. It now lives in ircd/label.c with its interface in include/label.h: - send_buffer() calls label_capture_intercept(), which returns whether it took the line into the active capture. - sendcmdto_one_hunted() asks label_capture_active_for() for the requester's active capture instead of reading the window statics. struct MsgTagCtx and msgtagctx_init() were private to send.c; label.c snapshots the context by value for every captured line, so they move into send.h. The intercept takes the effective tag context (cache ctx or explicit ctx) rather than the cache, so struct TagSendCache stays private. ms_ack() moves from m_batch.c into its own m_ack.c, mirroring the one-handler-per-file layout, with the same CAP_BATCH + CAP_LABELED_ RESPONSE delivery gate as ms_batch() and parse.c's capture start. No behaviour change. Comment pointers that said "in send.c" now say label.c.
On the server answering a hunted command for a *remote* requester, the capture window was keyed on cli_from(requester) -- the S2S link -- and send_buffer() compared the resolved destination. Every line the handler sent down that link during the dispatch was swept into the requester's response, whoever it was addressed to. RPING is the cleanest trigger: "RPING <target> <start>" is hunted to <start>, which then sends an RPING to <target>; with the requester's own server as <target> that RPING went back down the requesting link and was captured as if it were the reply. Remote CONNECT's WALLOPS broadcast had the same problem. - The capture list moves from struct Connection to struct Client, and label_capture_start() keys the window on the requesting client itself. A remote requester's captures are no longer shared with, or disposed of alongside, other remote users behind the same link. - send_buffer() runs the intercept on the *intended recipient* before resolving it to the link. sendcmdto_one() / sendcmdto_prio_one() pass the recipient through and only use the resolved link to choose the wire form; sendcmdto_one_hunted() looks up the capture by requester. - parse_server() only treats a *user*-prefixed labeled line as a request to answer. hunt_server_cmd() always forwards with the requester as prefix, so a labeled line with a *server* prefix can only be an answering server's reply being relayed back. Previously the relaying hop mistook such a single-line non-numeric reply (e.g. ms_connect()'s "Host not listed in ircd.conf" NOTICE) for a new labeled request, wrapped it, and stripped the label before delivering it. Numerics were never affected because do_numeric() relays them first. The now unreachable server-prefix verification branch is removed. Regression tests (multi-server): remote RPING back down the requesting link resolves as a bare ACK followed by a plain RPONG; a remote CONNECT NOTICE reply reaches the requester with its label. Also a test that a labeled WALLOPS labels only the sender's own echo -- other recipients, local and on a linked server, see no label.
msg_tag_s2s_needs_time() decided which commands must not get @time= invented on the S2S wire with a chain of 22 ircd_strcmp() calls against token strings, evaluated from msgtagctx_init() on every send. The list was the only place that knowledge lived, far from the command table. The policy is now a flag on the msgtab[] entries themselves: MFLG_NO_S2S_TIME on the link/state and net-admin protocol commands (BURST, EB, EA, SERVER, PING, PONG, SETTIME, ASLL, RPING, RPONG, UPING, PASS, ERROR, PROTO, SQUIT, CONFIG, JUPE, GLINE, SLINE, DESTRUCT) and on the server<->services RPC (XQUERY, XREPLY) that services parse positionally without stripping tags. msg_tag_s2s_needs_time() looks the token up through the existing token trie (msg_find_by_tok(), parse.c) and reads the flag -- a few character steps instead of 22 compares. Same set of commands, same behaviour: tokens not in the table at all still get @time=, as before.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Implements the IRCv3
batchandlabeled-responsecapabilities (labeled-response depends on batch;CAP REQfor one without the other is NAKed).How it works
Local commands. A labeled command's output to the requesting connection is captured while its handler runs (
label_capture_intercept()fromsend_buffer()), then released as a bareACK, a single line carrying@label=, or aBATCH +ref labeled-response…BATCH -refwith@batch=refbody lines, depending on how many lines the handler produced. The label is removed from the line's ambient tag list before the handler runs, so nothing a handler broadcasts (WALLOPS, channel traffic, S2S) can pick it up; only the requester's own response is labeled.LIST. Unconditionally multi-line and paginated across event-loop ticks, so it streams instead of buffering:
m_list()commits to a BATCH immediately and eachlist_next_channels()tick's output goes out tagged@batch=refthrough the real sendq path. A supersedingLISTcloses the old batch cleanly (RPL_LISTEND folded in) and opens a fresh one.Remote queries over S2S. For
hunt_server_cmd()-routed commands (WHOIS nick nick, remoteSTATS, …) the origin server forwards the command with@label=and drops its own capture. Only the server that actually answers captures its own output, keyed on the remote requester, and emits S2S-addressedBA <numnick> +ref/-ref/AK <numnick>lines. Intermediate hops relay them (ms_batch(),ms_ack(), anddo_numeric()for the tagged body) and never reassemble anything. Gated onNETWORK_FEATURESlike the other federated tags; on a mixed network the reply degrades to plain unlabeled output, as the spec allows.New files:
ircd/label.c/include/label.h(the capture machinery, split out of send.c),ircd/m_batch.c,ircd/m_ack.c. New featuresCAP_BATCH,CAP_LABELED_RESPONSE.Fixes made during review of this branch
cli_listing()pointer comparison, and the freed-then-reallocatedListingArgsroutinely comes back at the same address. Now matched by ref.@time=carve-out moved from a 22-waystrcmpchain in msg_tag.c onto the command table asMFLG_NO_S2S_TIME.Tests
tests/labeled_response/(30 tests, single- and multi-server): ACK / single-line / BATCH shapes, CAP dependency, label value limits and escaping, LIST at scale and genuinely parked mid-pagination, remote WHOIS/STATS at one and two hops, interim traffic not mislabeled, the regressions above, and no label leakage from a labeled WALLOPS.tests/pr_network_features_compat/test_nf_compat_labeled_response.pycovers the mixed-version network. The S2S time tests intests/pr_msgtags_compat/pin theMFLG_NO_S2S_TIMEset.Notes
send_buffer()flushes every 1KB. The new list-pause test does this.list_next_channels()re-sends the hash bucket it paused on (LIST: list_next_channels() re-sends the hash bucket it paused on (duplicate RPL_LIST entries) #109).NETWORK_FEATURESoff, a hunted command answers with an immediateACKand the real reply follows unlabeled.