Apply default user modes just before registration.

Previously, we were applying defaults before the user had completed
registration. This meant that the number of invisible users was
incremented when the user connected, and then the total was incremented
when registration was completed.

Now both counters are updated at the same time. If a user disconnects
prior to registration, +i has not yet been applied so it would not be
decremented.

irc/client.go

@@ -337,7 +337,6 @@ func (server *Server) RunClient(conn IRCConn) {
 	session.idletimer.Initialize(session)
 	session.resetFakelag()
 
-	ApplyUserModeChanges(client, config.Accounts.defaultUserModes, false, nil)
 	if wConn.Secure {
 		client.SetMode(modes.TLS, true)
 	}

irc/config.go

@@ -256,7 +256,7 @@ type AccountConfig struct {
 		exemptedNets []net.IPNet
 	} `yaml:"require-sasl"`
 	DefaultUserModes    *string `yaml:"default-user-modes"`
-	defaultUserModes    modes.ModeChanges
+	defaultUserModes    modes.Modes
 	LDAP                ldap.ServerConfig
 	LoginThrottling     ThrottleConfig `yaml:"login-throttling"`
 	SkipServerPassword  bool           `yaml:"skip-server-password"`

irc/modes.go

@@ -23,7 +23,7 @@ var (
 
 	// DefaultUserModes are set on all users when they login.
 	// this can be overridden in the `accounts` config, with the `default-user-modes` key
-	DefaultUserModes = modes.ModeChanges{}
+	DefaultUserModes = modes.Modes{}
 )
 
 // ApplyUserModeChanges applies the given changes, and returns the applied changes.
@@ -110,32 +110,35 @@ func ApplyUserModeChanges(client *Client, changes modes.ModeChanges, force bool,
 	return applied
 }
 
+// parseDefaultModes uses the provided mode change parser to parse the rawModes.
+func parseDefaultModes(rawModes string, parser func(params ...string) (modes.ModeChanges, map[rune]bool)) modes.Modes {
+	modeChangeStrings := strings.Fields(rawModes)
+	modeChanges, _ := parser(modeChangeStrings...)
+	defaultModes := make(modes.Modes, 0)
+	for _, modeChange := range modeChanges {
+		if modeChange.Op == modes.Add {
+			defaultModes = append(defaultModes, modeChange.Mode)
+		}
+	}
+	return defaultModes
+}
+
 // ParseDefaultChannelModes parses the `default-modes` line of the config
 func ParseDefaultChannelModes(rawModes *string) modes.Modes {
 	if rawModes == nil {
 		// not present in config, fall back to compile-time default
 		return DefaultChannelModes
 	}
-	modeChangeStrings := strings.Fields(*rawModes)
-	modeChanges, _ := modes.ParseChannelModeChanges(modeChangeStrings...)
-	defaultChannelModes := make(modes.Modes, 0)
-	for _, modeChange := range modeChanges {
-		if modeChange.Op == modes.Add {
-			defaultChannelModes = append(defaultChannelModes, modeChange.Mode)
-		}
-	}
-	return defaultChannelModes
+	return parseDefaultModes(*rawModes, modes.ParseChannelModeChanges)
 }
 
 // ParseDefaultUserModes parses the `default-user-modes` line of the config
-func ParseDefaultUserModes(rawModes *string) modes.ModeChanges {
+func ParseDefaultUserModes(rawModes *string) modes.Modes {
 	if rawModes == nil {
 		// not present in config, fall back to compile-time default
 		return DefaultUserModes
 	}
-	modeChangeStrings := strings.Fields(*rawModes)
-	modeChanges, _ := modes.ParseUserModeChanges(modeChangeStrings...)
-	return modeChanges
+	return parseDefaultModes(*rawModes, modes.ParseUserModeChanges)
 }
 
 // #1021: channel key must be valid as a non-final parameter

irc/modes_test.go

@@ -43,19 +43,19 @@ func TestParseDefaultUserModes(t *testing.T) {
 
 	var parseTests = []struct {
 		raw      *string
-		expected modes.ModeChanges
+		expected modes.Modes
 	}{
-		{&iR, modes.ModeChanges{{Mode: modes.Invisible, Op: modes.Add}, {Mode: modes.RegisteredOnly, Op: modes.Add}}},
-		{&i, modes.ModeChanges{{Mode: modes.Invisible, Op: modes.Add}}},
-		{&empty, modes.ModeChanges{}},
-		{&rminusi, modes.ModeChanges{{Mode: modes.RegisteredOnly, Op: modes.Add}}},
-		{nil, modes.ModeChanges{}},
+		{&iR, modes.Modes{modes.Invisible, modes.RegisteredOnly}},
+		{&i, modes.Modes{modes.Invisible}},
+		{&empty, modes.Modes{}},
+		{&rminusi, modes.Modes{modes.RegisteredOnly}},
+		{nil, modes.Modes{}},
 	}
 
 	for _, testcase := range parseTests {
 		result := ParseDefaultUserModes(testcase.raw)
 		if !reflect.DeepEqual(result, testcase.expected) {
-			t.Errorf("expected modes %v, got %v", testcase.expected, result)
+			t.Errorf("expected modes %s, got %s", testcase.expected, result)
 		}
 	}
 }

irc/server.go

@@ -266,11 +266,18 @@ func (server *Server) tryRegister(c *Client, session *Session) (exiting bool) {
 		return true
 	}
 
+	// Apply default user modes (without updating the invisible counter)
+	// The number of invisible users will be updated by server.stats.Register
+	// if we're using default user mode +i.
+	for _, defaultMode := range server.Config().Accounts.defaultUserModes {
+		c.SetMode(defaultMode, true)
+	}
+
 	// registration has succeeded:
 	c.SetRegistered()
 
 	// count new user in statistics
-	server.stats.Register()
+	server.stats.Register(c.HasMode(modes.Invisible))
 	server.monitorManager.AlertAbout(c, true)
 
 	server.playRegistrationBurst(session)

irc/stats.go

@@ -41,9 +41,12 @@ func (s *Stats) AddRegistered(invisible, operator bool) {
 }
 
 // Transition a client from unregistered to registered
-func (s *Stats) Register() {
+func (s *Stats) Register(invisible bool) {
 	s.mutex.Lock()
 	s.Unknown -= 1
+	if invisible {
+		s.Invisible += 1
+	}
 	s.Total += 1
 	s.setMax()
 	s.mutex.Unlock()