Apply MAX_MESSAGE_SIZE to the whole request body (closes #250)
check / check (push) Waiting to run
check / check (push) Waiting to run
Creating a session, logging in and posting a message stopped reading after the first JSON value, so the limit never saw the rest of the body and trailing bytes were ignored. All three now read the whole limited body and decode it with json.Unmarshal. A body over the limit, or anything but whitespace after the value, gets 400 invalid request body. Model: opus-5-5
This commit was merged in pull request #253.
This commit is contained in:
@@ -188,7 +188,7 @@ func (hdlr *Handlers) handleCreateSession(
|
||||
|
||||
var payload createRequest
|
||||
|
||||
err := json.NewDecoder(request.Body).Decode(&payload)
|
||||
err := decodeJSON(request, &payload)
|
||||
if err != nil {
|
||||
hdlr.respondError(
|
||||
writer, request,
|
||||
@@ -925,7 +925,7 @@ func (hdlr *Handlers) HandleSendCommand() http.HandlerFunc {
|
||||
|
||||
var payload commandRequest
|
||||
|
||||
err := json.NewDecoder(request.Body).Decode(&payload)
|
||||
err := decodeJSON(request, &payload)
|
||||
if err != nil {
|
||||
hdlr.respondError(
|
||||
writer, request,
|
||||
|
||||
@@ -797,6 +797,107 @@ func TestCreateSessionMalformed(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
func (tserver *testServer) postBody(
|
||||
path, token, body string,
|
||||
) (int, map[string]any) {
|
||||
tserver.t.Helper()
|
||||
|
||||
resp, err := doRequestAuth(
|
||||
tserver.t, http.MethodPost, tserver.url(path), token,
|
||||
strings.NewReader(body),
|
||||
)
|
||||
if err != nil {
|
||||
tserver.t.Fatalf("post %s: %v", path, err)
|
||||
}
|
||||
|
||||
defer func() { _ = resp.Body.Close() }()
|
||||
|
||||
var result map[string]any
|
||||
|
||||
_ = json.NewDecoder(resp.Body).Decode(&result)
|
||||
|
||||
return resp.StatusCode, result
|
||||
}
|
||||
|
||||
// TestRequestBodyLimit sends each endpoint a small valid JSON value
|
||||
// followed by other bytes. Only whitespace may follow the value, and
|
||||
// the whole body counts towards MAX_MESSAGE_SIZE.
|
||||
func TestRequestBodyLimit(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
const (
|
||||
maxMessageSize = 4096 // the default MAX_MESSAGE_SIZE
|
||||
nick = "alice_bodylimit"
|
||||
)
|
||||
|
||||
endpoints := []struct {
|
||||
name, path, value string
|
||||
acceptedStatus int
|
||||
}{
|
||||
{
|
||||
"create session", apiSession,
|
||||
`{"nick":"bob_bodylimit"}`,
|
||||
http.StatusCreated,
|
||||
},
|
||||
{
|
||||
"login", "/api/v1/login",
|
||||
`{"nick":"` + nick + `","password":"` + testPassword + `"}`,
|
||||
http.StatusOK,
|
||||
},
|
||||
{
|
||||
"post message", apiMessages,
|
||||
`{"command":"PING"}`,
|
||||
http.StatusOK,
|
||||
},
|
||||
}
|
||||
|
||||
for _, endpoint := range endpoints {
|
||||
t.Run(endpoint.name, func(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
tserver := newTestServer(t)
|
||||
token := tserver.createSession(nick)
|
||||
|
||||
tserver.sendCommand(token, map[string]any{
|
||||
commandKey: irc.CmdPass,
|
||||
bodyKey: []string{testPassword},
|
||||
})
|
||||
|
||||
bytesToLimit := maxMessageSize - len(endpoint.value)
|
||||
|
||||
for _, trailing := range []string{
|
||||
strings.Repeat(" ", bytesToLimit+1),
|
||||
strings.Repeat("x", bytesToLimit+1),
|
||||
"x",
|
||||
"{}",
|
||||
} {
|
||||
status, result := tserver.postBody(
|
||||
endpoint.path, token, endpoint.value+trailing,
|
||||
)
|
||||
if status != http.StatusBadRequest ||
|
||||
result["error"] != "invalid request body" {
|
||||
t.Errorf(
|
||||
"%d bytes of %.10q after the value: expected "+
|
||||
"400 invalid request body, got %d: %v",
|
||||
len(trailing), trailing, status, result,
|
||||
)
|
||||
}
|
||||
}
|
||||
|
||||
status, result := tserver.postBody(
|
||||
endpoint.path, token,
|
||||
endpoint.value+strings.Repeat(" ", bytesToLimit),
|
||||
)
|
||||
if status != endpoint.acceptedStatus {
|
||||
t.Errorf(
|
||||
"body of exactly the limit: expected %d, got %d: %v",
|
||||
endpoint.acceptedStatus, status, result,
|
||||
)
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
func TestAuthNoCookie(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
|
||||
@@ -1,7 +1,6 @@
|
||||
package handlers
|
||||
|
||||
import (
|
||||
"encoding/json"
|
||||
"net/http"
|
||||
"strings"
|
||||
|
||||
@@ -55,7 +54,7 @@ func (hdlr *Handlers) handleLogin(
|
||||
|
||||
var payload loginRequest
|
||||
|
||||
err := json.NewDecoder(request.Body).Decode(&payload)
|
||||
err := decodeJSON(request, &payload)
|
||||
if err != nil {
|
||||
hdlr.respondError(
|
||||
writer, request,
|
||||
|
||||
@@ -5,6 +5,7 @@ import (
|
||||
"context"
|
||||
"encoding/json"
|
||||
"errors"
|
||||
"io"
|
||||
"log/slog"
|
||||
"net/http"
|
||||
"time"
|
||||
@@ -159,6 +160,18 @@ func (hdlr *Handlers) respondError(
|
||||
)
|
||||
}
|
||||
|
||||
// decodeJSON reads the whole request body and decodes it into dest.
|
||||
// It fails if the body is larger than the http.MaxBytesReader limit,
|
||||
// or holds anything other than one JSON value and whitespace.
|
||||
func decodeJSON(request *http.Request, dest any) error {
|
||||
body, err := io.ReadAll(request.Body)
|
||||
if err != nil {
|
||||
return err
|
||||
}
|
||||
|
||||
return json.Unmarshal(body, dest)
|
||||
}
|
||||
|
||||
func (hdlr *Handlers) idleTimeout() time.Duration {
|
||||
raw := hdlr.params.Config.SessionIdleTimeout
|
||||
if raw == "" {
|
||||
|
||||
Reference in New Issue
Block a user