Fix IRC listener relay of bodies and modes, and JOIN names (closes #127)

The IRC listener relayed only the first line of a PRIVMSG or NOTICE
body; it now sends one line per body line. A MODE change reached IRC
listener members without its key, limit or mask, and "+o bob" went out
as one trailing parameter; each value is now its own parameter, in the
relay and in the echo to the IRC listener user who made the change.
Every line written to an IRC listener connection now has CR, LF and NUL
removed, so a stored body line, key or mask cannot start a line of its
own. The channel name check moved from the HTTP handler into
service.JoinChannel, so the IRC listener's JOIN refuses the same names
with 403.

Model: opus-5-5

internal/handlers/api.go

@@ -28,10 +28,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}$`,
 )
@@ -1488,16 +1484,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,19 @@ 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:
+// stored text can hold them, and on the wire they would end
+// the line early. They are removed, not replaced with a
+// space, because the line is already formatted and a space
+// would split a parameter in two.
 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/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,181 @@
+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.
+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{"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 :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)