Refuse a PRIVMSG or NOTICE whose body or meta has the wrong type (closes #245)
check / check (push) Waiting to run
check / check (push) Waiting to run
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. The check returns the lines it accepted, and the empty-body check counts those, so body is decoded once. A missing or empty body still gets ERR_NOTEXTTOSEND. README.md says so. Model: opus-5-5
This commit is contained in:
@@ -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):**
|
||||
|
||||
|
||||
@@ -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(
|
||||
|
||||
+147
-12
@@ -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()
|
||||
|
||||
|
||||
Reference in New Issue
Block a user