uniformize limit handling

* max-keys is enforced for channels as well
* remove unlimited configurations
* maintain the limit exactly without off-by-one cases

default.yaml

@@ -1093,10 +1093,9 @@ metadata:
     # can clients store metadata?
     enabled: true
     # how many keys can a client subscribe to?
-    # set to 0 to disable subscriptions or -1 to allow unlimited subscriptions.
     max-subs: 100
-    # how many keys can a user store about themselves? set to -1 to allow unlimited keys.
+    # how many keys can be stored per entity?
-    max-keys: 1000
+    max-keys: 100
 
 # experimental support for mobile push notifications
 # see the manual for potential security, privacy, and performance implications.

irc/config.go

@@ -1649,19 +1649,20 @@ func LoadConfig(filename string) (config *Config, err error) {
 		config.Server.supportedCaps.Disable(caps.Metadata)
 	} else {
 		var metadataValues []string
-		if config.Metadata.MaxSubs >= 0 {
+		// these are required for normal operation, so set sane defaults:
-			metadataValues = append(metadataValues, fmt.Sprintf("max-subs=%d", config.Metadata.MaxSubs))
+		if config.Metadata.MaxSubs == 0 {
+			config.Metadata.MaxSubs = 10
 		}
-		if config.Metadata.MaxKeys > 0 {
+		metadataValues = append(metadataValues, fmt.Sprintf("max-subs=%d", config.Metadata.MaxSubs))
-			metadataValues = append(metadataValues, fmt.Sprintf("max-keys=%d", config.Metadata.MaxKeys))
+		if config.Metadata.MaxKeys == 0 {
+			config.Metadata.MaxKeys = 10
 		}
+		metadataValues = append(metadataValues, fmt.Sprintf("max-keys=%d", config.Metadata.MaxKeys))
+		// this is not required since we enforce a hardcoded upper bound on key+value
 		if config.Metadata.MaxValueBytes > 0 {
 			metadataValues = append(metadataValues, fmt.Sprintf("max-value-bytes=%d", config.Metadata.MaxValueBytes))
 		}
-		if len(metadataValues) != 0 {
+		config.Server.capValues[caps.Metadata] = strings.Join(metadataValues, ",")
-			config.Server.capValues[caps.Metadata] = strings.Join(metadataValues, ",")
-		}
-
 	}
 
 	err = config.processExtjwt()

irc/getters.go

@@ -894,7 +894,7 @@ func (channel *Channel) GetMetadata(key string) (string, bool) {
 	return val, ok
 }
 
-func (channel *Channel) SetMetadata(key string, value string) (updated bool) {
+func (channel *Channel) SetMetadata(key string, value string, limit int) (updated bool, err error) {
 	defer channel.MarkDirty(IncludeAllAttrs)
 
 	channel.stateMutex.Lock()
@@ -905,11 +905,14 @@ func (channel *Channel) SetMetadata(key string, value string) (updated bool) {
 	}
 
 	existing, ok := channel.metadata[key]
+	if !ok && len(channel.metadata) >= limit {
+		return false, errLimitExceeded
+	}
 	updated = !ok || value != existing
 	if updated {
 		channel.metadata[key] = value
 	}
-	return updated
+	return updated, nil
 }
 
 func (channel *Channel) ListMetadata() map[string]string {
@@ -958,7 +961,7 @@ func (client *Client) GetMetadata(key string) (string, bool) {
 	return val, ok
 }
 
-func (client *Client) SetMetadata(key string, value string) (updated bool) {
+func (client *Client) SetMetadata(key string, value string, limit int) (updated bool, err error) {
 	client.stateMutex.Lock()
 	defer client.stateMutex.Unlock()
 
@@ -967,11 +970,14 @@ func (client *Client) SetMetadata(key string, value string) (updated bool) {
 	}
 
 	existing, ok := client.metadata[key]
+	if !ok && len(client.metadata) >= limit {
+		return false, errLimitExceeded
+	}
 	updated = !ok || value != existing
 	if updated {
 		client.metadata[key] = value
 	}
-	return updated
+	return updated, nil
 }
 
 func (client *Client) ListMetadata() map[string]string {

irc/handlers.go

@@ -3175,19 +3175,17 @@ func metadataHandler(server *Server, client *Client, msg ircmsg.Message, rb *Res
 				return
 			}
 
-			maxKeys := config.Metadata.MaxKeys
+			updated, err := targetObj.SetMetadata(key, value, config.Metadata.MaxKeys)
-			isSelf := targetClient != nil && client == targetClient
+			if err != nil {
-
+				// errLimitExceeded is the only possible error
-			if isSelf && maxKeys > 0 && targetObj.CountMetadata() >= maxKeys {
+				rb.Add(nil, server.name, "FAIL", "METADATA", "LIMIT_REACHED", client.t("Too many metadata keys"))
-				rb.Add(nil, server.name, "FAIL", "METADATA", "LIMIT_REACHED", client.t("You have too many keys set on yourself"))
 				return
 			}
-
-			if updated := targetObj.SetMetadata(key, value); updated {
-				notifySubscribers(server, rb.session, targetObj, target, key, value)
-			}
 			// echo the value to the client whether or not there was a real update
 			rb.Add(nil, server.name, RPL_KEYVALUE, client.Nick(), target, key, "*", value)
+			if updated {
+				notifySubscribers(server, rb.session, targetObj, target, key, value)
+			}
 		} else {
 			if updated := targetObj.DeleteMetadata(key); updated {
 				notifySubscribers(server, rb.session, targetObj, target, key, "")

irc/metadata.go

@@ -21,7 +21,7 @@ var (
 )
 
 type MetadataHaver = interface {
-	SetMetadata(key string, value string) (updated bool)
+	SetMetadata(key string, value string, limit int) (updated bool, err error)
 	GetMetadata(key string) (string, bool)
 	DeleteMetadata(key string) (updated bool)
 	ListMetadata() map[string]string

traditional.yaml

@@ -1064,10 +1064,9 @@ metadata:
     # can clients store metadata?
     enabled: true
     # how many keys can a client subscribe to?
-    # set to 0 to disable subscriptions or -1 to allow unlimited subscriptions.
     max-subs: 100
-    # how many keys can a user store about themselves? set to -1 to allow unlimited keys.
+    # how many keys can be stored per entity?
-    max-keys: 1000
+    max-keys: 100
 
 # experimental support for mobile push notifications
 # see the manual for potential security, privacy, and performance implications.