pkg/api/api0: Refactor server list updates

* Merge update and replace into ServerHybridUpdatePut.
* Integrate limits into ServerList itself.
* Allow new servers to replace live servers if gameserver and
  authserver IP and port are identical.
* Move server token generation into ServerList.

3 files changed, 207 insertions, 170 deletions

diff --git a/pkg/api/api0/api.go b/pkg/api/api0/api.go
index af3505a..3f84238 100644
--- a/pkg/api/api0/api.go
+++ b/pkg/api/api0/api.go
@@ -212,13 +212,3 @@ func marshalJSONBytesAsArray(b []byte) json.RawMessage {
 	e.WriteByte(']')
 	return json.RawMessage(e.Bytes())
 }
-
-func checkLimit(limit, def, cur int) bool {
-	if limit == -1 {
-		return true
-	}
-	if limit == 0 && cur < def {
-		return true
-	}
-	return cur < limit
-}
diff --git a/pkg/api/api0/server.go b/pkg/api/api0/server.go
index 4fd3b43..880d912 100644
--- a/pkg/api/api0/server.go
+++ b/pkg/api/api0/server.go
@@ -251,48 +251,36 @@ func (h *Handler) handleServerAddServer(w http.ResponseWriter, r *http.Request)
 		return
 	}
 
