README.md

@@ -2667,7 +2667,9 @@ IRC_LISTEN_ADDR: ""
 - **CAP negotiation**: `CAP LS` gets an empty capability list and `CAP END` is
   ignored, for compatibility with modern clients.
 - **Channel prefixes**: Channels must start with `#`. `JOIN` prepends it if
-  omitted; other commands use the name as given.
+  omitted; other commands use the name as given. As over the HTTP API, `JOIN`
+  answers a name that is not `#` followed by 1–63 letters, digits, `_` or `-`
+  with ERR_NOSUCHCHANNEL (403).
 - **First joiner**: The first user to join a channel is automatically granted
   operator status (`@`).
 - **Channel modes**: `+i` (invite-only), `+m` (moderated), `+n` (no external),
@@ -2689,21 +2691,20 @@ IRC_LISTEN_ADDR: ""
 Messages sent by IRC clients appear in channels visible to HTTP/JSON API clients
 and vice versa. The IRC listener and HTTP API share the same database, broker,
 and session infrastructure, so a user connected via IRC and a user connected via
-the HTTP API can talk in the same channels, with these gaps today:
+the HTTP API can talk in the same channels.
 
-- An IRC listener user receives only the first line of a multi-line `body`.
-- A `+k`, `+l` or `+b` change made over the HTTP API reaches IRC listener
-  members without its key, limit or mask.
-- The IRC listener's `JOIN` accepts any channel name, while over the HTTP API a
-  name that is not `#` followed by 1–63 letters, digits, `_` or `-` gets
-  ERR_NOSUCHCHANNEL (403), so HTTP API users cannot join such a channel.
-- The IRC listener does not pass on a `PRIVMSG`, `NOTICE`, `JOIN`, `PART`,
-  `NICK` or `QUIT` whose sender's nick matches the user's own nick ignoring
-  case, so an IRC listener user `alice` never sees these from `Alice`, although
-  nicks are case-sensitive.
+Each line of a multi-line `body` reaches an IRC listener user as its own
+`PRIVMSG` or `NOTICE` line. A CR, LF or NUL inside a line is removed before it
+is sent, so it cannot start a line of its own. A mode change made over the HTTP
+API reaches IRC listener members as an IRC `MODE` line with its value as a
+separate parameter, e.g. `MODE #general +k secretpass` or
+`MODE #general +o bob`.
 
-https://git.eeqj.de/sneak/neoirc/issues/127 tracks the first three and
-https://git.eeqj.de/sneak/neoirc/issues/128 the last.
+One gap remains: the IRC listener does not pass on a `PRIVMSG`, `NOTICE`,
+`JOIN`, `PART`, `NICK` or `QUIT` whose sender's nick matches the user's own nick
+ignoring case, so an IRC listener user `alice` never sees these from `Alice`,
+although nicks are case-sensitive. https://git.eeqj.de/sneak/neoirc/issues/128
+tracks it.
 
 ### Docker Usage
 

internal/handlers/api.go

@@ -29,10 +29,6 @@ var validNickRe = regexp.MustCompile(
 	`^[a-zA-Z_][a-zA-Z0-9_\-\[\]\\^{}|` + "`" + `]{0,31}$`,
 )
 
-var validChannelRe = regexp.MustCompile(
-	`^#[a-zA-Z0-9_\-]{1,63}$`,
-)
-
 var validUsernameRe = regexp.MustCompile(
 	`^[a-zA-Z0-9_\-\[\]\\^{}|` + "`" + `]{1,32}$`,
 )
@@ -1489,16 +1485,6 @@ func (hdlr *Handlers) handleJoin(
 		channel = "#" + channel
 	}
 
-	if !validChannelRe.MatchString(channel) {
-		hdlr.respondIRCError(
-			writer, request, clientID, sessionID,
-			irc.ErrNoSuchChannel, nick, []string{channel},
-			"No such channel",
-		)
-
-		return
-	}
-
 	// Extract key from body lines (JOIN #channel key).
 	var suppliedKey string
 

internal/ircserver/commands.go

