Multi-line bodies: the IRC listener now sends one PRIVMSG or NOTICE line for each line of body, not only the first.
Mode values: the MODE line is built from all body lines, with the first split on spaces. ["+k", "secret"] becomes MODE #chan +k secret and ["+o bob"] becomes MODE #chan +o bob, where before it was MODE #chan :+o bob.
Line breaks: CR, LF and NUL are removed from each parameter of an IRC listener line before deciding whether it needs a leading :, and from the whole line, so a stored body line, key or mask cannot start a line of its own.
Channel names: the check (# followed by 1–63 letters, digits, _ or -) moved from the HTTP handler's handleJoin into service.JoinChannel. Both transports call it, so a bad name gets 403 on both.
Not visible in the diff:
The bodies the HTTP API stores for a mode change are unchanged and still match README.md. Only the IRC line built from them changed.
The tests queue messages through the database and broker with the body the HTTP API stores, because internal/ircserver tests do not start the HTTP API.
Judgement call: the echo an IRC listener user gets for their own MODE change also changed, from one trailing parameter (:+o bob) to separate parameters. Without this, the user who made the change and the other members would see the same change in two different forms.
Not changed: as README.md documents, a +H change over the HTTP API is still not sent to members.
Model: opus-5-5
Closes https://git.eeqj.de/sneak/neoirc/issues/127.
- **Multi-line bodies**: the IRC listener now sends one `PRIVMSG` or `NOTICE` line for each line of `body`, not only the first.
- **Mode values**: the `MODE` line is built from all body lines, with the first split on spaces. `["+k", "secret"]` becomes `MODE #chan +k secret` and `["+o bob"]` becomes `MODE #chan +o bob`, where before it was `MODE #chan :+o bob`.
- **Line breaks**: CR, LF and NUL are removed from each parameter of an IRC listener line before deciding whether it needs a leading `:`, and from the whole line, so a stored body line, key or mask cannot start a line of its own.
- **Channel names**: the check (`#` followed by 1–63 letters, digits, `_` or `-`) moved from the HTTP handler's `handleJoin` into `service.JoinChannel`. Both transports call it, so a bad name gets 403 on both.
Not visible in the diff:
- The bodies the HTTP API stores for a mode change are unchanged and still match `README.md`. Only the IRC line built from them changed.
- The tests queue messages through the database and broker with the body the HTTP API stores, because `internal/ircserver` tests do not start the HTTP API.
Judgement call: the echo an IRC listener user gets for their own `MODE` change also changed, from one trailing parameter (`:+o bob`) to separate parameters. Without this, the user who made the change and the other members would see the same change in two different forms.
Not changed: as `README.md` documents, a `+H` change over the HTTP API is still not sent to members.
Model: opus-5-5
internal/ircserver/relay.go, deliverTextMessage and deliverMode: body lines and mode values go onto the IRC connection unchanged. The HTTP API accepts and stores a body line, +k key or +b mask containing a CR or LF, so an HTTP API user can make IRC listener users receive whole extra lines with any sender and command, e.g. a body ["hi", "x\r\n:admin!admin@host PRIVMSG bob :..."] or a key k\r\nERROR :Closing link. Before this change only the first body line was exposed; now every line, the key and the mask are. A line holding a line break also does not arrive as its own PRIVMSG or NOTICE line, as the definition of done in #127 asks. Acceptable: no CR, LF or NUL from a stored message ever reaches the connection as a line break (handling it where every outgoing line is written covers the first line too), with an internal/ircserver test that queues such a body line, key and mask and checks that no extra line arrives.
Judgement call: the change to the echo an IRC listener user gets for their own MODE is taken as part of item 2 of the definition of done, not as scope creep.
Model: opus-5-5
FAIL (`needs-rework`)
1. `internal/ircserver/relay.go`, `deliverTextMessage` and `deliverMode`: body lines and mode values go onto the IRC connection unchanged. The HTTP API accepts and stores a body line, `+k` key or `+b` mask containing a CR or LF, so an HTTP API user can make IRC listener users receive whole extra lines with any sender and command, e.g. a body `["hi", "x\r\n:admin!admin@host PRIVMSG bob :..."]` or a key `k\r\nERROR :Closing link`. Before this change only the first body line was exposed; now every line, the key and the mask are. A line holding a line break also does not arrive as its own `PRIVMSG` or `NOTICE` line, as the definition of done in https://git.eeqj.de/sneak/neoirc/issues/127 asks. Acceptable: no CR, LF or NUL from a stored message ever reaches the connection as a line break (handling it where every outgoing line is written covers the first line too), with an `internal/ircserver` test that queues such a body line, key and mask and checks that no extra line arrives.
Judgement call: the change to the echo an IRC listener user gets for their own `MODE` is taken as part of item 2 of the definition of done, not as scope creep.
Model: opus-5-5
Fixed: the one function that writes every IRC listener line now removes CR, LF and NUL before writing, so no stored body line (first or later), key, mask or other relayed value can start a line of its own; a body line holding a line break arrives inside its own PRIVMSG or NOTICE. New test TestRelayLineBreaks in internal/ircserver queues such a body, key and mask and checks the exact lines that arrive. README.md says so in one sentence.
Judgement call: the characters are removed rather than replaced with a space, because the line is already formatted and an added space would split a parameter in two.
Model: opus-5-5
Rework:
1. Fixed: the one function that writes every IRC listener line now removes CR, LF and NUL before writing, so no stored body line (first or later), key, mask or other relayed value can start a line of its own; a body line holding a line break arrives inside its own `PRIVMSG` or `NOTICE`. New test `TestRelayLineBreaks` in `internal/ircserver` queues such a body, key and mask and checks the exact lines that arrive. `README.md` says so in one sentence.
Judgement call: the characters are removed rather than replaced with a space, because the line is already formatted and an added space would split a parameter in two.
Model: opus-5-5
internal/ircserver/conn.go, sendWithin: CR, LF and NUL are removed from the finished line, after FormatMessage has already decided whether the last parameter needs a leading :. A value made only of those characters therefore loses its parameter, and a value that starts with : once they are removed loses that :. An HTTP API body line "\r\n" reaches IRC listener users as :carol!carol@* PRIVMSG bob with no text, which is not a valid IRC line, and a body line "\x00:hi" arrives as text hi instead of :hi. The same applies to every other relayed value, such as a topic, away message or reason. Acceptable: the characters are removed from each parameter before FormatMessage decides on the : (also removing them from the whole line is fine), so "\r\n" arrives as PRIVMSG bob : and "\x00:hi" as PRIVMSG bob ::hi, with both cases in TestRelayLineBreaks.
Model: opus-5-5
FAIL (`needs-rework`)
1. `internal/ircserver/conn.go`, `sendWithin`: CR, LF and NUL are removed from the finished line, after `FormatMessage` has already decided whether the last parameter needs a leading `:`. A value made only of those characters therefore loses its parameter, and a value that starts with `:` once they are removed loses that `:`. An HTTP API body line `"\r\n"` reaches IRC listener users as `:carol!carol@* PRIVMSG bob ` with no text, which is not a valid IRC line, and a body line `"\x00:hi"` arrives as text `hi` instead of `:hi`. The same applies to every other relayed value, such as a topic, away message or reason. Acceptable: the characters are removed from each parameter before `FormatMessage` decides on the `:` (also removing them from the whole line is fine), so `"\r\n"` arrives as `PRIVMSG bob :` and `"\x00:hi"` as `PRIVMSG bob ::hi`, with both cases in `TestRelayLineBreaks`.
Model: opus-5-5
Fixed: FormatMessage now removes CR, LF and NUL from each parameter before deciding on the leading :, so a body line "\r\n" arrives as PRIVMSG bob : and "\x00:hi" as PRIVMSG bob ::hi; both cases are in TestRelayLineBreaks. The whole-line removal in sendWithin stays, for lines built without FormatMessage. The PR description's line-break item now says this.
Model: opus-5-5
Rework:
1. Fixed: `FormatMessage` now removes CR, LF and NUL from each parameter before deciding on the leading `:`, so a body line `"\r\n"` arrives as `PRIVMSG bob :` and `"\x00:hi"` as `PRIVMSG bob ::hi`; both cases are in `TestRelayLineBreaks`. The whole-line removal in `sendWithin` stays, for lines built without `FormatMessage`. The PR description's line-break item now says this.
Model: opus-5-5
internal/ircserver/conn.go, sendWithin: no test fails when the removal of CR, LF and NUL from the whole line is taken out, yet it is the only protection for lines not built with FormatMessage, and for the prefix. An HTTP API operator's KILL reason is a body line that reaches the victim's ERROR :Closing Link: ... line this way. A later cleanup that drops this removal as a duplicate of the one in FormatMessage would let that reason start a line of its own, and no test would catch it. Acceptable: an internal/ircserver test that kills an IRC listener user with a reason holding CR, LF and NUL (for example through Disconnect, next to TestDisconnectNotifiesAndClosesVictim) and checks that exactly the KILL and ERROR lines arrive.
Landing commit message: the body is well over about 120 words. Acceptable: a body of at most about 120 words, still ending with Model: opus-5-5.
Model: opus-5-5
FAIL (`needs-rework`)
1. `internal/ircserver/conn.go`, `sendWithin`: no test fails when the removal of CR, LF and NUL from the whole line is taken out, yet it is the only protection for lines not built with `FormatMessage`, and for the prefix. An HTTP API operator's `KILL` reason is a body line that reaches the victim's `ERROR :Closing Link: ...` line this way. A later cleanup that drops this removal as a duplicate of the one in `FormatMessage` would let that reason start a line of its own, and no test would catch it. Acceptable: an `internal/ircserver` test that kills an IRC listener user with a reason holding CR, LF and NUL (for example through `Disconnect`, next to `TestDisconnectNotifiesAndClosesVictim`) and checks that exactly the `KILL` and `ERROR` lines arrive.
2. Landing commit message: the body is well over about 120 words. Acceptable: a body of at most about 120 words, still ending with `Model: opus-5-5`.
Model: opus-5-5
The IRC listener relayed only the first line of a PRIVMSG or NOTICE
body; it now sends one line per body line. A relayed MODE change now
carries its key, limit or mask, each value its own parameter, also in
the echo to the user who made it. CR, LF and NUL are removed from each
parameter and from each whole line sent to an IRC listener connection,
so no stored value can start a line of its own. The channel name check
moved into service.JoinChannel, so the IRC listener's JOIN refuses the
same names with 403.
Model: opus-5-5
Fixed: new test TestDisconnectReasonCannotStartLine in internal/ircserver kills an IRC listener user through Disconnect with a reason holding CR, LF and NUL and checks that exactly the KILL and ERROR lines arrive; it fails when the removal in sendWithin is taken out.
Fixed: commit body cut to under 100 words.
Model: opus-5-5
Rework:
1. Fixed: new test `TestDisconnectReasonCannotStartLine` in `internal/ircserver` kills an IRC listener user through `Disconnect` with a reason holding CR, LF and NUL and checks that exactly the `KILL` and `ERROR` lines arrive; it fails when the removal in `sendWithin` is taken out.
2. Fixed: commit body cut to under 100 words.
Model: opus-5-5
PASS: the change meets the definition of done in #127, and every finding from the earlier reviews is resolved.
Judgement call: the PR body is slightly over 250 words; I took that as within the limit of about 250.
Model: opus-5-5
PASS: the change meets the definition of done in https://git.eeqj.de/sneak/neoirc/issues/127, and every finding from the earlier reviews is resolved.
Judgement call: the PR body is slightly over 250 words; I took that as within the limit of about 250.
Model: opus-5-5
clawbot
merged commit 1308769a25 into next2026-10-20 13:48:17 +02:00
Blocking a user prevents them from interacting with repositories, such as opening or commenting on pull requests or issues. Learn more about blocking a user.
Closes #127.
PRIVMSGorNOTICEline for each line ofbody, not only the first.MODEline is built from all body lines, with the first split on spaces.["+k", "secret"]becomesMODE #chan +k secretand["+o bob"]becomesMODE #chan +o bob, where before it wasMODE #chan :+o bob.:, and from the whole line, so a stored body line, key or mask cannot start a line of its own.#followed by 1–63 letters, digits,_or-) moved from the HTTP handler'shandleJoinintoservice.JoinChannel. Both transports call it, so a bad name gets 403 on both.Not visible in the diff:
README.md. Only the IRC line built from them changed.internal/ircservertests do not start the HTTP API.Judgement call: the echo an IRC listener user gets for their own
MODEchange also changed, from one trailing parameter (:+o bob) to separate parameters. Without this, the user who made the change and the other members would see the same change in two different forms.Not changed: as
README.mddocuments, a+Hchange over the HTTP API is still not sent to members.Model: opus-5-5
FAIL (
needs-rework)internal/ircserver/relay.go,deliverTextMessageanddeliverMode: body lines and mode values go onto the IRC connection unchanged. The HTTP API accepts and stores a body line,+kkey or+bmask containing a CR or LF, so an HTTP API user can make IRC listener users receive whole extra lines with any sender and command, e.g. a body["hi", "x\r\n:admin!admin@host PRIVMSG bob :..."]or a keyk\r\nERROR :Closing link. Before this change only the first body line was exposed; now every line, the key and the mask are. A line holding a line break also does not arrive as its ownPRIVMSGorNOTICEline, as the definition of done in #127 asks. Acceptable: no CR, LF or NUL from a stored message ever reaches the connection as a line break (handling it where every outgoing line is written covers the first line too), with aninternal/ircservertest that queues such a body line, key and mask and checks that no extra line arrives.Judgement call: the change to the echo an IRC listener user gets for their own
MODEis taken as part of item 2 of the definition of done, not as scope creep.Model: opus-5-5
275670d092to802c0d3c99802c0d3c99to54ecbda48854ecbda488toe9a5e55ed6Rework:
PRIVMSGorNOTICE. New testTestRelayLineBreaksininternal/ircserverqueues such a body, key and mask and checks the exact lines that arrive.README.mdsays so in one sentence.Judgement call: the characters are removed rather than replaced with a space, because the line is already formatted and an added space would split a parameter in two.
Model: opus-5-5
FAIL (
needs-rework)internal/ircserver/conn.go,sendWithin: CR, LF and NUL are removed from the finished line, afterFormatMessagehas already decided whether the last parameter needs a leading:. A value made only of those characters therefore loses its parameter, and a value that starts with:once they are removed loses that:. An HTTP API body line"\r\n"reaches IRC listener users as:carol!carol@* PRIVMSG bobwith no text, which is not a valid IRC line, and a body line"\x00:hi"arrives as texthiinstead of:hi. The same applies to every other relayed value, such as a topic, away message or reason. Acceptable: the characters are removed from each parameter beforeFormatMessagedecides on the:(also removing them from the whole line is fine), so"\r\n"arrives asPRIVMSG bob :and"\x00:hi"asPRIVMSG bob ::hi, with both cases inTestRelayLineBreaks.Model: opus-5-5
e9a5e55ed6tobf509c5b06Rework:
FormatMessagenow removes CR, LF and NUL from each parameter before deciding on the leading:, so a body line"\r\n"arrives asPRIVMSG bob :and"\x00:hi"asPRIVMSG bob ::hi; both cases are inTestRelayLineBreaks. The whole-line removal insendWithinstays, for lines built withoutFormatMessage. The PR description's line-break item now says this.Model: opus-5-5
FAIL (
needs-rework)internal/ircserver/conn.go,sendWithin: no test fails when the removal of CR, LF and NUL from the whole line is taken out, yet it is the only protection for lines not built withFormatMessage, and for the prefix. An HTTP API operator'sKILLreason is a body line that reaches the victim'sERROR :Closing Link: ...line this way. A later cleanup that drops this removal as a duplicate of the one inFormatMessagewould let that reason start a line of its own, and no test would catch it. Acceptable: aninternal/ircservertest that kills an IRC listener user with a reason holding CR, LF and NUL (for example throughDisconnect, next toTestDisconnectNotifiesAndClosesVictim) and checks that exactly theKILLandERRORlines arrive.Model: opus-5-5.Model: opus-5-5
bf509c5b06to65ae37d97aRework:
TestDisconnectReasonCannotStartLineininternal/ircserverkills an IRC listener user throughDisconnectwith a reason holding CR, LF and NUL and checks that exactly theKILLandERRORlines arrive; it fails when the removal insendWithinis taken out.Model: opus-5-5
PASS: the change meets the definition of done in #127, and every finding from the earlier reviews is resolved.
Judgement call: the PR body is slightly over 250 words; I took that as within the limit of about 250.
Model: opus-5-5
clawbot referenced this pull request2026-10-20 14:14:27 +02:00