-	if tok, err := cryptoRandHex(32); err != nil {
-		hlog.FromRequest(r).Error().
-			Err(err).
-			Msgf("failed to generate random token")
-		respJSON(w, r, http.StatusInternalServerError, map[string]any{
-			"success": false,
-			"error":   ErrorCode_INTERNAL_SERVER_ERROR,
-			"msg":     ErrorCode_INTERNAL_SERVER_ERROR.Message(),
-		})
-		return
-	} else {
-		s.ServerAuthToken = tok
+	var l ServerListLimit
+	if n := h.MaxServers; n > 0 {
+		l.MaxServers = n
+	} else if n == 0 {
+		l.MaxServers = 1000
 	}
-
-	// these checks are racy, but it's meant to be a safety net, not a hard limit
-	if !checkLimit(h.MaxServers, 1000, h.ServerList.GetServerCountByIP(netip.Addr{})) {
-		respJSON(w, r, http.StatusInternalServerError, map[string]any{
-			"success": false,
-			"error":   ErrorCode_INTERNAL_SERVER_ERROR,
-			"msg":     ErrorCode_INTERNAL_SERVER_ERROR.Messagef("game server limit reached"),
-		})
-		return
-	}
-	if !checkLimit(h.MaxServers, 50, h.ServerList.GetServerCountByIP(s.Addr.Addr())) {
-		respJSON(w, r, http.StatusTooManyRequests, map[string]any{
-			"success": false,
-			"error":   ErrorCode_INTERNAL_SERVER_ERROR,
-			"msg":     ErrorCode_INTERNAL_SERVER_ERROR.Messagef("game server per-ip limit reached"),
-		})
-		return
+	if n := h.MaxServersPerIP; n > 0 {
+		l.MaxServersPerIP = n
+	} else if n == 0 {
+		l.MaxServersPerIP = 50
 	}
 
-	if sid, _, err := h.ServerList.PutServerByAddr(&s); err == nil {
-		s.ID = sid
-	} else if errors.Is(err, ErrServerListDuplicateAuthAddr) {
-		respJSON(w, r, http.StatusForbidden, map[string]any{
-			"success": false,
-			"error":   ErrorCode_DUPLICATE_SERVER,
-			"msg":     ErrorCode_DUPLICATE_SERVER.Messagef("%v", err),
-		})
-		return
-	} else {
+	nsrv, err := h.ServerList.ServerHybridUpdatePut(nil, &s, l)
+	if err != nil {
+		if errors.Is(err, ErrServerListDuplicateAuthAddr) {
+			respJSON(w, r, http.StatusForbidden, map[string]any{
+				"success": false,
+				"error":   ErrorCode_DUPLICATE_SERVER,
+				"msg":     ErrorCode_DUPLICATE_SERVER.Messagef("%v", err),
+			})
+			return
+		}
+		if errors.Is(err, ErrServerListLimitExceeded) {
+			respJSON(w, r, http.StatusInternalServerError, map[string]any{
+				"success": false,
+				"error":   ErrorCode_INTERNAL_SERVER_ERROR,
+				"msg":     ErrorCode_INTERNAL_SERVER_ERROR.Messagef("%v", err),
+			})
+			return
+		}
 		hlog.FromRequest(r).Error().
 			Err(err).
 			Msgf("failed to add server to list")
@@ -306,7 +294,7 @@ func (h *Handler) handleServerAddServer(w http.ResponseWriter, r *http.Request)
 
 	respJSON(w, r, http.StatusInternalServerError, map[string]any{
 		"success":         true,
-		"id":              s.ID,
-		"serverAuthToken": s.ServerAuthToken,
+		"id":              nsrv.ID,
+		"serverAuthToken": nsrv.ServerAuthToken,
 	})
 }
diff --git a/pkg/api/api0/serverlist.go b/pkg/api/api0/serverlist.go
index 40e84c5..ba51223 100644
--- a/pkg/api/api0/serverlist.go
+++ b/pkg/api/api0/serverlist.go
@@ -87,6 +87,9 @@ func (s Server) clone() Server {
 }
 
 type ServerUpdate struct {
+	ID       string     // server to update
+	ExpectIP netip.Addr // require the server for ID to have this IP address to successfully update
+
 	Heartbeat   bool
 	Name        *string
 	Description *string
@@ -96,6 +99,16 @@ type ServerUpdate struct {
 	Playlist    *string
 }
 
+type ServerListLimit struct {
+	// MaxServers limits the number of registered servers. If <= 0, no limit is
+	// applied.
+	MaxServers int
+
+	// MaxServersPerIP limits the number of registered servers per IP. If <= 0,
+	// no limit is applied.
+	MaxServersPerIP int
+}
+
 // NewServerList initializes a new server list.
 //
 // deadTime is the time since the last heartbeat after which a server is
@@ -410,19 +423,31 @@ func (s *ServerList) GetServerCountByIP(ip netip.Addr) int {
 	return n
 }
 
-// ErrServerListDuplicateAuthAddr is returned by PutServerByAddr if the auth
-// addr is already used by another live server.
-var ErrServerListDuplicateAuthAddr = errors.New("already have server with auth addr")
-
-// PutServerByAddr creates or replaces a server by x.Addr and returns the new
-// server ID (x.ID, x.LastHeartbeat, and x.Order is ignored). The bool
-// represents whether a live server was replaced. An error is only returned if
-// it fails to generate a new ID, if x.Addr is not set, or
-// ErrServerListDuplicateAuthAddr if the auth port is duplicated by another live
-// server (if so, the server list remains unchanged). Note that even if a ghost
-// server has a matching Addr, a new server with a new ID is created (use
-// UpdateServerByID to revive servers).
-func (s *ServerList) PutServerByAddr(x *Server) (string, bool, error) {
+var (
+	ErrServerListDuplicateAuthAddr = errors.New("already have server with auth addr")
+	ErrServerListUpdateServerDead  = errors.New("no server found")
+	ErrServerListUpdateWrongIP     = errors.New("wrong server update ip")
+	ErrServerListLimitExceeded     = errors.New("would exceed server list limits")
+)
+
+// ServerHybridUpdatePut attempts to update a server by the server ID (if u is
+// non-nil) (reviving it if necessary), and if that fails, then attempts to
+// create/replace a server by the gameserver ip/port instead (if c is non-nil)
+// while following the limits in l. It returns a copy of the resulting Server.
+//
+// If the returned error is non-nil, it will either be an unavoidable internal
+// error (e.g, failure to get random data for the server id) or one of the
+// following (use errors.Is):
+//
+//   - ErrServerListDuplicateAuthAddr - if the auth ip/port of the server to create (if c) or revive (if u and server is a ghost) has already been used by a live server
+//   - ErrServerListUpdateServerDead - if no server matching the provided id exists (if u) AND c is not provided
+//   - ErrServerListUpdateWrongIP - if a server matching the provided id exists, but the ip doesn't match (if u and u.ExpectIP)
+//   - ErrServerListLimitExceeded - if adding the server would exceed server limits (if c and l)
+//
+// When creating a server using the values from c: c.Order, c.ID,
+// c.ServerAuthToken, and c.LastHeartbeat will be generated by this function
+// (any existing value is ignored).
+func (s *ServerList) ServerHybridUpdatePut(u *ServerUpdate, c *Server, l ServerListLimit) (*Server, error) {
 	t := s.now()
 
 	// take a write lock on the server list
@@ -440,135 +465,169 @@ func (s *ServerList) PutServerByAddr(x *Server) (string, bool, error) {
 		s.servers3 = make(map[netip.AddrPort]*Server)
 	}
 
-	// force an update when we're finished
-	defer s.csForceUpdate()
-	defer s.csUpdateNextUpdateTime()
+	// if we have an update
+	if u != nil {
 
-	// deep copy the server info
-	nsrv := x.clone()
+		// check if the server with the ID is alive or that u has a heartbeat
+		// and the server is a ghost
+		if esrv, exists := s.servers2[u.ID]; exists || s.isServerAlive(esrv, t) || (u.Heartbeat && s.isServerGhost(esrv, t)) {
 
-	// check the addresses
-	if !nsrv.Addr.IsValid() {
-		return "", false, fmt.Errorf("addr is missing")
-	}
-	if nsrv.AuthPort == 0 {
-		return "", false, fmt.Errorf("authport is missing")
-	}
+			// ensure a live server hasn't already taken the auth port (which
+			// can happen if it was a ghost and a new server got registered)
+			if osrv, exists := s.servers3[esrv.AuthAddr()]; !exists || esrv == osrv {
 
-	// error if there's an existing server with a matching auth addr
-	if esrv, exists := s.servers3[nsrv.AuthAddr()]; exists {
-		if s.isServerAlive(esrv, t) {
-			return "", false, fmt.Errorf("%w %s (used for server %s)", ErrServerListDuplicateAuthAddr, nsrv.AuthAddr(), esrv.Addr)
-		}
-	}
+				// check the update ip
+				if u.ExpectIP.IsValid() && esrv.Addr.Addr() != u.ExpectIP {
+					return nil, ErrServerListUpdateWrongIP
+				}
 
-	// allocate a new server ID, skipping any which already exist
-	for {
-		sid, err := cryptoRandHex(32)
-		if err != nil {
-			return "", false, fmt.Errorf("failed to generate new server id: %w", err)
-		}
-		if _, exists := s.servers2[sid]; exists {
-			continue // try another id
+				// do the update
+				var changed bool
+				if u.Heartbeat {
+					esrv.LastHeartbeat, changed = t, true
+					s.csUpdateNextUpdateTime()
+				}
+				if u.Name != nil {
+					esrv.Name, changed = *u.Name, true
+				}
+				if u.Description != nil {
+					esrv.Description, changed = *u.Description, true
+				}
+				if u.Map != nil {
+					esrv.Map, changed = *u.Map, true
+				}
+				if u.Playlist != nil {
+					esrv.Playlist, changed = *u.Playlist, true
+				}
+				if u.PlayerCount != nil {
+					esrv.PlayerCount, changed = *u.PlayerCount, true
+				}
+				if u.MaxPlayers != nil {
+					esrv.MaxPlayers, changed = *u.MaxPlayers, true
+				}
+				if changed {
+					s.csForceUpdate()
+				}
+
+				// return a copy of the updated server
+				r := esrv.clone()
+				return &r, nil
+			}
+		} else {
+			if s.isServerGone(esrv, t) {
+				s.freeServer(esrv) // if the server we found shouldn't exist anymore, clean it up
+			}
 		}
-		nsrv.ID = sid
-		break
+		// fallthough - no eligible server to update, try to create one instead
 	}
 
-	// set the heartbeat time to the current time
-	nsrv.LastHeartbeat = t
+	// create/replace a server instead if we have s
+	if s != nil {
 
-	// set the server order
-	nsrv.Order = s.order.Add(1)
+		// deep copy the new server info
+		nsrv := c.clone()
 
-	// remove any existing server with a matching game address/port
-	var replaced bool
-	if esrv, exists := s.servers1[nsrv.Addr]; exists {
-		if s.isServerAlive(esrv, t) {
-			replaced = true
+		// check the addresses
+		if !nsrv.Addr.IsValid() {
+			return nil, fmt.Errorf("addr is missing")
+		}
+		if nsrv.AuthPort == 0 {
+			return nil, fmt.Errorf("authport is missing")
 		}
-		s.freeServer(esrv)
-	}
 
-	// add it to the indexes (the pointers MUST be the same or stuff will break)
-	s.servers1[nsrv.Addr] = &nsrv
-	s.servers2[nsrv.ID] = &nsrv
-	s.servers3[nsrv.AuthAddr()] = &nsrv
+		// error if there's an existing server with a matching auth addr (note:
+		// same ip as gameserver, different port) but different gameserver addr
+		// (it's probably a config mistake on the server owner's side)
+		if esrv, exists := s.servers3[nsrv.AuthAddr()]; exists && s.isServerAlive(esrv, t) {
+
+			// we want to allow the server to be replaced if the gameserver and
+			// authserver addr are the same since it probably just restarted
+			// after a crash (it's not like you can have multiple servers
+			// listening on the same port with default config, so presumably the
+			// old server must be gone anyways)
+			if esrv.Addr != nsrv.Addr {
+				return nil, fmt.Errorf("%w %s (used for server %s)", ErrServerListDuplicateAuthAddr, nsrv.AuthAddr(), esrv.Addr)
+			}
+		}
 
-	// return the new ID
-	return nsrv.ID, replaced, nil
-}
+		// we will need to remove an existing server with a matching game
+		// address/port if it exists
+		var toReplace *Server
+		if esrv, exists := s.servers1[nsrv.Addr]; exists {
+			if s.isServerGone(esrv, t) {
+				s.freeServer(esrv) // if the server we found shouldn't exist anymore, clean it up
+			} else {
+				toReplace = esrv
+			}
+		}
 
-var (
-	ErrServerListUpdateServerDead = errors.New("no server found")
-	ErrServerListUpdateWrongIP    = errors.New("wrong server update ip")
-)
+		// check limits
+		if l.MaxServers != 0 || l.MaxServersPerIP != 0 {
+			nSrv, nSrvIP := 1, 1
+			for _, esrv := range s.servers1 {
+				if s.isServerAlive(esrv, t) && esrv != toReplace {
+					if esrv.Addr.Addr() == nsrv.Addr.Addr() {
+						nSrvIP++
+					}
+					nSrv++
+				}
+			}
+			if l.MaxServers > 0 && nSrv > l.MaxServers {
+				return nil, fmt.Errorf("%w: too many servers (%d)", ErrServerListLimitExceeded, nSrv)
+			}
+			if l.MaxServersPerIP > 0 && nSrvIP > l.MaxServersPerIP {
+				return nil, fmt.Errorf("%w: too many servers for ip %s (%d)", ErrServerListLimitExceeded, nsrv.Addr.Addr(), nSrv)
+			}
+		}
 
-// UpdateServerByID updates values for the server with the provided ID. If the
-// error nil, the server was updated. If no live server (or a ghost server which
-// could be made alive from u.Heartbeat) was found, errors.Is(err,
-// ErrServerListUpdateServerDead). If ip is valid and doesn't match the target
-// server, errors.Is(err, ErrServerListUpdateWrongIP).
-func (s *ServerList) UpdateServerByID(id string, ip netip.Addr, u *ServerUpdate) error {
-	t := s.now()
+		// generate a new server token
+		if tok, err := cryptoRandHex(32); err != nil {
+			return nil, fmt.Errorf("generate new server auth token: %w", err)
+		} else {
+			nsrv.ServerAuthToken = tok
+		}
 
-	// take a write lock on the server list
-	s.mu.Lock()
-	defer s.mu.Unlock()
+		// allocate a new server ID, skipping any which already exist
+		for {
+			sid, err := cryptoRandHex(32)
+			if err != nil {
+				return nil, fmt.Errorf("generate new server id: %w", err)
+			}
+			if _, exists := s.servers2[sid]; exists {
+				continue // try another id since another server already used it
+			}
+			nsrv.ID = sid
+			break
+		}
 
-	// if the map isn't initialized, we don't have any servers
-	if s.servers2 == nil {
-		return ErrServerListUpdateServerDead
-	}
+		// set the server order
+		nsrv.Order = s.order.Add(1)
 
-	// force an update when we're finished
-	defer s.csForceUpdate()
-	defer s.csUpdateNextUpdateTime()
+		// set the heartbeat time to the current time
+		nsrv.LastHeartbeat = t
 
-	// get the server if it's eligible for updates
-	esrv, exists := s.servers2[id]
-	if !(exists || s.isServerAlive(esrv, t) || (u.Heartbeat && s.isServerGhost(esrv, t))) {
-		if s.isServerGone(esrv, t) {
-			s.freeServer(esrv)
+		// remove the existing server so we can add the new one
+		if toReplace != nil {
+			s.freeServer(toReplace)
 		}
-		return ErrServerListUpdateServerDead
-	}
-
-	// ensure another server hasn't already taken the auth port (which can
-	// happen if it was a ghost)
-	if osrv, exists := s.servers3[esrv.AuthAddr()]; exists && esrv != osrv {
-		return ErrServerListUpdateServerDead
-	}
 
-	if ip.IsValid() && esrv.Addr.Addr() != ip {
-		return ErrServerListUpdateWrongIP
-	}
+		// add the new one (the pointers MUST be the same or stuff will break)
+		s.servers1[nsrv.Addr] = &nsrv
+		s.servers2[nsrv.ID] = &nsrv
+		s.servers3[nsrv.AuthAddr()] = &nsrv
 
-	// do the update
-	if u.Heartbeat {
-		esrv.LastHeartbeat = t
+		// trigger /client/servers updates
+		s.csForceUpdate()
 		s.csUpdateNextUpdateTime()
+
+		// return a copy of the new server
+		r := nsrv.clone()
+		return &r, nil
 	}
-	if u.Name != nil {
-		esrv.Name = *u.Name
-	}
-	if u.Description != nil {
-		esrv.Description = *u.Description
-	}
-	if u.Map != nil {
-		esrv.Map = *u.Map
-	}
-	if u.Playlist != nil {
-		esrv.Playlist = *u.Playlist
-	}
-	if u.PlayerCount != nil {
-		esrv.PlayerCount = *u.PlayerCount
-	}
-	if u.MaxPlayers != nil {
-		esrv.MaxPlayers = *u.MaxPlayers
-	}
-	s.csForceUpdate()
-	return nil
+
+	// if we don't have an update or a new server to create/replace instead, we
+	// didn't find any eligible live servers, so...
+	return nil, ErrServerListUpdateServerDead
 }
 
 // ReapServers deletes dead servers from memory.