@@ -4,6 +4,7 @@ import (
 	"context"
 	"encoding/json"
 	"errors"
+	"slices"
 	"strconv"
 	"strings"
 	"time"
@@ -683,9 +684,11 @@ func (c *Conn) applyChannelModes(
 	if applied.Len() > 0 {
 		modeReply := applied.String() + appliedArgs.String()
 
-		c.send(FormatMessage(
-			c.hostmask(), "MODE", channel, modeReply,
-		))
+		params := slices.Concat(
+			[]string{channel}, strings.Fields(modeReply),
+		)
+
+		c.send(FormatMessage(c.hostmask(), "MODE", params...))
 
 		c.svc.BroadcastMode(
 			ctx, c.nick, channel, chID, modeReply,

internal/ircserver/conn.go

@@ -297,11 +297,17 @@ func (c *Conn) send(line string) {
 }
 
 // sendWithin writes a formatted IRC line to the connection,
-// giving up after timeout.
+// giving up after timeout. CR, LF and NUL are removed first,
+// as FormatMessage does for each parameter, so that a line
+// built without it cannot end early either.
 func (c *Conn) sendWithin(
 	timeout time.Duration,
 	line string,
 ) {
+	line = strings.ReplaceAll(line, "\r", "")
+	line = strings.ReplaceAll(line, "\n", "")
+	line = strings.ReplaceAll(line, "\x00", "")
+
 	c.writeMu.Lock()
 	defer c.writeMu.Unlock()
 

internal/ircserver/conn_test.go

@@ -5,6 +5,7 @@ import (
 	"log/slog"
 	"net"
 	"os"
+	"slices"
 	"strings"
 	"testing"
 	"time"
@@ -100,3 +101,48 @@ func TestDisconnectNotifiesAndClosesVictim(t *testing.T) {
 		t.Errorf("missing ERROR line, got: %q", joined)
 	}
 }
+
+// TestDisconnectReasonCannotStartLine checks that CR, LF and
+// NUL in a KILL reason are removed from both the KILL line
+// and the ERROR line, so the victim gets exactly those two
+// lines. The ERROR line is not built with FormatMessage, so
+// only the removal in sendWithin protects it.
+func TestDisconnectReasonCannotStartLine(t *testing.T) {
+	t.Parallel()
+
+	victim, clientSide := newKillVictim(t)
+	received := make(chan []string, 1)
+
+	go func() {
+		var lines []string
+
+		scanner := bufio.NewScanner(clientSide)
+		for scanner.Scan() {
+			lines = append(lines, scanner.Text())
+		}
+
+		received <- lines
+	}()
+
+	victim.Disconnect("bye\r\nPRIVMSG victim :forged\x00")
+
+	var lines []string
+
+	select {
+	case lines = <-received:
+	case <-time.After(testTimeout):
+		t.Fatal("the victim's connection was not closed")
+	}
+
+	// The test connection has no host name, hence the two
+	// spaces after "Closing Link:".
+	want := []string{
+		":" + testServerName +
+			" KILL victim :byePRIVMSG victim :forged",
+		"ERROR :Closing Link:  (byePRIVMSG victim :forged)",
+	}
+
+	if !slices.Equal(lines, want) {
+		t.Errorf("got lines %q, want %q", lines, want)
+	}
+}

internal/ircserver/parser.go

@@ -90,7 +90,10 @@ func ParseMessage(line string) *Message {
 }
 
 // FormatMessage formats an IRC message into wire protocol
-// format (without the trailing CR-LF).
+// format (without the trailing CR-LF). CR, LF and NUL are
+// removed from each parameter, because on the wire they would
+// end the line early; this happens before deciding whether
+// the last parameter needs a leading ':'.
 func FormatMessage(
 	prefix, command string,
 	params ...string,
@@ -106,6 +109,10 @@ func FormatMessage(
 	buf.WriteString(command)
 
 	for i, param := range params {
+		param = strings.ReplaceAll(param, "\r", "")
+		param = strings.ReplaceAll(param, "\n", "")
+		param = strings.ReplaceAll(param, "\x00", "")
+
 		buf.WriteByte(' ')
 
 		isLast := i == len(params)-1

internal/ircserver/relay.go

@@ -3,6 +3,7 @@ package ircserver
 import (
 	"context"
 	"encoding/json"
+	"slices"
 	"strings"
 	"time"
 
@@ -105,7 +106,7 @@ func (c *Conn) deliverIRCMessage(
 	case isNumeric(command):
 		c.deliverNumeric(msg, text)
 	case command == irc.CmdPrivmsg || command == irc.CmdNotice:
-		c.deliverTextMessage(msg, command, text)
+		c.deliverTextMessage(msg, command, bodyLines)
 	case command == irc.CmdJoin:
 		c.deliverJoin(msg)
 	case command == irc.CmdPart:
@@ -123,7 +124,7 @@ func (c *Conn) deliverIRCMessage(
 	case command == irc.CmdWallops:
 		c.deliverWallops(msg, text)
 	case command == irc.CmdMode:
-		c.deliverMode(msg, text)
+		c.deliverMode(msg, bodyLines)
 	case command == irc.CmdPing:
 		// Server-originated PING — reply with PONG.
 		c.sendFromServer("PING", c.serverSfx)
@@ -179,10 +180,12 @@ func (c *Conn) deliverNumeric(
 	c.send(FormatMessage(from, msg.Command, allParams...))
 }
 
-// deliverTextMessage sends PRIVMSG or NOTICE.
+// deliverTextMessage sends PRIVMSG or NOTICE, one line for
+// each line of the body.
 func (c *Conn) deliverTextMessage(
 	msg *db.IRCMessage,
-	command, text string,
+	command string,
+	bodyLines []string,
 ) {
 	from := msg.From
 	target := msg.To
@@ -201,7 +204,9 @@ func (c *Conn) deliverTextMessage(
 		prefix = from + "!" + from + "@*"
 	}
 
-	c.send(FormatMessage(prefix, command, target, text))
+	for _, line := range bodyLines {
+		c.send(FormatMessage(prefix, command, target, line))
+	}
 }
 
 // deliverJoin sends a JOIN notification.
@@ -351,15 +356,26 @@ func (c *Conn) deliverWallops(
 	))
 }
 
-// deliverMode sends a MODE change notification.
+// deliverMode sends a MODE change notification. The first
+// body line is the change, possibly followed by its values
+// separated by spaces ("+o bob"); any further lines are
+// values ("+k", "secret"). Each goes out as its own
+// parameter, as IRC sends them.
 func (c *Conn) deliverMode(
 	msg *db.IRCMessage,
-	text string,
+	bodyLines []string,
 ) {
-	prefix := msg.From + "!" + msg.From + "@*"
-	target := msg.To
-
-	if text != "" {
-		c.send(FormatMessage(prefix, "MODE", target, text))
+	if len(bodyLines) == 0 || bodyLines[0] == "" {
+		return
 	}
+
+	prefix := msg.From + "!" + msg.From + "@*"
+
+	params := slices.Concat(
+		[]string{msg.To},
+		strings.Fields(bodyLines[0]),
+		bodyLines[1:],
+	)
+
+	c.send(FormatMessage(prefix, "MODE", params...))
 }

internal/ircserver/relay_test.go

@@ -0,0 +1,187 @@
+package ircserver_test
+
+import (
+	"encoding/json"
+	"slices"
+	"strings"
+	"testing"
+)
+
+// queueMessage queues a message for the user with nick
+// recipient the way the HTTP API sends one: stored once, then
+// queued to the recipient's session.
+func (env *testEnv) queueMessage(
+	t *testing.T,
+	recipient, command, from, to string,
+	body []string,
+) {
+	t.Helper()
+
+	ctx := t.Context()
+
+	bodyJSON, err := json.Marshal(body)
+	if err != nil {
+		t.Fatalf("marshal body: %v", err)
+	}
+
+	msgID, _, err := env.database.InsertMessage(
+		ctx, command, from, to, nil, bodyJSON, nil,
+	)
+	if err != nil {
+		t.Fatalf("insert message: %v", err)
+	}
+
+	sessionID, err := env.database.GetSessionByNick(ctx, recipient)
+	if err != nil {
+		t.Fatalf("get session of %s: %v", recipient, err)
+	}
+
+	err = env.database.EnqueueToSession(ctx, sessionID, msgID)
+	if err != nil {
+		t.Fatalf("enqueue: %v", err)
+	}
+
+	env.brk.Notify(sessionID)
+}
+
+// TestRelayMultiLineBody checks that each line of a PRIVMSG
+// or NOTICE body sent over the HTTP API reaches an IRC
+// listener user as its own line.
+func TestRelayMultiLineBody(t *testing.T) {
+	t.Parallel()
+
+	env := newTestEnv(t)
+
+	alice := env.dial(t)
+	alice.register("alice")
+
+	for _, command := range []string{"PRIVMSG", "NOTICE"} {
+		env.queueMessage(
+			t, "alice", command, "carol", "alice",
+			[]string{"first line", "second line"},
+		)
+
+		lines := alice.readUntil(func(l string) bool {
+			return strings.Contains(l, "second line")
+		})
+
+		want := []string{
+			":carol!carol@* " + command + " alice :first line",
+			":carol!carol@* " + command + " alice :second line",
+		}
+
+		got := lines[max(0, len(lines)-len(want)):]
+		if !slices.Equal(got, want) {
+			t.Errorf("%s: got %q, want %q", command, got, want)
+		}
+	}
+}
+
+// TestRelayModeParameters checks that a MODE change reaches
+// IRC listener members with its values as separate
+// parameters, both for the bodies the HTTP API stores and
+// for a change made on the IRC listener.
+func TestRelayModeParameters(t *testing.T) {
+	t.Parallel()
+
+	env := newTestEnv(t)
+
+	alice := env.dial(t)
+	alice.register("alice")
+
+	for _, step := range []struct {
+		body []string
+		want string
+	}{
+		{[]string{"+k", "secret"}, "+k secret"},
+		{[]string{"-k", "*"}, "-k *"},
+		{[]string{"+l", "50"}, "+l 50"},
+		{[]string{"-l"}, "-l"},
+		{[]string{"+b", "*!*@*.example.com"}, "+b *!*@*.example.com"},
+		{[]string{"-b", "*!*@*.example.com"}, "-b *!*@*.example.com"},
+		{[]string{"+o bob"}, "+o bob"},
+		{[]string{"+m"}, "+m"},
+	} {
+		env.queueMessage(t, "alice", "MODE", "op", "#modes", step.body)
+
+		lines := alice.readUntil(func(l string) bool {
+			return strings.Contains(l, " MODE ")
+		})
+
+		got := lines[len(lines)-1]
+		want := ":op!op@* MODE #modes " + step.want
+
+		if got != want {
+			t.Errorf("body %q: got %q, want %q", step.body, got, want)
+		}
+	}
+
+	bob := env.dial(t)
+	bob.register("bob")
+
+	alice.joinAndDrain("#modes")
+	bob.joinAndDrain("#modes")
+
+	alice.send("MODE #modes +v bob")
+
+	const change = " MODE #modes +v bob"
+
+	for name, client := range map[string]*testClient{
+		"alice": alice, "bob": bob,
+	} {
+		lines := client.readUntil(func(l string) bool {
+			return strings.Contains(l, " MODE ")
+		})
+
+		got := lines[len(lines)-1]
+		if !strings.HasSuffix(got, change) {
+			t.Errorf("%s: got %q, want it to end in %q", name, got, change)
+		}
+	}
+}
+
+// TestRelayLineBreaks checks that a CR, LF or NUL in a stored
+// body line, key or mask never reaches an IRC listener user
+// as a line break, so no extra line arrives, and that a body
+// line keeps its parameter and leading ':' once they are
+// removed.
+func TestRelayLineBreaks(t *testing.T) {
+	t.Parallel()
+
+	env := newTestEnv(t)
+
+	bob := env.dial(t)
+	bob.register("bob")
+
+	env.queueMessage(t, "bob", "PRIVMSG", "carol", "bob", []string{
+		"one\r\n:admin!a@b PRIVMSG bob :hi",
+		"two\n:admin!a@b NOTICE bob :hi\x00",
+	})
+	env.queueMessage(t, "bob", "MODE", "op", "#breaks",
+		[]string{"+k", "key\r\nERROR :Closing link"})
+	env.queueMessage(t, "bob", "MODE", "op", "#breaks",
+		[]string{"+b", "*!*@*\r\n:admin!a@b KICK #breaks bob"})
+	env.queueMessage(t, "bob", "PRIVMSG", "carol", "bob",
+		[]string{"\r\n", "\x00:hi"})
+	env.queueMessage(t, "bob", "PRIVMSG", "carol", "bob",
+		[]string{"the end"})
+
+	lines := bob.readUntil(func(l string) bool {
+		return strings.HasSuffix(l, " :the end")
+	})
+
+	want := []string{
+		":carol!carol@* PRIVMSG bob :one:admin!a@b PRIVMSG bob :hi",
+		":carol!carol@* PRIVMSG bob :two:admin!a@b NOTICE bob :hi",
+		":op!op@* MODE #breaks +k :keyERROR :Closing link",
+		":op!op@* MODE #breaks +b :*!*@*:admin!a@b KICK #breaks bob",
+		":carol!carol@* PRIVMSG bob :",
+		":carol!carol@* PRIVMSG bob ::hi",
+		":carol!carol@* PRIVMSG bob :the end",
+	}
+
+	got := lines[max(0, len(lines)-len(want)):]
+	if !slices.Equal(got, want) {
+		t.Errorf("got %q, want %q", got, want)
+	}
+}

internal/ircserver/server_test.go

@@ -438,6 +438,37 @@ func TestJoinChannel(t *testing.T) {
 	assertContains(t, lines, " 366 ", "RPL_ENDOFNAMES")
 }
 
+// TestJoinInvalidChannelName checks that JOIN refuses a name
+// that is not # followed by 1 to 63 letters, digits, _ or -
+// with ERR_NOSUCHCHANNEL, and creates no such channel.
+func TestJoinInvalidChannelName(t *testing.T) {
+	t.Parallel()
+
+	env := newTestEnv(t)
+	client := env.dial(t)
+
+	client.register("joiner")
+
+	for _, channel := range []string{
+		"#bad.name",
+		"#",
+		"#" + strings.Repeat("a", 64),
+	} {
+		lines := client.sendAndExpect("JOIN "+channel, " 403 ")
+
+		assertContains(
+			t, lines,
+			" 403 joiner "+channel+" :No such channel",
+			channel,
+		)
+
+		_, err := env.database.GetChannelByName(t.Context(), channel)
+		if err == nil {
+			t.Errorf("channel %q was created", channel)
+		}
+	}
+}
+
 func TestPrivmsgBetweenClients(t *testing.T) {
 	t.Parallel()
 

internal/service/service.go

@@ -10,6 +10,7 @@ import (
 	"errors"
 	"fmt"
 	"log/slog"
+	"regexp"
 	"strconv"
 	"strings"
 	"sync"
@@ -35,6 +36,12 @@ const (
 // for (RFC 2812).
 const maxUserhostNicks = 5
 
+// validChannelRe matches the channel names JOIN accepts: #
+// followed by 1 to 63 letters, digits, _ or -.
+var validChannelRe = regexp.MustCompile(
+	`^#[a-zA-Z0-9_\-]{1,63}$`,
+)
+
 // Params defines the dependencies for creating a Service.
 type Params struct {
 	fx.In
@@ -473,12 +480,21 @@ func (s *Service) SendDirectMessage(
 
 // JoinChannel creates or joins a channel, making the
 // first joiner the operator. Fans out the JOIN to all
-// channel members.
+// channel members. A channel name that validChannelRe
+// does not match gets ERR_NOSUCHCHANNEL.
 func (s *Service) JoinChannel(
 	ctx context.Context,
 	sessionID int64,
 	nick, channel, suppliedKey string,
 ) (*JoinResult, error) {
+	if !validChannelRe.MatchString(channel) {
+		return nil, &IRCError{
+			irc.ErrNoSuchChannel,
+			[]string{channel},
+			msgNoSuchChannel,
+		}
+	}
+
 	chID, err := s.db.GetOrCreateChannel(ctx, channel)
 	if err != nil {
 		return nil, fmt.Errorf("get/create channel: %w", err)