diff --git a/internal/cli/app.go b/internal/cli/app.go index 659c9e2..c6926ec 100644 --- a/internal/cli/app.go +++ b/internal/cli/app.go @@ -108,7 +108,7 @@ func (a *App) handleInput(text string) { }) if err != nil { a.ui.AddStatus( - "[red]Send error: " + err.Error(), + "[red]Send error: " + escape(err.Error()), ) } } @@ -166,7 +166,7 @@ func (a *App) dispatchInfoCommand(cmd, args string) { a.cmdWhois(args) default: a.ui.AddStatus( - "[red]Unknown command: " + cmd, + "[red]Unknown command: " + escape(cmd), ) } } @@ -182,7 +182,7 @@ func (a *App) cmdConnect(serverURL string) { serverURL = strings.TrimRight(serverURL, "/") - a.ui.AddStatus("Connecting to " + serverURL + "...") + a.ui.AddStatus("Connecting to " + escape(serverURL) + "...") a.mu.Lock() nick := a.nick @@ -192,9 +192,9 @@ func (a *App) cmdConnect(serverURL string) { resp, err := client.CreateSession(nick) if err != nil { - a.ui.AddStatus(fmt.Sprintf( - "[red]Connection failed: %v", err, - )) + a.ui.AddStatus( + "[red]Connection failed: " + escape(err.Error()), + ) return } @@ -218,7 +218,7 @@ func (a *App) cmdConnect(serverURL string) { a.ui.AddStatus(fmt.Sprintf( "[green]Connected! Nick: %s, Session: %d", - resp.Nick, resp.ID, + escape(resp.Nick), resp.ID, )) a.ui.SetStatus(resp.Nick, "", "connected") @@ -244,7 +244,7 @@ func (a *App) cmdNick(nick string) { a.mu.Unlock() a.ui.AddStatus( - "Nick set to " + nick + + "Nick set to " + escape(nick) + " (will be used on connect)", ) @@ -256,9 +256,9 @@ func (a *App) cmdNick(nick string) { Body: []string{nick}, }) if err != nil { - a.ui.AddStatus(fmt.Sprintf( - "[red]Nick change failed: %v", err, - )) + a.ui.AddStatus( + "[red]Nick change failed: " + escape(err.Error()), + ) return } @@ -269,7 +269,7 @@ func (a *App) cmdNick(nick string) { a.mu.Unlock() a.ui.SetStatus(nick, target, "connected") - a.ui.AddStatus("Nick changed to " + nick) + a.ui.AddStatus("Nick changed to " + escape(nick)) } func (a *App) cmdJoin(channel string) { @@ -297,9 +297,9 @@ func (a *App) cmdJoin(channel string) { err := a.client.JoinChannel(channel) if err != nil { - a.ui.AddStatus(fmt.Sprintf( - "[red]Join failed: %v", err, - )) + a.ui.AddStatus( + "[red]Join failed: " + escape(err.Error()), + ) return } @@ -337,9 +337,9 @@ func (a *App) cmdPart(channel string) { err := a.client.PartChannel(channel) if err != nil { - a.ui.AddStatus(fmt.Sprintf( - "[red]Part failed: %v", err, - )) + a.ui.AddStatus( + "[red]Part failed: " + escape(err.Error()), + ) return } @@ -384,9 +384,9 @@ func (a *App) cmdMsg(args string) { Body: []string{text}, }) if err != nil { - a.ui.AddStatus(fmt.Sprintf( - "[red]Send failed: %v", err, - )) + a.ui.AddStatus( + "[red]Send failed: " + escape(err.Error()), + ) } } @@ -445,9 +445,9 @@ func (a *App) cmdTopic(args string) { Body: []string{args}, }) if err != nil { - a.ui.AddStatus(fmt.Sprintf( - "[red]Topic set failed: %v", err, - )) + a.ui.AddStatus( + "[red]Topic set failed: " + escape(err.Error()), + ) } } @@ -471,9 +471,9 @@ func (a *App) cmdNames() { members, err := a.client.GetMembers(target) if err != nil { - a.ui.AddStatus(fmt.Sprintf( - "[red]Names failed: %v", err, - )) + a.ui.AddStatus( + "[red]Names failed: " + escape(err.Error()), + ) return } @@ -486,7 +486,7 @@ func (a *App) cmdNames() { a.ui.AddLine(target, fmt.Sprintf( "[cyan]*** Members of %s: %s", - target, strings.Join(nicks, " "), + escape(target), escape(strings.Join(nicks, " ")), )) } @@ -517,9 +517,9 @@ func (a *App) cmdList() { channels, err := a.client.ListChannels() if err != nil { - a.ui.AddStatus(fmt.Sprintf( - "[red]List failed: %v", err, - )) + a.ui.AddStatus( + "[red]List failed: " + escape(err.Error()), + ) return } @@ -528,7 +528,7 @@ func (a *App) cmdList() { for _, ch := range channels { a.ui.AddStatus(fmt.Sprintf( - " %s %s", ch.Name, ch.Topic, + " %s %s", escape(ch.Name), escape(ch.Topic), )) } @@ -550,9 +550,9 @@ func (a *App) cmdMotd() { &api.Message{Command: irc.CmdMotd}, //nolint:exhaustruct ) if err != nil { - a.ui.AddStatus(fmt.Sprintf( - "[red]MOTD failed: %v", err, - )) + a.ui.AddStatus( + "[red]MOTD failed: " + escape(err.Error()), + ) } } @@ -588,9 +588,9 @@ func (a *App) cmdWho(args string) { }, ) if err != nil { - a.ui.AddStatus(fmt.Sprintf( - "[red]WHO failed: %v", err, - )) + a.ui.AddStatus( + "[red]WHO failed: " + escape(err.Error()), + ) } } @@ -619,9 +619,9 @@ func (a *App) cmdWhois(args string) { }, ) if err != nil { - a.ui.AddStatus(fmt.Sprintf( - "[red]WHOIS failed: %v", err, - )) + a.ui.AddStatus( + "[red]WHOIS failed: " + escape(err.Error()), + ) } } @@ -824,7 +824,7 @@ func (a *App) handlePrivmsgEvent( a.ui.AddLine(target, fmt.Sprintf( "[gray]%s [green]<%s>[white] %s", - timestamp, msg.From, text, + timestamp, escape(msg.From), escape(text), )) } @@ -837,7 +837,7 @@ func (a *App) handleJoinEvent( a.ui.AddLine(msg.To, fmt.Sprintf( "[gray]%s [yellow]*** %s has joined %s", - timestamp, msg.From, msg.To, + timestamp, escape(msg.From), escape(msg.To), )) } @@ -856,11 +856,11 @@ func (a *App) handlePartEvent( line := fmt.Sprintf( "[gray]%s [yellow]*** %s has left %s", - timestamp, msg.From, msg.To, + timestamp, escape(msg.From), escape(msg.To), ) if reason != "" { - line += " (" + reason + ")" + line += " (" + escape(reason) + ")" } if msg.From == myNick { @@ -881,12 +881,12 @@ func (a *App) handleQuitEvent( if reason != "" { a.ui.AddStatus(fmt.Sprintf( "[gray]%s [yellow]*** %s has quit (%s)", - timestamp, msg.From, reason, + timestamp, escape(msg.From), escape(reason), )) } else { a.ui.AddStatus(fmt.Sprintf( "[gray]%s [yellow]*** %s has quit", - timestamp, msg.From, + timestamp, escape(msg.From), )) } } @@ -913,7 +913,7 @@ func (a *App) handleNickEvent( a.ui.AddStatus(fmt.Sprintf( "[gray]%s [yellow]*** %s is now known as %s", - timestamp, msg.From, newNick, + timestamp, escape(msg.From), escape(newNick), )) } @@ -925,7 +925,7 @@ func (a *App) handleNoticeEvent( a.ui.AddStatus(fmt.Sprintf( "[gray]%s [magenta]--%s-- %s", - timestamp, msg.From, text, + timestamp, escape(msg.From), escape(text), )) } @@ -941,7 +941,7 @@ func (a *App) handleTopicEvent( a.ui.AddLine(msg.To, fmt.Sprintf( "[gray]%s [cyan]*** %s set topic: %s", - timestamp, msg.From, text, + timestamp, escape(msg.From), escape(text), )) } @@ -951,10 +951,12 @@ func (a *App) handleDefaultEvent( lines := msg.BodyLines() text := strings.Join(lines, " ") + // The brackets around the command are escaped with it, as tview + // reads a command such as "[MODE]" as a tag. if text != "" { a.ui.AddStatus(fmt.Sprintf( - "[gray]%s [white][%s] %s", - timestamp, msg.Command, text, + "[gray]%s [white]%s %s", + timestamp, escape("["+msg.Command+"]"), escape(text), )) } } diff --git a/internal/cli/app_internal_test.go b/internal/cli/app_internal_test.go index a6bf8af..79571b1 100644 --- a/internal/cli/app_internal_test.go +++ b/internal/cli/app_internal_test.go @@ -8,6 +8,7 @@ import ( "strings" "testing" "time" + "unicode/utf8" "github.com/gdamore/tcell/v2" api "sneak.berlin/go/neoirc/internal/cli/api" @@ -892,6 +893,260 @@ func TestOwnEventFromOtherClient(t *testing.T) { } } +// TestBracketsInMessageWindow checks that each kind of line drawn +// from a server event shows nicks, channel names, texts, reasons +// and topics holding square brackets as written. +func TestBracketsInMessageWindow(t *testing.T) { + t.Parallel() + + // Every event is sent to this channel and has a body of one line. + const channel = "#chan[x]" + + tests := []struct { + name string + command string + from string + body string + // Whether the line goes to the channel's window rather than + // the status window, which is the one shown at start. + inChannel bool + // How the line ends, after the timestamps. + want string + }{ + { + name: "message", command: irc.CmdPrivmsg, inChannel: true, + from: "bob[|[]", body: "x[^[] [:::y|z] [red]", + want: " x[^[] [:::y|z] [red]", + }, + { + name: "joined", command: irc.CmdJoin, from: "x[^[]", inChannel: true, + 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[x] ([:::y|z])", + }, + { + name: "topic", command: irc.CmdTopic, inChannel: true, + from: "dave[afk]", body: "[red]news", + want: "*** dave[afk] set topic: [red]news", + }, + { + 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[x]", body: "[::b]note", + want: "--bob[x]-- [::b]note", + }, + { + name: "other command", command: irc.CmdMode, body: "+v dave[afk]", + want: "[MODE] +v dave[afk]", + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + t.Parallel() + + app := &App{ui: NewUI(), nick: testNick} + screen := runUI(t, app.ui) + + if test.inChannel { + app.ui.SwitchToBuffer(channel) + } + + app.handleServerMessage(&api.Message{ + Command: test.command, From: test.from, To: channel, + Body: []string{test.body}, + }) + + 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) + } + }) + } +} + +// TestColourTagInMessage checks that a colour tag in a message +// shows as written and leaves the message text white, as the +// client draws it. +func TestColourTagInMessage(t *testing.T) { + t.Parallel() + + const want = " [red]x" + + app := &App{ui: NewUI(), nick: testNick} + screen := runUI(t, app.ui) + app.ui.SwitchToBuffer(testChannel) + + app.handleServerMessage(&api.Message{ + Command: irc.CmdPrivmsg, From: testOtherNick, To: testChannel, + Body: []string{"[red]x"}, + }) + + 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) + } +} + // bufferState returns the number of lines in the named // buffer and how many of them are unread. func bufferState(ui *UI, name string) (int, int) { @@ -919,6 +1174,41 @@ func statusBarText(ui *UI) string { return text } +// shownLine returns the given line of the message window, counting +// from 0, as the screen shows it, without the blanks after it, and +// the colour of its last character. Each line in these tests takes +// one row of the screen. +func shownLine( + ui *UI, screen tcell.SimulationScreen, line int, +) (string, tcell.Color) { + var ( + text string + colour tcell.Color + ) + + ui.app.QueueUpdate(func() { + cells, width, _ := screen.GetContents() + cells = cells[line*width : (line+1)*width] + + var row strings.Builder + + for _, cell := range cells { + row.WriteString(string(cell.Runes)) + } + + text = strings.TrimRight(row.String(), " ") + if text == "" { + return + } + + // Each character in these tests takes one cell. + last := cells[utf8.RuneCountInString(text)-1] + colour, _, _ = last.Style.Decompose() + }) + + return text, colour +} + // handledLines makes app.ui pass each typed line to // app.handleInput, as Run does, and returns a channel that // receives each line once it has been handled. @@ -964,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. @@ -1011,11 +1318,15 @@ func waitPoll(t *testing.T, polled <-chan struct{}, server string) { } } -// runUI runs ui on a simulated screen until the test ends. -func runUI(t *testing.T, ui *UI) { +// runUI runs ui on a simulated screen until the test ends, and +// returns the screen. +// +//nolint:ireturn // tcell has no exported type for the screen +func runUI(t *testing.T, ui *UI) tcell.SimulationScreen { t.Helper() - ui.app.SetScreen(tcell.NewSimulationScreen("")) + screen := tcell.NewSimulationScreen("") + ui.app.SetScreen(screen) // One slot, so that Run's goroutine can end even after the // cleanup below has stopped waiting for it. @@ -1035,4 +1346,6 @@ func runUI(t *testing.T, ui *UI) { t.Error("the UI did not stop: its event loop stopped responding") } }) + + return screen } diff --git a/internal/cli/ui.go b/internal/cli/ui.go index 6f229df..80708a6 100644 --- a/internal/cli/ui.go +++ b/internal/cli/ui.go @@ -235,6 +235,16 @@ func (ui *UI) initMessages() { ui.messages.SetBorder(false) } +// escape returns text so that the message window shows it exactly +// as written. It writes each "[" as "[[:]": a "[" followed by +// another "[" never starts a tag, and the tag "[:]" changes +// nothing. tview.Escape is not enough: it does not escape a bracket +// pair holding a symbol such as "|" or "^", and nothing it adds +// stops a tag with a URL, such as "[:::y|z]". +func escape(text string) string { + return strings.ReplaceAll(text, "[", "[[:]") +} + func (ui *UI) initStatusBar() { // Colour tags stay off. With them on, tview reads text in // square brackets, such as "[connected]" or part of a nick,