Show square brackets in the neoirc-cli message window as written (closes #233)

The message window has colour tags on, so tview read square-bracket
text in nicks, channel names, reasons, topics and message text as
tags. It dropped that text or changed the colour of the rest of the
line.

A new escape in internal/cli/ui.go writes each "[" as "[[:]", and
app.go applies it to every part of a line that comes from the user or
the server. tview.Escape is not used: it misses bracket pairs holding
symbols such as "|", and tags with a URL.

Lines for other server commands now show the command, such as
"[MODE]", which tview had read as a tag.

Model: opus-5-5

README.md

@@ -643,8 +643,10 @@ Send a message to a channel or user. This is the primary messaging command.
   message echoed back via the queue).
 - If `to` is a bare nick, the message is a DM. The server fans out to the
   recipient and the sender (so all of the sender's clients see the DM).
-- `body` must be a non-empty array of strings; otherwise the server sends
+- A `body` that is not an array of strings, or a `meta` that is not an object,
-  ERR_NOTEXTTOSEND (412). A missing `to` gets ERR_NORECIPIENT (411).
+  gets HTTP 400 `invalid request body`, and nothing is relayed. A missing or
+  empty `body` gets ERR_NOTEXTTOSEND (412). A missing `to` gets ERR_NORECIPIENT
+  (411).
 - Only channel members can send to a channel. A non-member, a banned user, and
   on a `+m` channel a user with neither `+o` nor `+v`, get ERR_CANNOTSENDTOCHAN
   (404).
@@ -1749,17 +1751,18 @@ command). **Numeric replies**, for success and for errors, are delivered through
 the message queue (see [Numeric Reply Codes](#numeric-reply-codes-s2c-only)); a
 command that has none, such as a successful `PRIVMSG`, is answered by the HTTP
 response alone, and `PING`'s `PONG` is the HTTP response body. HTTP error codes
-(4xx/5xx) are reserved for transport-level problems: malformed JSON (400),
+(4xx/5xx) are reserved for transport-level problems: a malformed request (400),
 missing/invalid auth cookies (401), and server errors (500).
 
 **HTTP errors (transport-level only):**
 
-| Status | Error                  | When                                                    |
+| Status | Error                  | When                                                                                              |
-| ------ | ---------------------- | ------------------------------------------------------- |
+| ------ | ---------------------- | ------------------------------------------------------------------------------------------------- |
-| 400    | `invalid request body` | Malformed JSON, or a body over `MAX_MESSAGE_SIZE` bytes |
+| 400    | `invalid request body` | Malformed JSON, or a body over `MAX_MESSAGE_SIZE` bytes                                           |
-| 400    | `command required`     | Empty command                                           |
+| 400    | `invalid request body` | A `PRIVMSG` or `NOTICE` whose `body` is not an array of strings, or whose `meta` is not an object |
-| 401    | `not registered`       | Missing or invalid auth cookie                          |
+| 400    | `command required`     | Empty command                                                                                     |
-| 500    | `internal error`       | Server-side failure                                     |
+| 401    | `not registered`       | Missing or invalid auth cookie                                                                    |
+| 500    | `internal error`       | Server-side failure                                                                               |
 
 **IRC numeric error replies (delivered via message queue):**
 

internal/cli/app.go

@@ -402,10 +402,16 @@ func (a *App) cmdQuery(nick string) {
 	a.mu.Lock()
 	a.target = nick
 	myNick := a.nick
+	connected := a.connected
 	a.mu.Unlock()
 
+	connStatus := "disconnected"
+	if connected {
+		connStatus = "connected"
+	}
+
 	a.ui.SwitchToBuffer(nick)
-	a.ui.SetStatus(myNick, nick, "connected")
+	a.ui.SetStatus(myNick, nick, connStatus)
 }
 
 func (a *App) cmdTopic(args string) {

internal/cli/app_internal_test.go

@@ -24,6 +24,13 @@ const (
 	testText      = "hello"
 )
 
+// What the status bar shows while the client is connected, and
+// while it is not.
+const (
+	statusConnected    = "[connected]"
+	statusDisconnected = "[disconnected]"
+)
+
 // eventLoopTimeout is how long a test waits for a typed line to
 // be handled, for the welcome lines, for the UI to stop, for a poll
 // or for a poll loop to end, before it fails.
@@ -362,6 +369,46 @@ func TestWindowConnectionState(t *testing.T) {
 	}
 }
 
+// TestQueryConnectionState checks that typing /query bob leaves the
+// status bar showing [connected] only while the client is connected,
+// and [disconnected] otherwise.
+func TestQueryConnectionState(t *testing.T) {
+	t.Parallel()
+
+	const line = "/query " + testOtherNick
+
+	tests := []struct {
+		name      string
+		connected bool
+		want      string
+	}{
+		{name: "not connected", connected: false, want: statusDisconnected},
+		{name: "connected", connected: true, want: statusConnected},
+	}
+
+	for _, test := range tests {
+		t.Run(test.name, func(t *testing.T) {
+			t.Parallel()
+
+			app := &App{ui: NewUI(), nick: testNick, connected: test.connected}
+			handled := handledLines(app)
+			runUI(t, app.ui)
+
+			for _, r := range line {
+				app.ui.app.QueueEvent(tcell.NewEventKey(tcell.KeyRune, r, tcell.ModNone))
+			}
+
+			app.ui.app.QueueEvent(tcell.NewEventKey(tcell.KeyEnter, 0, tcell.ModNone))
+			waitHandled(t, handled, line)
+
+			status := statusBarText(app.ui)
+			if !strings.Contains(status, test.want) {
+				t.Fatalf("status bar is %q, want it to show %s", status, test.want)
+			}
+		})
+	}
+}
+
 // TestEventLoopKeepsRunning sends Alt and 1, then a typed line and
 // Enter, to a running UI. It checks that both are handled, so the
 // event loop did not stop on either.
@@ -847,12 +894,14 @@ func TestOwnEventFromOtherClient(t *testing.T) {
 }
 
 // TestBracketsInMessageWindow checks that each kind of line drawn
-// from a server event shows nicks, texts, reasons and topics
+// from a server event shows nicks, channel names, texts, reasons
-// holding square brackets as written.
+// and topics holding square brackets as written.
 func TestBracketsInMessageWindow(t *testing.T) {
 	t.Parallel()
 
-	// Every event is sent to the channel and has a body of one line.
+	// Every event is sent to this channel and has a body of one line.
+	const channel = "#chan[x]"
+
 	tests := []struct {
 		name    string
 		command string
@@ -871,12 +920,12 @@ func TestBracketsInMessageWindow(t *testing.T) {
 		},
 		{
 			name: "joined", command: irc.CmdJoin, from: "x[^[]", inChannel: true,
-			want: "*** x[^[] has joined #chan",
+			want: "*** x[^[] has joined #chan[x]",
 		},
 		{
 			name: "left", command: irc.CmdPart, inChannel: true,
 			from: "erin[away]", body: "[:::y|z]",
-			want: "*** erin[away] has left #chan ([:::y|z])",
+			want: "*** erin[away] has left #chan[x] ([:::y|z])",
 		},
 		{
 			name: "topic", command: irc.CmdTopic, inChannel: true,
@@ -887,13 +936,17 @@ func TestBracketsInMessageWindow(t *testing.T) {
 			name: "quit", command: irc.CmdQuit, from: "carol[x]", body: "bye [red]",
 			want: "*** carol[x] has quit (bye [red])",
 		},
+		{
+			name: "quit without reason", command: irc.CmdQuit, from: "frank[x]",
+			want: "*** frank[x] has quit",
+		},
 		{
 			name: "nick change", command: irc.CmdNick, from: "erin[m]", body: "erin[x]",
 			want: "*** erin[m] is now known as erin[x]",
 		},
 		{
-			name: "notice", command: irc.CmdNotice, from: "bob[]", body: "[::b]note",
+			name: "notice", command: irc.CmdNotice, from: "bob[x]", body: "[::b]note",
-			want: "--bob[]-- [::b]note",
+			want: "--bob[x]-- [::b]note",
 		},
 		{
 			name: "other command", command: irc.CmdMode, body: "+v dave[afk]",
@@ -909,15 +962,160 @@ func TestBracketsInMessageWindow(t *testing.T) {
 			screen := runUI(t, app.ui)
 
 			if test.inChannel {
-				app.ui.SwitchToBuffer(testChannel)
+				app.ui.SwitchToBuffer(channel)
 			}
 
 			app.handleServerMessage(&api.Message{
-				Command: test.command, From: test.from, To: testChannel,
+				Command: test.command, From: test.from, To: channel,
 				Body: []string{test.body},
 			})
 
-			got, _ := shownLine(app.ui, screen)
+			got, _ := shownLine(app.ui, screen, 0)
+			if !strings.HasSuffix(got, test.want) {
+				t.Fatalf("message window shows %q, want it to end %q", got, test.want)
+			}
+		})
+	}
+}
+
+// TestBracketsInCommandErrors checks that when the server refuses
+// a typed command, the error the client shows holds the server's
+// answer as written.
+func TestBracketsInCommandErrors(t *testing.T) {
+	t.Parallel()
+
+	server := newRefusingServer(t)
+
+	tests := []struct {
+		typed string
+		// The start of the line, before the error.
+		reply string
+	}{
+		{typed: testText, reply: "Send error"},
+		{typed: "/nick bob", reply: "Nick change failed"},
+		{typed: "/join #x", reply: "Join failed"},
+		{typed: "/part", reply: "Part failed"},
+		{typed: "/msg bob hi", reply: "Send failed"},
+		{typed: "/topic hi", reply: "Topic set failed"},
+		{typed: "/names", reply: "Names failed"},
+		{typed: "/list", reply: "List failed"},
+		{typed: "/motd", reply: "MOTD failed"},
+		{typed: "/who", reply: "WHO failed"},
+		{typed: "/whois bob", reply: "WHOIS failed"},
+	}
+
+	for _, test := range tests {
+		t.Run(test.reply, func(t *testing.T) {
+			t.Parallel()
+
+			app := &App{
+				ui:        NewUI(),
+				client:    api.NewClient(server.URL),
+				nick:      testNick,
+				target:    testChannel,
+				connected: true,
+			}
+			screen := runUI(t, app.ui)
+
+			app.handleInput(test.typed)
+
+			want := test.reply + ": HTTP error 400: [red]no"
+
+			got, _ := shownLine(app.ui, screen, 0)
+			if !strings.HasSuffix(got, want) {
+				t.Fatalf("message window shows %q, want it to end %q", got, want)
+			}
+		})
+	}
+}
+
+// TestBracketsInCommandReplies checks that the other lines the
+// client draws for a typed command show square brackets as
+// written, whether they come from the typed line or the server.
+func TestBracketsInCommandReplies(t *testing.T) {
+	t.Parallel()
+
+	const channel = "#chan[x]"
+
+	refusing := newRefusingServer(t)
+	session := newServer(t, `{"id": 1, "nick": "erin[x]"}`)
+
+	tests := []struct {
+		name string
+		// The server the client is connected to before the line is
+		// typed, if any.
+		server *httptest.Server
+		typed  string
+		// Whether the line goes to the channel's window rather than
+		// the status window, and which line of the window it is,
+		// counting from 0.
+		inChannel bool
+		line      int
+		want      string
+	}{
+		{name: "unknown command", typed: "/x[red]", want: "Unknown command: /x[red]"},
+		{
+			name: "nick set", typed: "/nick erin[x]",
+			want: "Nick set to erin[x] (will be used on connect)",
+		},
+		{
+			name: "nick changed", server: session, typed: "/nick erin[x]",
+			want: "Nick changed to erin[x]",
+		},
+		{
+			name: "connecting", typed: "/connect " + refusing.URL + "/[x]",
+			want: "Connecting to " + refusing.URL + "/[x]...",
+		},
+		{
+			name: "connection failed", typed: "/connect " + refusing.URL, line: 1,
+			want: "Connection failed: HTTP error 400: [red]no",
+		},
+		{
+			name: "connected", typed: "/connect " + session.URL, line: 1,
+			want: "Connected! Nick: erin[x], Session: 1",
+		},
+		{
+			name: "members", server: newServer(t, `[{"nick": "bob[x]"}]`),
+			typed: "/names", inChannel: true, want: "*** Members of #chan[x]: bob[x]",
+		},
+		{
+			name: "channel list", typed: "/list", line: 1, want: "  #chan[x] [red]news",
+			server: newServer(t, `[{"name": "#chan[x]", "topic": "[red]news"}]`),
+		},
+	}
+
+	for _, test := range tests {
+		t.Run(test.name, func(t *testing.T) {
+			t.Parallel()
+
+			app := &App{ui: NewUI(), nick: testNick, target: channel}
+
+			if test.server != nil {
+				app.client = api.NewClient(test.server.URL)
+				app.connected = true
+			}
+
+			screen := runUI(t, app.ui)
+
+			// Stop the poll loop a /connect starts, as /quit does.
+			// Cleanups run last first, so this runs before the UI
+			// stops.
+			t.Cleanup(func() {
+				app.mu.Lock()
+				defer app.mu.Unlock()
+
+				if app.stopPoll != nil {
+					close(app.stopPoll)
+				}
+			})
+
+			if test.inChannel {
+				app.ui.SwitchToBuffer(channel)
+			}
+
+			app.handleInput(test.typed)
+
+			got, _ := shownLine(app.ui, screen, test.line)
 			if !strings.HasSuffix(got, test.want) {
 				t.Fatalf("message window shows %q, want it to end %q", got, test.want)
 			}
@@ -942,7 +1140,7 @@ func TestColourTagInMessage(t *testing.T) {
 		Body: []string{"[red]x"},
 	})
 
-	got, colour := shownLine(app.ui, screen)
+	got, colour := shownLine(app.ui, screen, 0)
 	if !strings.HasSuffix(got, want) || colour != tcell.ColorWhite {
 		t.Fatalf("message window shows %q, its last character in %v; "+
 			"want it to end %q, in white", got, colour, want)
@@ -976,11 +1174,12 @@ func statusBarText(ui *UI) string {
 	return text
 }
 
-// shownLine returns the first line of the message window as the
+// shownLine returns the given line of the message window, counting
-// screen shows it, without the blanks after it, and the colour of
+// from 0, as the screen shows it, without the blanks after it, and
-// its last character.
+// the colour of its last character. Each line in these tests takes
+// one row of the screen.
 func shownLine(
-	ui *UI, screen tcell.SimulationScreen,
+	ui *UI, screen tcell.SimulationScreen, line int,
 ) (string, tcell.Color) {
 	var (
 		text   string
@@ -989,10 +1188,11 @@ func shownLine(
 
 	ui.app.QueueUpdate(func() {
 		cells, width, _ := screen.GetContents()
+		cells = cells[line*width : (line+1)*width]
 
 		var row strings.Builder
 
-		for _, cell := range cells[:width] {
+		for _, cell := range cells {
 			row.WriteString(string(cell.Runes))
 		}
 
@@ -1054,6 +1254,23 @@ func newServer(t *testing.T, body string) *httptest.Server {
 	return server
 }
 
+// newRefusingServer starts a server that answers every request
+// with the status 400 Bad Request and the body "[red]no" until
+// the test ends. The client shows that body in its error.
+func newRefusingServer(t *testing.T) *httptest.Server {
+	t.Helper()
+
+	server := httptest.NewServer(http.HandlerFunc(
+		func(writer http.ResponseWriter, _ *http.Request) {
+			writer.WriteHeader(http.StatusBadRequest)
+			_, _ = io.WriteString(writer, "[red]no")
+		},
+	))
+	t.Cleanup(server.Close)
+
+	return server
+}
+
 // newHoldingServer starts a server that sends on the returned
 // channel for each poll and holds the poll until its connection
 // closes. It answers any other request with body.

internal/handlers/api.go

@@ -994,7 +994,7 @@ func (hdlr *Handlers) dispatchCommand(
 		hdlr.handlePrivmsg(
 			writer, request,
 			sessionID, clientID, nick,
-			command, target, body, meta, bodyLines,
+			command, target, body, meta,
 		)
 	case irc.CmdJoin:
 		hdlr.handleJoin(
@@ -1139,8 +1139,18 @@ func (hdlr *Handlers) handlePrivmsg(
 	nick, command, target string,
 	body json.RawMessage,
 	meta json.RawMessage,
-	bodyLines func() []string,
 ) {
+	lines, ok := checkBodyAndMeta(body, meta)
+	if !ok {
+		hdlr.respondError(
+			writer, request,
+			"invalid request body",
+			http.StatusBadRequest,
+		)
+
+		return
+	}
+
 	if target == "" {
 		hdlr.enqueueNumeric(
 			request.Context(), clientID,
@@ -1155,7 +1165,6 @@ func (hdlr *Handlers) handlePrivmsg(
 		return
 	}
 
-	lines := bodyLines()
 	if len(lines) == 0 {
 		hdlr.enqueueNumeric(
 			request.Context(), clientID,
@@ -1189,6 +1198,44 @@ func (hdlr *Handlers) handlePrivmsg(
 	)
 }
 
+// checkBodyAndMeta returns the lines in body. It returns false
+// when body is present and is not an array of strings, or meta
+// is present and is not an object. Each line is decoded as any
+// because decoding into []string turns a null line into "". A
+// null body or meta decodes without an error into nil.
+func checkBodyAndMeta(body, meta json.RawMessage) ([]string, bool) {
+	var lines []string
+
+	if body != nil {
+		var values []any
+
+		err := json.Unmarshal(body, &values)
+		if err != nil || values == nil {
+			return nil, false
+		}
+
+		for _, value := range values {
+			line, isString := value.(string)
+			if !isString {
+				return nil, false
+			}
+
+			lines = append(lines, line)
+		}
+	}
+
+	if meta != nil {
+		var fields map[string]any
+
+		err := json.Unmarshal(meta, &fields)
+		if err != nil || fields == nil {
+			return nil, false
+		}
+	}
+
+	return lines, true
+}
+
 // respondIRCError enqueues a numeric error reply, notifies
 // the broker, and sends HTTP 200 with {"status":"error"}.
 func (hdlr *Handlers) respondIRCError(

internal/handlers/api_test.go

@@ -8,10 +8,12 @@ import (
 	"errors"
 	"fmt"
 	"io"
+	"maps"
 	"net/http"
 	"net/http/httptest"
 	"net/url"
 	"os"
+	"reflect"
 	"slices"
 	"strconv"
 	"strings"
@@ -999,6 +1001,8 @@ func TestChannelMessage(t *testing.T) {
 	}
 }
 
+// TestMessageMissingBody checks that a PRIVMSG with no body,
+// or an empty one, gets ERR_NOTEXTTOSEND.
 func TestMessageMissingBody(t *testing.T) {
 	t.Parallel()
 
@@ -1013,20 +1017,25 @@ func TestMessageMissingBody(t *testing.T) {
 
 	_, lastID := tserver.pollMessages(token, 0)
 
-	status, _ := tserver.sendCommand(token, map[string]any{
+	for _, request := range []map[string]any{
-		commandKey: privmsgCmd, toKey: channel,
+		{commandKey: privmsgCmd, toKey: channel},
-	})
+		{commandKey: privmsgCmd, toKey: channel, bodyKey: []string{}},
-	if status != http.StatusOK {
+	} {
-		t.Fatalf("expected 200, got %d", status)
+		status, _ := tserver.sendCommand(token, request)
-	}
+		if status != http.StatusOK {
+			t.Fatalf("%v: expected 200, got %d", request, status)
+		}
 
-	msgs, _ := tserver.pollMessages(token, lastID)
+		var msgs []map[string]any
 
-	if !findNumeric(msgs, "412") {
+		msgs, lastID = tserver.pollMessages(token, lastID)
-		t.Fatalf(
+
-			"expected ERR_NOTEXTTOSEND (412), got %v",
+		if !findNumeric(msgs, "412") {
-			msgs,
+			t.Fatalf(
-		)
+				"%v: expected ERR_NOTEXTTOSEND (412), got %v",
+				request, msgs,
+			)
+		}
 	}
 }
 
@@ -1056,6 +1065,132 @@ func TestMessageMissingTo(t *testing.T) {
 	}
 }
 
+// TestMessageInvalidBodyOrMeta checks that a PRIVMSG or
+// NOTICE, to a channel or a nick, whose body is not an array
+// of strings or whose meta is not an object gets 400 and
+// reaches nobody.
+func TestMessageInvalidBodyOrMeta(t *testing.T) {
+	t.Parallel()
+
+	cases := []struct {
+		name   string
+		fields map[string]any
+	}{
+		{"null line", map[string]any{bodyKey: []any{"hi", nil}}},
+		{"number line", map[string]any{bodyKey: []any{"hi", 1}}},
+		{"object line", map[string]any{bodyKey: []any{map[string]any{}}}},
+		{"null body", map[string]any{bodyKey: nil}},
+		{"object body", map[string]any{bodyKey: map[string]any{}}},
+		{"null meta", map[string]any{bodyKey: []string{"hi"}, metaKey: nil}},
+		{"array meta", map[string]any{bodyKey: []string{"hi"}, metaKey: []string{}}},
+		{"string meta", map[string]any{bodyKey: []string{"hi"}, metaKey: "hi"}},
+	}
+
+	for _, tc := range cases {
+		t.Run(tc.name, func(t *testing.T) {
+			t.Parallel()
+
+			const channel = "#badmsg"
+
+			tserver := newTestServer(t)
+			aliceToken := tserver.createSession("alice_badmsg")
+			bobToken := tserver.createSession("bob_badmsg")
+
+			tserver.sendCommand(aliceToken, map[string]any{
+				commandKey: joinCmd, toKey: channel,
+			})
+			tserver.sendCommand(bobToken, map[string]any{
+				commandKey: joinCmd, toKey: channel,
+			})
+
+			_, bobLastID := tserver.pollMessages(bobToken, 0)
+
+			for _, command := range []string{privmsgCmd, irc.CmdNotice} {
+				for _, target := range []string{channel, "bob_badmsg"} {
+					request := map[string]any{
+						commandKey: command, toKey: target,
+					}
+					maps.Copy(request, tc.fields)
+
+					status, result := tserver.sendCommand(
+						aliceToken, request,
+					)
+					if status != http.StatusBadRequest ||
+						result["error"] != "invalid request body" {
+						t.Errorf(
+							"%s to %s: expected 400 invalid "+
+								"request body, got %d: %v",
+							command, target, status, result,
+						)
+					}
+				}
+			}
+
+			msgs, _ := tserver.pollMessages(bobToken, bobLastID)
+			if len(msgs) != 0 {
+				t.Fatalf("expected no messages, bob got %v", msgs)
+			}
+		})
+	}
+}
+
+// TestMessageRelaysBodyAndMeta checks that a PRIVMSG or
+// NOTICE, to a channel or a nick, reaches its recipient with
+// body and meta as sent.
+func TestMessageRelaysBodyAndMeta(t *testing.T) {
+	t.Parallel()
+
+	const channel = "#relaymsg"
+
+	tserver := newTestServer(t)
+	aliceToken := tserver.createSession("alice_relaymsg")
+	bobToken := tserver.createSession("bob_relaymsg")
+
+	tserver.sendCommand(aliceToken, map[string]any{
+		commandKey: joinCmd, toKey: channel,
+	})
+	tserver.sendCommand(bobToken, map[string]any{
+		commandKey: joinCmd, toKey: channel,
+	})
+
+	_, bobLastID := tserver.pollMessages(bobToken, 0)
+
+	body := []any{"line one", ""}
+	meta := map[string]any{"sig": "abc", "alg": "ed25519"}
+
+	for _, command := range []string{privmsgCmd, irc.CmdNotice} {
+		for _, target := range []string{channel, "bob_relaymsg"} {
+			status, result := tserver.sendCommand(
+				aliceToken,
+				map[string]any{
+					commandKey: command, toKey: target,
+					bodyKey: body, metaKey: meta,
+				},
+			)
+			if status != http.StatusOK {
+				t.Fatalf(
+					"%s to %s: expected 200, got %d: %v",
+					command, target, status, result,
+				)
+			}
+
+			var msgs []map[string]any
+
+			msgs, bobLastID = tserver.pollMessages(
+				bobToken, bobLastID,
+			)
+			if len(msgs) != 1 ||
+				msgs[0][commandKey] != command ||
+				!reflect.DeepEqual(msgs[0][bodyKey], body) ||
+				!reflect.DeepEqual(msgs[0][metaKey], meta) {
+				t.Fatalf(
+					"%s to %s: bob got %v", command, target, msgs,
+				)
+			}
+		}
+	}
+}
+
 func TestNonMemberCannotSend(t *testing.T) {
 	t.Parallel()