Refuse a PRIVMSG or NOTICE whose body or meta has the wrong type (closes #245)

Over the HTTP API, a PRIVMSG or NOTICE body was checked only by
decoding it into a list of strings. JSON null decodes into an empty
string there, and meta was not checked at all, so a null line, or a
meta that is not an object, reached every recipient.

handlePrivmsg now answers 400 invalid request body, before anything
else, when body is present and is not an array of strings, or meta is
present and is not an object. A missing or empty body still gets
ERR_NOTEXTTOSEND. README.md says so.

Model: opus-5-5

internal/handlers/api.go

@@ -994,7 +994,7 @@ func (hdlr *Handlers) dispatchCommand(
 		hdlr.handlePrivmsg(
 			writer, request,
 			sessionID, clientID, nick,
-			command, target, body, meta,
+			command, target, body, meta, bodyLines,
 		)
 	case irc.CmdJoin:
 		hdlr.handleJoin(
@@ -1139,9 +1139,9 @@ func (hdlr *Handlers) handlePrivmsg(
 	nick, command, target string,
 	body json.RawMessage,
 	meta json.RawMessage,
+	bodyLines func() []string,
 ) {
-	lines, ok := checkBodyAndMeta(body, meta)
+	if !validBodyAndMeta(body, meta) {
-	if !ok {
 		hdlr.respondError(
 			writer, request,
 			"invalid request body",
@@ -1165,6 +1165,7 @@ func (hdlr *Handlers) handlePrivmsg(
 		return
 	}
 
+	lines := bodyLines()
 	if len(lines) == 0 {
 		hdlr.enqueueNumeric(
 			request.Context(), clientID,
@@ -1198,29 +1199,25 @@ func (hdlr *Handlers) handlePrivmsg(
 	)
 }
 
-// checkBodyAndMeta returns the lines in body. It returns false
+// validBodyAndMeta reports whether body, when present, is an
-// when body is present and is not an array of strings, or meta
+// array of strings and meta, when present, is an object.
-// is present and is not an object. Each line is decoded as any
+// Decoding into []string would turn a null line into "", so
-// because decoding into []string turns a null line into "". A
+// each line is decoded as any and checked. A null body or
-// null body or meta decodes without an error into nil.
+// meta decodes without an error into nil.
-func checkBodyAndMeta(body, meta json.RawMessage) ([]string, bool) {
+func validBodyAndMeta(body, meta json.RawMessage) bool {
-	var lines []string
-
 	if body != nil {
-		var values []any
+		var lines []any
 
-		err := json.Unmarshal(body, &values)
+		err := json.Unmarshal(body, &lines)
-		if err != nil || values == nil {
+		if err != nil || lines == nil {
-			return nil, false
+			return false
 		}
 
-		for _, value := range values {
+		for _, line := range lines {
-			line, isString := value.(string)
+			_, isString := line.(string)
 			if !isString {
-				return nil, false
+				return false
 			}
-
-			lines = append(lines, line)
 		}
 	}
 
@@ -1229,11 +1226,11 @@ func checkBodyAndMeta(body, meta json.RawMessage) ([]string, bool) {
 
 		err := json.Unmarshal(meta, &fields)
 		if err != nil || fields == nil {
-			return nil, false
+			return false
 		}
 	}
 
-	return lines, true
+	return true
 }
 
 // respondIRCError enqueues a numeric error reply, notifies

internal/handlers/api_test.go

@@ -1001,8 +1001,6 @@ func TestChannelMessage(t *testing.T) {
 	}
 }
 
-// TestMessageMissingBody checks that a PRIVMSG with no body,
-// or an empty one, gets ERR_NOTEXTTOSEND.
 func TestMessageMissingBody(t *testing.T) {
 	t.Parallel()
 
@@ -1017,25 +1015,20 @@ func TestMessageMissingBody(t *testing.T) {
 
 	_, lastID := tserver.pollMessages(token, 0)
 
-	for _, request := range []map[string]any{
+	status, _ := tserver.sendCommand(token, map[string]any{
-		{commandKey: privmsgCmd, toKey: channel},
+		commandKey: privmsgCmd, toKey: channel,
-		{commandKey: privmsgCmd, toKey: channel, bodyKey: []string{}},
+	})
-	} {
+	if status != http.StatusOK {
-		status, _ := tserver.sendCommand(token, request)
+		t.Fatalf("expected 200, got %d", status)
-		if status != http.StatusOK {
+	}
-			t.Fatalf("%v: expected 200, got %d", request, status)
-		}
 
-		var msgs []map[string]any
+	msgs, _ := tserver.pollMessages(token, lastID)
 
-		msgs, lastID = tserver.pollMessages(token, lastID)
+	if !findNumeric(msgs, "412") {
-
+		t.Fatalf(
-		if !findNumeric(msgs, "412") {
+			"expected ERR_NOTEXTTOSEND (412), got %v",
-			t.Fatalf(
+			msgs,
-				"%v: expected ERR_NOTEXTTOSEND (412), got %v",
+		)
-				request, msgs,
-			)
-		}
 	}
 }