Accept nick from 001 message. Fixes #110.

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) {

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
+// :<server> 001 <nick> :Welcome message <nick>!<user>@<host>
 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
 		}
 	}
 }

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)

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)

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)
+		}
+	}
+}