diff --git a/irc/client.go b/irc/client.go index 3e56927b..2f1e88c5 100644 --- a/irc/client.go +++ b/irc/client.go @@ -374,34 +374,17 @@ func (client *Client) Ping() { } -// Register sets the client details as appropriate when entering the network. -func (client *Client) Register() { - client.stateMutex.Lock() - alreadyRegistered := client.registered - client.registered = true - client.stateMutex.Unlock() - - if alreadyRegistered { - return - } - - // apply resume details if we're able to. - client.TryResume() - - // finish registration - client.updateNickMask() - client.server.monitorManager.AlertAbout(client, true) -} - -// TryResume tries to resume if the client asked us to. -func (client *Client) TryResume() { - if client.resumeDetails == nil { - return - } - +// tryResume tries to resume if the client asked us to. +func (client *Client) tryResume() (success bool) { server := client.server config := server.Config() + defer func() { + if !success { + client.resumeDetails = nil + } + }() + oldnick := client.resumeDetails.OldNick timestamp := client.resumeDetails.Timestamp var timestampString string @@ -412,7 +395,6 @@ func (client *Client) TryResume() { oldClient := server.clients.Get(oldnick) if oldClient == nil { client.Send(nil, server.name, "RESUME", "ERR", oldnick, client.t("Cannot resume connection, old client not found")) - client.resumeDetails = nil return } oldNick := oldClient.Nick() @@ -421,24 +403,23 @@ func (client *Client) TryResume() { resumeAllowed := config.Server.AllowPlaintextResume || (oldClient.HasMode(modes.TLS) && client.HasMode(modes.TLS)) if !resumeAllowed { client.Send(nil, server.name, "RESUME", "ERR", oldnick, client.t("Cannot resume connection, old and new clients must have TLS")) - client.resumeDetails = nil return } oldResumeToken := oldClient.ResumeToken() if oldResumeToken == "" || !utils.SecretTokensMatch(oldResumeToken, client.resumeDetails.PresentedToken) { client.Send(nil, server.name, "RESUME", "ERR", client.t("Cannot resume connection, invalid resume token")) - client.resumeDetails = nil return } err := server.clients.Resume(client, oldClient) if err != nil { - client.resumeDetails = nil client.Send(nil, server.name, "RESUME", "ERR", client.t("Cannot resume connection")) return } + success = true + // this is a bit racey client.resumeDetails.ResumedAt = time.Now() @@ -520,13 +501,11 @@ func (client *Client) TryResume() { client.Send(nil, client.server.name, "RESUME", "SUCCESS", oldNick) // after we send the rest of the registration burst, we'll try rejoining channels + return } func (client *Client) tryResumeChannels() { details := client.resumeDetails - if details == nil { - return - } channels := make([]*Channel, len(details.Channels)) for _, name := range details.Channels { diff --git a/irc/getters.go b/irc/getters.go index 457d311a..3ae77959 100644 --- a/irc/getters.go +++ b/irc/getters.go @@ -127,6 +127,16 @@ func (client *Client) Registered() bool { return client.registered } +func (client *Client) SetRegistered() { + // `registered` is only written from the client's own goroutine, but may be + // read from other goroutines; therefore, the client's own goroutine may read + // the value without synchronization, but must write it with synchronization, + // and other goroutines must read it with synchronization + client.stateMutex.Lock() + client.registered = true + client.stateMutex.Unlock() +} + func (client *Client) Destroyed() bool { client.stateMutex.RLock() defer client.stateMutex.RUnlock() diff --git a/irc/server.go b/irc/server.go index 57aa5ac7..5d799ad9 100644 --- a/irc/server.go +++ b/irc/server.go @@ -384,49 +384,58 @@ func (server *Server) generateMessageID() string { // func (server *Server) tryRegister(c *Client) { - if c.registered { - return + resumed := false + // try to complete registration, either via RESUME token or normally + if c.resumeDetails != nil { + if !c.tryResume() { + return + } + resumed = true + } else { + if c.preregNick == "" || !c.HasUsername() || c.capState == caps.NegotiatingState { + return + } + + // client MUST send PASS if necessary, or authenticate with SASL if necessary, + // before completing the other registration commands + config := server.Config() + if !c.isAuthorized(config) { + c.Quit(c.t("Bad password")) + c.destroy(false) + return + } + + rb := NewResponseBuffer(c) + nickAssigned := performNickChange(server, c, c, c.preregNick, rb) + rb.Send(true) + if !nickAssigned { + c.preregNick = "" + return + } + + // check KLINEs + isBanned, info := server.klines.CheckMasks(c.AllNickmasks()...) + if isBanned { + c.Quit(info.BanMessage(c.t("You are banned from this server (%s)"))) + c.destroy(false) + return + } } - if c.preregNick == "" || !c.HasUsername() || c.capState == caps.NegotiatingState { - return - } - - // client MUST send PASS if necessary, or authenticate with SASL if necessary, - // before completing the other registration commands - config := server.Config() - if !c.isAuthorized(config) { - c.Quit(c.t("Bad password")) - c.destroy(false) - return - } - - rb := NewResponseBuffer(c) - nickAssigned := performNickChange(server, c, c, c.preregNick, rb) - rb.Send(true) - if !nickAssigned { - c.preregNick = "" - return - } - - // check KLINEs - isBanned, info := server.klines.CheckMasks(c.AllNickmasks()...) - if isBanned { - c.Quit(info.BanMessage(c.t("You are banned from this server (%s)"))) - c.destroy(false) - return - } + // registration has succeeded: + c.SetRegistered() // count new user in statistics server.stats.ChangeTotal(1) + if !resumed { + server.monitorManager.AlertAbout(c, true) + } + // continue registration server.logger.Debug("localconnect", fmt.Sprintf("Client connected [%s] [u:%s] [r:%s]", c.nick, c.username, c.realname)) server.snomasks.Send(sno.LocalConnects, fmt.Sprintf("Client connected [%s] [u:%s] [h:%s] [ip:%s] [r:%s]", c.nick, c.username, c.rawHostname, c.IPString(), c.realname)) - // "register"; this includes the initial phase of session resumption - c.Register() - // send welcome text //NOTE(dan): we specifically use the NICK here instead of the nickmask // see http://modern.ircdocs.horse/#rplwelcome-001 for details on why we avoid using the nickmask @@ -436,7 +445,7 @@ func (server *Server) tryRegister(c *Client) { //TODO(dan): Look at adding last optional [] parameter c.Send(nil, server.name, RPL_MYINFO, c.nick, server.name, Ver, supportedUserModesString, supportedChannelModesString) - rb = NewResponseBuffer(c) + rb := NewResponseBuffer(c) c.RplISupport(rb) server.MOTD(c, rb) rb.Send(true) @@ -449,8 +458,9 @@ func (server *Server) tryRegister(c *Client) { c.Notice(c.t("This server is in debug mode and is logging all user I/O. If you do not wish for everything you send to be readable by the server owner(s), please disconnect.")) } - // if resumed, send fake channel joins - c.tryResumeChannels() + if resumed { + c.tryResumeChannels() + } } // t returns the translated version of the given string, based on the languages configured by the client.