diff --git a/README.md b/README.md index d9cef51..f1b0de3 100644 --- a/README.md +++ b/README.md @@ -643,8 +643,10 @@ Send a message to a channel or user. This is the primary messaging command. message echoed back via the queue). - If `to` is a bare nick, the message is a DM. The server fans out to the recipient and the sender (so all of the sender's clients see the DM). -- `body` must be a non-empty array of strings; otherwise the server sends - ERR_NOTEXTTOSEND (412). A missing `to` gets ERR_NORECIPIENT (411). +- A `body` that is not an array of strings, or a `meta` that is not an object, + gets HTTP 400 `invalid request body`, and nothing is relayed. A missing or + empty `body` gets ERR_NOTEXTTOSEND (412). A missing `to` gets ERR_NORECIPIENT + (411). - Only channel members can send to a channel. A non-member, a banned user, and on a `+m` channel a user with neither `+o` nor `+v`, get ERR_CANNOTSENDTOCHAN (404). @@ -1749,17 +1751,18 @@ command). **Numeric replies**, for success and for errors, are delivered through the message queue (see [Numeric Reply Codes](#numeric-reply-codes-s2c-only)); a command that has none, such as a successful `PRIVMSG`, is answered by the HTTP response alone, and `PING`'s `PONG` is the HTTP response body. HTTP error codes -(4xx/5xx) are reserved for transport-level problems: malformed JSON (400), +(4xx/5xx) are reserved for transport-level problems: a malformed request (400), missing/invalid auth cookies (401), and server errors (500). **HTTP errors (transport-level only):** -| Status | Error | When | -| ------ | ---------------------- | ------------------------------------------------------- | -| 400 | `invalid request body` | Malformed JSON, or a body over `MAX_MESSAGE_SIZE` bytes | -| 400 | `command required` | Empty command | -| 401 | `not registered` | Missing or invalid auth cookie | -| 500 | `internal error` | Server-side failure | +| Status | Error | When | +| ------ | ---------------------- | ------------------------------------------------------------------------------------------------- | +| 400 | `invalid request body` | Malformed JSON, or a body over `MAX_MESSAGE_SIZE` bytes | +| 400 | `invalid request body` | A `PRIVMSG` or `NOTICE` whose `body` is not an array of strings, or whose `meta` is not an object | +| 400 | `command required` | Empty command | +| 401 | `not registered` | Missing or invalid auth cookie | +| 500 | `internal error` | Server-side failure | **IRC numeric error replies (delivered via message queue):** diff --git a/internal/handlers/api.go b/internal/handlers/api.go index 1ec917b..0924d7a 100644 --- a/internal/handlers/api.go +++ b/internal/handlers/api.go @@ -994,7 +994,7 @@ func (hdlr *Handlers) dispatchCommand( hdlr.handlePrivmsg( writer, request, sessionID, clientID, nick, - command, target, body, meta, bodyLines, + command, target, body, meta, ) case irc.CmdJoin: hdlr.handleJoin( @@ -1139,8 +1139,18 @@ func (hdlr *Handlers) handlePrivmsg( nick, command, target string, body json.RawMessage, meta json.RawMessage, - bodyLines func() []string, ) { + lines, ok := checkBodyAndMeta(body, meta) + if !ok { + hdlr.respondError( + writer, request, + "invalid request body", + http.StatusBadRequest, + ) + + return + } + if target == "" { hdlr.enqueueNumeric( request.Context(), clientID, @@ -1155,7 +1165,6 @@ func (hdlr *Handlers) handlePrivmsg( return } - lines := bodyLines() if len(lines) == 0 { hdlr.enqueueNumeric( request.Context(), clientID, @@ -1189,6 +1198,44 @@ func (hdlr *Handlers) handlePrivmsg( ) } +// checkBodyAndMeta returns the lines in body. It returns false +// when body is present and is not an array of strings, or meta +// is present and is not an object. Each line is decoded as any +// because decoding into []string turns a null line into "". A +// null body or meta decodes without an error into nil. +func checkBodyAndMeta(body, meta json.RawMessage) ([]string, bool) { + var lines []string + + if body != nil { + var values []any + + err := json.Unmarshal(body, &values) + if err != nil || values == nil { + return nil, false + } + + for _, value := range values { + line, isString := value.(string) + if !isString { + return nil, false + } + + lines = append(lines, line) + } + } + + if meta != nil { + var fields map[string]any + + err := json.Unmarshal(meta, &fields) + if err != nil || fields == nil { + return nil, false + } + } + + return lines, true +} + // respondIRCError enqueues a numeric error reply, notifies // the broker, and sends HTTP 200 with {"status":"error"}. func (hdlr *Handlers) respondIRCError( diff --git a/internal/handlers/api_test.go b/internal/handlers/api_test.go index f3642ca..728f8c9 100644 --- a/internal/handlers/api_test.go +++ b/internal/handlers/api_test.go @@ -8,10 +8,12 @@ import ( "errors" "fmt" "io" + "maps" "net/http" "net/http/httptest" "net/url" "os" + "reflect" "slices" "strconv" "strings" @@ -999,6 +1001,8 @@ 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() @@ -1013,20 +1017,25 @@ func TestMessageMissingBody(t *testing.T) { _, lastID := tserver.pollMessages(token, 0) - status, _ := tserver.sendCommand(token, map[string]any{ - commandKey: privmsgCmd, toKey: channel, - }) - if status != http.StatusOK { - t.Fatalf("expected 200, got %d", status) - } + for _, request := range []map[string]any{ + {commandKey: privmsgCmd, toKey: channel}, + {commandKey: privmsgCmd, toKey: channel, bodyKey: []string{}}, + } { + status, _ := tserver.sendCommand(token, request) + if status != http.StatusOK { + t.Fatalf("%v: expected 200, got %d", request, status) + } - msgs, _ := tserver.pollMessages(token, lastID) + var msgs []map[string]any - if !findNumeric(msgs, "412") { - t.Fatalf( - "expected ERR_NOTEXTTOSEND (412), got %v", - msgs, - ) + msgs, lastID = tserver.pollMessages(token, lastID) + + if !findNumeric(msgs, "412") { + t.Fatalf( + "%v: expected ERR_NOTEXTTOSEND (412), got %v", + request, msgs, + ) + } } } @@ -1056,6 +1065,132 @@ func TestMessageMissingTo(t *testing.T) { } } +// TestMessageInvalidBodyOrMeta checks that a PRIVMSG or +// NOTICE, to a channel or a nick, whose body is not an array +// of strings or whose meta is not an object gets 400 and +// reaches nobody. +func TestMessageInvalidBodyOrMeta(t *testing.T) { + t.Parallel() + + cases := []struct { + name string + fields map[string]any + }{ + {"null line", map[string]any{bodyKey: []any{"hi", nil}}}, + {"number line", map[string]any{bodyKey: []any{"hi", 1}}}, + {"object line", map[string]any{bodyKey: []any{map[string]any{}}}}, + {"null body", map[string]any{bodyKey: nil}}, + {"object body", map[string]any{bodyKey: map[string]any{}}}, + {"null meta", map[string]any{bodyKey: []string{"hi"}, metaKey: nil}}, + {"array meta", map[string]any{bodyKey: []string{"hi"}, metaKey: []string{}}}, + {"string meta", map[string]any{bodyKey: []string{"hi"}, metaKey: "hi"}}, + } + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + + const channel = "#badmsg" + + tserver := newTestServer(t) + aliceToken := tserver.createSession("alice_badmsg") + bobToken := tserver.createSession("bob_badmsg") + + tserver.sendCommand(aliceToken, map[string]any{ + commandKey: joinCmd, toKey: channel, + }) + tserver.sendCommand(bobToken, map[string]any{ + commandKey: joinCmd, toKey: channel, + }) + + _, bobLastID := tserver.pollMessages(bobToken, 0) + + for _, command := range []string{privmsgCmd, irc.CmdNotice} { + for _, target := range []string{channel, "bob_badmsg"} { + request := map[string]any{ + commandKey: command, toKey: target, + } + maps.Copy(request, tc.fields) + + status, result := tserver.sendCommand( + aliceToken, request, + ) + if status != http.StatusBadRequest || + result["error"] != "invalid request body" { + t.Errorf( + "%s to %s: expected 400 invalid "+ + "request body, got %d: %v", + command, target, status, result, + ) + } + } + } + + msgs, _ := tserver.pollMessages(bobToken, bobLastID) + if len(msgs) != 0 { + t.Fatalf("expected no messages, bob got %v", msgs) + } + }) + } +} + +// TestMessageRelaysBodyAndMeta checks that a PRIVMSG or +// NOTICE, to a channel or a nick, reaches its recipient with +// body and meta as sent. +func TestMessageRelaysBodyAndMeta(t *testing.T) { + t.Parallel() + + const channel = "#relaymsg" + + tserver := newTestServer(t) + aliceToken := tserver.createSession("alice_relaymsg") + bobToken := tserver.createSession("bob_relaymsg") + + tserver.sendCommand(aliceToken, map[string]any{ + commandKey: joinCmd, toKey: channel, + }) + tserver.sendCommand(bobToken, map[string]any{ + commandKey: joinCmd, toKey: channel, + }) + + _, bobLastID := tserver.pollMessages(bobToken, 0) + + body := []any{"line one", ""} + meta := map[string]any{"sig": "abc", "alg": "ed25519"} + + for _, command := range []string{privmsgCmd, irc.CmdNotice} { + for _, target := range []string{channel, "bob_relaymsg"} { + status, result := tserver.sendCommand( + aliceToken, + map[string]any{ + commandKey: command, toKey: target, + bodyKey: body, metaKey: meta, + }, + ) + if status != http.StatusOK { + t.Fatalf( + "%s to %s: expected 200, got %d: %v", + command, target, status, result, + ) + } + + var msgs []map[string]any + + msgs, bobLastID = tserver.pollMessages( + bobToken, bobLastID, + ) + if len(msgs) != 1 || + msgs[0][commandKey] != command || + !reflect.DeepEqual(msgs[0][bodyKey], body) || + !reflect.DeepEqual(msgs[0][metaKey], meta) { + t.Fatalf( + "%s to %s: bob got %v", command, target, msgs, + ) + } + } + } +} + func TestNonMemberCannotSend(t *testing.T) { t.Parallel()