Check nicks on the IRC listener with the HTTP API's rule (closes #142)

The nick rule moves from internal/handlers to internal/service as
ValidNick, and the HTTP API and both IRC listener NICK paths call it.
The IRC listener used to cut a nick to 32 bytes and accept anything
else, so a nick with spaces such as "bob PRIVMSG #inj :hi" made the
lines built from it read as another user's PRIVMSG. It now answers a
nick the rule refuses with ERR_ERRONEUSNICKNAME (432), before and
after registration, and keeps the nick as it was.

Model: opus-5-5

README.md

@@ -249,9 +249,11 @@ what channel bans (`+b` mode) match against.
   hold the same nick simultaneously.
 - Nicks are **case-sensitive** (unlike traditional IRC). `Alice` and `alice` are
   different nicks.
-- Nick length: 1–32 characters. Over the HTTP API a nick starts with a letter or
+- Nick length: 1–32 characters. A nick starts with a letter or `_`, followed by
-  `_`, followed by letters, digits and `` _-[]\^{}|` ``. The IRC listener cuts a
+  letters, digits and `` _-[]\^{}|` ``. The HTTP API and the IRC listener both
-  longer nick to 32 bytes and checks nothing else.
+  apply this rule. On the IRC listener, a `NICK` with any other nick, before or
+  after registration, gets ERR_ERRONEUSNICKNAME (432) and leaves the nick as it
+  was.
 - Nicks are **released when a session is destroyed** (via `QUIT` command or
   session expiry). There is no nick registration or reservation system.
 - Nick changes are broadcast to all users sharing a channel with the changer, as

internal/handlers/api.go

@@ -19,10 +19,6 @@ import (
 	"sneak.berlin/go/neoirc/pkg/irc"
 )
 
-var validNickRe = regexp.MustCompile(
-	`^[a-zA-Z_][a-zA-Z0-9_\-\[\]\\^{}|` + "`" + `]{0,31}$`,
-)
-
 var validUsernameRe = regexp.MustCompile(
 	`^[a-zA-Z0-9_\-\[\]\\^{}|` + "`" + `]{1,32}$`,
 )
@@ -211,7 +207,7 @@ func (hdlr *Handlers) handleCreateSession(
 
 	payload.Nick = strings.TrimSpace(payload.Nick)
 
-	if !validNickRe.MatchString(payload.Nick) {
+	if !service.ValidNick(payload.Nick) {
 		hdlr.respondError(
 			writer, request,
 			"invalid nick format",
@@ -1551,7 +1547,7 @@ func (hdlr *Handlers) handleNick(
 
 	newNick := strings.TrimSpace(lines[0])
 
-	if !validNickRe.MatchString(newNick) {
+	if !service.ValidNick(newNick) {
 		hdlr.respondIRCError(
 			writer, request, clientID, sessionID,
 			irc.ErrErroneusNickname, nick, []string{newNick},

internal/ircserver/commands.go

@@ -71,8 +71,13 @@ func (c *Conn) handleNick(
 	}
 
 	newNick := msg.Params[0]
-	if len(newNick) > maxNickLen {
+	if !service.ValidNick(newNick) {
-		newNick = newNick[:maxNickLen]
+		c.sendNumeric(
+			irc.ErrErroneusNickname,
+			newNick, "Erroneous nickname",
+		)
+
+		return
 	}
 
 	oldMask := c.hostmask()

internal/ircserver/conn.go

@@ -27,7 +27,6 @@ const (
 	pollInterval    = 100 * time.Millisecond
 	pingInterval    = 90 * time.Second
 	pongDeadline    = 30 * time.Second
-	maxNickLen      = 32
 	minPasswordLen  = 8
 	maxHashcashBits = 40
 )
@@ -426,11 +425,16 @@ func (c *Conn) handlePreRegistration(
 			return
 		}
 
-		c.nick = msg.Params[0]
+		if !service.ValidNick(msg.Params[0]) {
-		if len(c.nick) > maxNickLen {
+			c.sendNumeric(
-			c.nick = c.nick[:maxNickLen]
+				irc.ErrErroneusNickname,
+				msg.Params[0], "Erroneous nickname",
+			)
+
+			return
 		}
 
+		c.nick = msg.Params[0]
 		c.gotNick = true
 	case irc.CmdUser:
 		if len(msg.Params) < 4 { //nolint:mnd

internal/ircserver/server_test.go

@@ -519,6 +519,68 @@ func TestDuplicateNick(t *testing.T) {
 	assertContains(t, lines, " 433 ", "ERR_NICKNAMEINUSE")
 }
 
+// TestNickRefused checks that NICK, before and after
+// registration, answers a nick the HTTP API refuses with
+// ERR_ERRONEUSNICKNAME and keeps the nick as it was, and
+// that another member of the channel sees no line from it.
+func TestNickRefused(t *testing.T) {
+	t.Parallel()
+
+	// Each is what follows "NICK " on the wire.
+	refused := []string{
+		":bob PRIVMSG #inj :hi", // contains spaces
+		":",                     // empty
+		strings.Repeat("a", 33), // one character too long
+		"bob!x",
+		"bob@x",
+		"#bob",
+	}
+
+	env := newTestEnv(t)
+
+	// Before registration: the 432 is addressed to the nick
+	// sent first, and USER registers with that nick.
+	early := env.dial(t)
+	early.send("NICK early")
+
+	for _, param := range refused {
+		lines := early.sendAndExpect("NICK "+param, " 432 ")
+		assertContains(t, lines, " 432 early ", "before: "+param)
+	}
+
+	welcome := early.sendAndExpect("USER early 0 * :Test", " 001 ")
+	assertContains(t, welcome, " 001 early ", "registered as early")
+
+	// After registration.
+	bob := env.dial(t)
+	bob.register("bob")
+	bob.joinAndDrain("#inj")
+
+	mallory := env.dial(t)
+	mallory.register("mallory")
+	mallory.joinAndDrain("#inj")
+
+	bob.readUntil(func(l string) bool {
+		return strings.Contains(l, "JOIN") &&
+			strings.Contains(l, "mallory")
+	})
+
+	for _, param := range refused {
+		lines := mallory.sendAndExpect("NICK "+param, " 432 ")
+		assertContains(t, lines, " 432 mallory ", "after: "+param)
+	}
+
+	mallory.send("PRIVMSG #inj :still mallory")
+
+	bobLines := bob.readUntil(func(l string) bool {
+		return strings.Contains(l, "still mallory")
+	})
+	assertNotContains(t, bobLines, " NICK ", "bob sees no NICK")
+	assertContains(
+		t, bobLines, ":mallory!", "bob sees mallory's message",
+	)
+}
+
 func TestListChannels(t *testing.T) {
 	t.Parallel()
 

internal/service/service.go

@@ -44,6 +44,18 @@ var validChannelRe = regexp.MustCompile(
 	`^#[a-zA-Z0-9_\-]{1,63}$`,
 )
 
+// validNickRe matches the nicks both transports accept: a
+// letter or _, then up to 31 letters, digits or
+// _-[]\^{}|`.
+var validNickRe = regexp.MustCompile(
+	`^[a-zA-Z_][a-zA-Z0-9_\-\[\]\\^{}|` + "`" + `]{0,31}$`,
+)
+
+// ValidNick reports whether validNickRe matches nick.
+func ValidNick(nick string) bool {
+	return validNickRe.MatchString(nick)
+}
+
 // Params defines the dependencies for creating a Service.
 type Params struct {
 	fx.In