From 1bb2dff2988578be620f573643a60260bce13775 Mon Sep 17 00:00:00 2001 From: Alex Bramley Date: Fri, 26 Mar 2021 11:20:00 +0000 Subject: [PATCH] Accept nick from 001 message. Fixes #110. --- client/connection_test.go | 34 ++++++++++++++++++---------------- client/handlers.go | 36 +++++++++++++++++++++++++----------- client/handlers_test.go | 9 +++++++-- client/line.go | 18 +++++++++++++----- client/line_test.go | 28 ++++++++++++++++++++++++++++ 5 files changed, 91 insertions(+), 34 deletions(-) diff --git a/client/connection_test.go b/client/connection_test.go index 3b7e5b7..e56df44 100644 --- a/client/connection_test.go +++ b/client/connection_test.go @@ -458,20 +458,22 @@ func TestRunLoop(t *testing.T) { c, s := setUp(t, false) defer s.tearDown() - // Set up a handler to detect whether 001 handler is called - h001 := callCheck(t) - c.HandleFunc("001", func(conn *Conn, line *Line) { - h001.call() - }) + // Set up a handler to detect whether 002 handler is called. + // Don't use 001 here, since there's already a handler for that + // and it hangs this test unless we mock the state tracker calls. h002 := callCheck(t) - // Set up a handler to detect whether 002 handler is called c.HandleFunc("002", func(conn *Conn, line *Line) { h002.call() }) + h003 := callCheck(t) + // Set up a handler to detect whether 002 handler is called + c.HandleFunc("003", func(conn *Conn, line *Line) { + h003.call() + }) - l1 := ParseLine(":irc.server.org 001 test :First test line.") - c.in <- l1 - h001.assertNotCalled("001 handler called before runLoop started.") + l2 := ParseLine(":irc.server.org 002 test :First test line.") + c.in <- l2 + h002.assertNotCalled("002 handler called before runLoop started.") // We want to test that the a goroutine calling runLoop will exit correctly. // Now, we can expect the call to Dispatch to take place as runLoop starts. @@ -482,13 +484,13 @@ func TestRunLoop(t *testing.T) { c.runLoop() exited.call() }() - h001.assertWasCalled("001 handler not called after runLoop started.") + h002.assertWasCalled("002 handler not called after runLoop started.") // Send another line, just to be sure :-) - h002.assertNotCalled("002 handler called before expected.") - l2 := ParseLine(":irc.server.org 002 test :Second test line.") - c.in <- l2 - h002.assertWasCalled("002 handler not called while runLoop started.") + h003.assertNotCalled("003 handler called before expected.") + l3 := ParseLine(":irc.server.org 003 test :Second test line.") + c.in <- l3 + h003.assertWasCalled("003 handler not called while runLoop started.") // Now, use the control channel to exit send and kill the goroutine. // This sneakily uses the fact that the other two goroutines that would @@ -500,8 +502,8 @@ func TestRunLoop(t *testing.T) { exited.assertWasCalled("Didn't exit after signal.") // Sending more on c.in shouldn't dispatch any further events - c.in <- l1 - h001.assertNotCalled("001 handler called after runLoop ended.") + c.in <- l2 + h002.assertNotCalled("002 handler called after runLoop ended.") } func TestWrite(t *testing.T) { diff --git a/client/handlers.go b/client/handlers.go index b538579..24165ad 100644 --- a/client/handlers.go +++ b/client/handlers.go @@ -6,6 +6,8 @@ package client import ( "strings" "time" + + "github.com/fluffle/goirc/logging" ) // sets up the internal event handlers to do essential IRC protocol things @@ -41,20 +43,32 @@ func (conn *Conn) h_REGISTER(line *Line) { } // Handler to trigger a CONNECTED event on receipt of numeric 001 +// : 001 :Welcome message !@ func (conn *Conn) h_001(line *Line) { - // we're connected! - conn.dispatch(&Line{Cmd: CONNECTED, Time: time.Now()}) - // and we're being given our hostname (from the server's perspective) - t := line.Args[len(line.Args)-1] + // We're connected! Defer this for control flow reasons. + defer conn.dispatch(&Line{Cmd: CONNECTED, Time: time.Now()}) + + // Accept the server's opinion of what our nick actually is + // and record our ident and hostname (from the server's perspective) + me, nick, t := conn.Me(), line.Target(), line.Text() if idx := strings.LastIndex(t, " "); idx != -1 { t = t[idx+1:] - if idx = strings.Index(t, "@"); idx != -1 { - if conn.st != nil { - me := conn.Me() - conn.st.NickInfo(me.Nick, me.Ident, t[idx+1:], me.Name) - } else { - conn.cfg.Me.Host = t[idx+1:] - } + } + _, ident, host, ok := parseUserHost(t) + + if me.Nick != nick { + logging.Warn("Server changed our nick on connect: old=%q new=%q", me.Nick, nick) + } + if conn.st != nil { + if ok { + conn.st.NickInfo(me.Nick, ident, host, me.Name) + } + conn.cfg.Me = conn.st.ReNick(me.Nick, nick) + } else { + conn.cfg.Me.Nick = nick + if ok { + conn.cfg.Me.Ident = ident + conn.cfg.Me.Host = host } } } diff --git a/client/handlers_test.go b/client/handlers_test.go index d706ae6..171e5c9 100644 --- a/client/handlers_test.go +++ b/client/handlers_test.go @@ -44,7 +44,7 @@ func Test001(t *testing.T) { c, s := setUp(t) defer s.tearDown() - l := ParseLine(":irc.server.org 001 test :Welcome to IRC test!ident@somehost.com") + l := ParseLine(":irc.server.org 001 newnick :Welcome to IRC newnick!ident@somehost.com") // Set up a handler to detect whether connected handler is called from 001 hcon := false c.HandleFunc("connected", func(conn *Conn, line *Line) { @@ -54,7 +54,12 @@ func Test001(t *testing.T) { // Test state tracking first. gomock.InOrder( s.st.EXPECT().Me().Return(c.cfg.Me), - s.st.EXPECT().NickInfo("test", "test", "somehost.com", "Testing IRC"), + s.st.EXPECT().NickInfo("test", "ident", "somehost.com", "Testing IRC"), + s.st.EXPECT().ReNick("test", "newnick").Return(&state.Nick{ + Nick: "newnick", + Ident: c.cfg.Me.Ident, + Name: c.cfg.Me.Name, + }), ) // Call handler with a valid 001 line c.h_001(l) diff --git a/client/line.go b/client/line.go index bfa473a..5275767 100644 --- a/client/line.go +++ b/client/line.go @@ -154,11 +154,10 @@ func ParseLine(s string) *Line { // src can be the hostname of the irc server or a nick!user@host line.Host = line.Src - nidx, uidx := strings.Index(line.Src, "!"), strings.Index(line.Src, "@") - if uidx != -1 && nidx != -1 { - line.Nick = line.Src[:nidx] - line.Ident = line.Src[nidx+1 : uidx] - line.Host = line.Src[uidx+1:] + if n, i, h, ok := parseUserHost(line.Src); ok { + line.Nick = n + line.Ident = i + line.Host = h } } @@ -205,6 +204,15 @@ func ParseLine(s string) *Line { return line } +func parseUserHost(uh string) (nick, ident, host string, ok bool) { + uh = strings.TrimSpace(uh) + nidx, uidx := strings.Index(uh, "!"), strings.Index(uh, "@") + if uidx == -1 || nidx == -1 { + return "", "", "", false + } + return uh[:nidx], uh[nidx+1 : uidx], uh[uidx+1:], true +} + func (line *Line) argslen(minlen int) bool { pc, _, _, _ := runtime.Caller(1) fn := runtime.FuncForPC(pc) diff --git a/client/line_test.go b/client/line_test.go index 88b758d..7316525 100644 --- a/client/line_test.go +++ b/client/line_test.go @@ -184,3 +184,31 @@ func TestLineTags(t *testing.T) { } } } + +func TestParseUserHost(t *testing.T) { + tests := []struct { + in, nick, ident, host string + ok bool + }{ + {"", "", "", "", false}, + {" ", "", "", "", false}, + {"somestring", "", "", "", false}, + {" s p ", "", "", "", false}, + {"foo!bar", "", "", "", false}, + {"foo@baz.com", "", "", "", false}, + {"foo!bar@baz.com", "foo", "bar", "baz.com", true}, + {" foo!bar@baz.com", "foo", "bar", "baz.com", true}, + {" foo!bar@baz.com ", "foo", "bar", "baz.com", true}, + } + + for i, test := range tests { + nick, ident, host, ok := parseUserHost(test.in) + if test.nick != nick || + test.ident != ident || + test.host != host || + test.ok != ok { + t.Errorf("%d: parseUserHost(%q) = %q, %q, %q, %t; want %q, %q, %q, %t", + i, test.in, nick, ident, host, ok, test.nick, test.ident, test.host, test.ok) + } + } +}