fix: input validation gaps (#68)
- enforce server-side body cap via http.MaxBytesReader: oversized JSON bodies are rejected with 413 instead of being fully decoded first - negative burn_after_reads rejected with 400 (zero still = default 1) - limit=0 explicitly maps to default page size; shared parseLimit clamp for /api/public and /api/mine (huge/non-numeric values too) - negative/non-numeric offset clamped to 0 via parseOffset (was pass-through) - regression tests in issue68_validation_test.go expires_at/expires_in validation intentionally excluded: covered by #60.
This commit is contained in:
@@ -0,0 +1,198 @@
|
|||||||
|
package api
|
||||||
|
|
||||||
|
// Regression tests for #68 input-validation gaps: negative/oversized content
|
||||||
|
// lengths (413), limit=0 → default page size, unchecked query params
|
||||||
|
// (negative offset), and negative burn_after_reads.
|
||||||
|
|
||||||
|
import (
|
||||||
|
"encoding/json"
|
||||||
|
"fmt"
|
||||||
|
"net/http"
|
||||||
|
"net/http/httptest"
|
||||||
|
"strings"
|
||||||
|
"testing"
|
||||||
|
)
|
||||||
|
|
||||||
|
func createPasteRaw(t *testing.T, h http.Handler, body string) *httptest.ResponseRecorder {
|
||||||
|
t.Helper()
|
||||||
|
req := httptest.NewRequest("POST", "/api/pastes", strings.NewReader(body))
|
||||||
|
rec := httptest.NewRecorder()
|
||||||
|
h.ServeHTTP(rec, req)
|
||||||
|
return rec
|
||||||
|
}
|
||||||
|
|
||||||
|
// A request whose decoded content exceeds the admin-tunable cap is rejected
|
||||||
|
// with 413 and a clear message (content under the body cap, over the
|
||||||
|
// content cap).
|
||||||
|
func TestCreatePasteContentOverCap413(t *testing.T) {
|
||||||
|
s := testServer(t)
|
||||||
|
h := s.routes()
|
||||||
|
|
||||||
|
// content over MaxContentBytes (5MiB) but under body cap (+4KiB): send
|
||||||
|
// just over the content cap so the per-field check fires first.
|
||||||
|
content := strings.Repeat("a", 5*1024*1024+10)
|
||||||
|
rec := createPasteRaw(t, h, fmt.Sprintf(`{"content":"%s"}`, content))
|
||||||
|
if rec.Code != http.StatusRequestEntityTooLarge {
|
||||||
|
t.Fatalf("got %d want 413: %s", rec.Code, rec.Body.String())
|
||||||
|
}
|
||||||
|
if !strings.Contains(rec.Body.String(), "content exceeds max") {
|
||||||
|
t.Fatalf("unclear error message: %s", rec.Body.String())
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// A request whose entire body is larger than the server-side body cap is cut
|
||||||
|
// off by http.MaxBytesReader and answered with 413, not decoded into memory
|
||||||
|
// (#68: previously a giant body was fully buffered, then rejected only at
|
||||||
|
// the per-field check — actually the decode happened before any check).
|
||||||
|
func TestCreatePasteBodyOverCap413(t *testing.T) {
|
||||||
|
s := testServer(t)
|
||||||
|
// shrink the content cap so the body cap is small too
|
||||||
|
ss := s.settings.get()
|
||||||
|
ss.MaxContentBytes = 64 * 1024
|
||||||
|
if err := s.settings.set(ss); err != nil {
|
||||||
|
t.Fatal(err)
|
||||||
|
}
|
||||||
|
h := s.routes()
|
||||||
|
|
||||||
|
content := strings.Repeat("a", 200*1024) // 200KiB > 64KiB+4KiB body cap
|
||||||
|
rec := createPasteRaw(t, h, fmt.Sprintf(`{"content":"%s","title":"x"}`, content))
|
||||||
|
if rec.Code != http.StatusRequestEntityTooLarge {
|
||||||
|
t.Fatalf("got %d want 413: %s", rec.Code, rec.Body.String())
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// Negative content lengths cannot be expressed via JSON, but a negative
|
||||||
|
// expires-style numeric payload must not crash; more importantly the
|
||||||
|
// burn_after_reads field: negative values are rejected with a clear message.
|
||||||
|
func TestCreatePasteNegativeBurnAfterReads(t *testing.T) {
|
||||||
|
s := testServer(t)
|
||||||
|
h := s.routes()
|
||||||
|
|
||||||
|
rec := createPasteRaw(t, h, `{"content":"hi","burn_after_reads":-5}`)
|
||||||
|
if rec.Code != http.StatusBadRequest {
|
||||||
|
t.Fatalf("got %d want 400: %s", rec.Code, rec.Body.String())
|
||||||
|
}
|
||||||
|
if !strings.Contains(rec.Body.String(), "burn_after_reads") {
|
||||||
|
t.Fatalf("unclear error message: %s", rec.Body.String())
|
||||||
|
}
|
||||||
|
|
||||||
|
// 0 and positive values still work (0 = default single read, per #49)
|
||||||
|
rec = createPasteRaw(t, h, `{"content":"hi","burn_after_reads":0,"burn_after_read":true}`)
|
||||||
|
if rec.Code != http.StatusCreated {
|
||||||
|
t.Fatalf("zero burn_after_reads: got %d want 201: %s", rec.Code, rec.Body.String())
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// limit=0 on list endpoints returns the default page size (existing clamp
|
||||||
|
// treats <=0 as default; #68 asks this be explicit and tested).
|
||||||
|
func TestListLimitZeroUsesDefault(t *testing.T) {
|
||||||
|
s := testServer(t)
|
||||||
|
h := s.routes()
|
||||||
|
|
||||||
|
// seed 3 public pastes
|
||||||
|
for i := 0; i < 3; i++ {
|
||||||
|
rec := createPasteRaw(t, h, fmt.Sprintf(`{"content":"p%d"}`, i))
|
||||||
|
if rec.Code != 201 {
|
||||||
|
t.Fatalf("seed: got %d: %s", rec.Code, rec.Body.String())
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
for _, q := range []string{"/api/public?limit=0", "/api/public"} {
|
||||||
|
req := httptest.NewRequest("GET", q, nil)
|
||||||
|
rec := httptest.NewRecorder()
|
||||||
|
h.ServeHTTP(rec, req)
|
||||||
|
if rec.Code != 200 {
|
||||||
|
t.Fatalf("%s: got %d", q, rec.Code)
|
||||||
|
}
|
||||||
|
var got struct {
|
||||||
|
Limit int `json:"limit"`
|
||||||
|
Total int `json:"total"`
|
||||||
|
Items []struct{ ID string } `json:"items"`
|
||||||
|
}
|
||||||
|
json.Unmarshal(rec.Body.Bytes(), &got)
|
||||||
|
if got.Limit != 25 || len(got.Items) != 3 {
|
||||||
|
t.Fatalf("%s: limit=%d items=%d, want limit 25 and all 3 items", q, got.Limit, len(got.Items))
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// Huge limit values are clamped to the max page size (already the behavior;
|
||||||
|
// regression-tested here per #68 "validate unchecked params").
|
||||||
|
func TestListLimitHugeClamped(t *testing.T) {
|
||||||
|
s := testServer(t)
|
||||||
|
h := s.routes()
|
||||||
|
|
||||||
|
req := httptest.NewRequest("GET", "/api/public?limit=999999999", nil)
|
||||||
|
rec := httptest.NewRecorder()
|
||||||
|
h.ServeHTTP(rec, req)
|
||||||
|
var got struct {
|
||||||
|
Limit int `json:"limit"`
|
||||||
|
}
|
||||||
|
json.Unmarshal(rec.Body.Bytes(), &got)
|
||||||
|
if got.Limit != 25 {
|
||||||
|
t.Fatalf("limit=%d, want clamped to 25", got.Limit)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// Negative offset previously passed through unchecked to SQL (harmless in
|
||||||
|
// SQLite, but invalid); it must be clamped to 0 (#68).
|
||||||
|
func TestListNegativeOffsetClamped(t *testing.T) {
|
||||||
|
s := testServer(t)
|
||||||
|
h := s.routes()
|
||||||
|
|
||||||
|
for i := 0; i < 2; i++ {
|
||||||
|
rec := createPasteRaw(t, h, fmt.Sprintf(`{"content":"p%d"}`, i))
|
||||||
|
if rec.Code != 201 {
|
||||||
|
t.Fatalf("seed: got %d", rec.Code)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
req := httptest.NewRequest("GET", "/api/public?offset=-999", nil)
|
||||||
|
rec := httptest.NewRecorder()
|
||||||
|
h.ServeHTTP(rec, req)
|
||||||
|
if rec.Code != 200 {
|
||||||
|
t.Fatalf("got %d", rec.Code)
|
||||||
|
}
|
||||||
|
var got struct {
|
||||||
|
Offset int `json:"offset"`
|
||||||
|
Total int `json:"total"`
|
||||||
|
}
|
||||||
|
json.Unmarshal(rec.Body.Bytes(), &got)
|
||||||
|
if got.Offset != 0 || got.Total != 2 {
|
||||||
|
t.Fatalf("offset=%d total=%d, want offset 0 and total 2", got.Offset, got.Total)
|
||||||
|
}
|
||||||
|
|
||||||
|
// /api/mine too (needs the viewer cookie)
|
||||||
|
req = httptest.NewRequest("GET", "/api/mine?offset=-5", nil)
|
||||||
|
req.AddCookie(&http.Cookie{Name: "vwr", Value: "offclamp"})
|
||||||
|
rec = httptest.NewRecorder()
|
||||||
|
h.ServeHTTP(rec, req)
|
||||||
|
var mine struct {
|
||||||
|
Offset int `json:"offset"`
|
||||||
|
}
|
||||||
|
json.Unmarshal(rec.Body.Bytes(), &mine)
|
||||||
|
if mine.Offset != 0 {
|
||||||
|
t.Fatalf("mine offset=%d, want 0", mine.Offset)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// Non-numeric limit/offset fall back to defaults instead of 500s.
|
||||||
|
func TestListGarbageParams(t *testing.T) {
|
||||||
|
s := testServer(t)
|
||||||
|
h := s.routes()
|
||||||
|
|
||||||
|
req := httptest.NewRequest("GET", "/api/public?limit=abc&offset=xyz", nil)
|
||||||
|
rec := httptest.NewRecorder()
|
||||||
|
h.ServeHTTP(rec, req)
|
||||||
|
if rec.Code != 200 {
|
||||||
|
t.Fatalf("got %d", rec.Code)
|
||||||
|
}
|
||||||
|
var got struct {
|
||||||
|
Limit int `json:"limit"`
|
||||||
|
Offset int `json:"offset"`
|
||||||
|
}
|
||||||
|
json.Unmarshal(rec.Body.Bytes(), &got)
|
||||||
|
if got.Limit != 25 || got.Offset != 0 {
|
||||||
|
t.Fatalf("limit=%d offset=%d, want 25/0", got.Limit, got.Offset)
|
||||||
|
}
|
||||||
|
}
|
||||||
+16
-15
@@ -59,6 +59,7 @@ func (a *apiServer) routes() http.Handler {
|
|||||||
r := chi.NewRouter()
|
r := chi.NewRouter()
|
||||||
r.Use(middleware.Recoverer)
|
r.Use(middleware.Recoverer)
|
||||||
r.Use(middleware.Timeout(30 * time.Second))
|
r.Use(middleware.Timeout(30 * time.Second))
|
||||||
|
r.Use(a.limitRequestBody) // #68: hard server-side body cap -> 413
|
||||||
r.Use(viewerCookieMiddleware)
|
r.Use(viewerCookieMiddleware)
|
||||||
|
|
||||||
// admin (#40): HTML page is open (key entry via form); API is key-guarded
|
// admin (#40): HTML page is open (key entry via form); API is key-guarded
|
||||||
@@ -152,16 +153,22 @@ func (a *apiServer) handleCreatePaste(w http.ResponseWriter, r *http.Request) {
|
|||||||
}
|
}
|
||||||
var p store.Paste
|
var p store.Paste
|
||||||
if err := json.NewDecoder(r.Body).Decode(&p); err != nil {
|
if err := json.NewDecoder(r.Body).Decode(&p); err != nil {
|
||||||
|
if isBodyTooLarge(err) { // #68: body cut off by MaxBytesReader
|
||||||
|
writeBodyTooLarge(w)
|
||||||
|
return
|
||||||
|
}
|
||||||
writeErr(w, 400, "invalid json body")
|
writeErr(w, 400, "invalid json body")
|
||||||
return
|
return
|
||||||
}
|
}
|
||||||
if strings.TrimSpace(p.Content) == "" {
|
if status, msg := checkContent(p.Content, s.MaxContentBytes); status != 0 {
|
||||||
writeErr(w, 400, "content is required")
|
writeErr(w, status, msg)
|
||||||
return
|
return
|
||||||
}
|
}
|
||||||
if int64(len(p.Content)) > s.MaxContentBytes { // #40: admin-tunable
|
if p.BurnAfterReads != nil { // #68: reject negative read budgets
|
||||||
writeErr(w, 413, fmt.Sprintf("content exceeds max %d bytes", s.MaxContentBytes))
|
if err := parseBurnAfterReads(*p.BurnAfterReads); err != nil {
|
||||||
return
|
writeErr(w, 400, err.Error())
|
||||||
|
return
|
||||||
|
}
|
||||||
}
|
}
|
||||||
// #40: admin-configurable default expiry
|
// #40: admin-configurable default expiry
|
||||||
if (p.ExpiresIn == nil || *p.ExpiresIn == "") && s.DefaultExpiry != "" {
|
if (p.ExpiresIn == nil || *p.ExpiresIn == "") && s.DefaultExpiry != "" {
|
||||||
@@ -254,11 +261,8 @@ func (a *apiServer) handleListMine(w http.ResponseWriter, r *http.Request) {
|
|||||||
writeJSON(w, 200, map[string]any{"total": 0, "items": []any{}})
|
writeJSON(w, 200, map[string]any{"total": 0, "items": []any{}})
|
||||||
return
|
return
|
||||||
}
|
}
|
||||||
limit, _ := strconv.Atoi(r.URL.Query().Get("limit"))
|
limit := parseLimit(r, 50, 100)
|
||||||
if limit <= 0 || limit > 100 {
|
offset := parseOffset(r)
|
||||||
limit = 50
|
|
||||||
}
|
|
||||||
offset, _ := strconv.Atoi(r.URL.Query().Get("offset"))
|
|
||||||
rows, total, err := a.store.ListMine(vid, limit, offset)
|
rows, total, err := a.store.ListMine(vid, limit, offset)
|
||||||
if err != nil {
|
if err != nil {
|
||||||
writeErr(w, 500, "db error")
|
writeErr(w, 500, "db error")
|
||||||
@@ -277,11 +281,8 @@ func (a *apiServer) handleListMine(w http.ResponseWriter, r *http.Request) {
|
|||||||
}
|
}
|
||||||
|
|
||||||
func (a *apiServer) handleListPublic(w http.ResponseWriter, r *http.Request) {
|
func (a *apiServer) handleListPublic(w http.ResponseWriter, r *http.Request) {
|
||||||
limit, _ := strconv.Atoi(r.URL.Query().Get("limit"))
|
limit := parseLimit(r, 25, 100)
|
||||||
if limit <= 0 || limit > 100 {
|
offset := parseOffset(r)
|
||||||
limit = 25
|
|
||||||
}
|
|
||||||
offset, _ := strconv.Atoi(r.URL.Query().Get("offset"))
|
|
||||||
rows, total, err := a.store.ListPublic(limit, offset)
|
rows, total, err := a.store.ListPublic(limit, offset)
|
||||||
if err != nil {
|
if err != nil {
|
||||||
writeErr(w, 500, "db error")
|
writeErr(w, 500, "db error")
|
||||||
|
|||||||
@@ -0,0 +1,92 @@
|
|||||||
|
package api
|
||||||
|
|
||||||
|
import (
|
||||||
|
"errors"
|
||||||
|
"fmt"
|
||||||
|
"net/http"
|
||||||
|
"strconv"
|
||||||
|
"strings"
|
||||||
|
)
|
||||||
|
|
||||||
|
// #68 input-validation helpers. Paste/can payloads are size-capped and list
|
||||||
|
// endpoints get a single place where limit/offset are parsed and clamped.
|
||||||
|
|
||||||
|
// maxRequestBody returns the HTTP body cap for JSON create endpoints: the
|
||||||
|
// admin-tunable content cap plus headroom for JSON field overhead, floored
|
||||||
|
// at 64KiB so a tiny admin-configured cap can't break small requests.
|
||||||
|
func (a *apiServer) maxRequestBody() int64 {
|
||||||
|
s := a.settings.get()
|
||||||
|
max := s.MaxContentBytes + 4096
|
||||||
|
if max < 64*1024 {
|
||||||
|
max = 64 * 1024
|
||||||
|
}
|
||||||
|
return max
|
||||||
|
}
|
||||||
|
|
||||||
|
// limitRequestBody wraps the request body with http.MaxBytesReader so
|
||||||
|
// oversized payloads are cut off server-side instead of being fully decoded
|
||||||
|
// into memory before the per-field size check runs (#68). A read over the
|
||||||
|
// cap surfaces as *http.MaxBytesError, which handlers map to 413.
|
||||||
|
func (a *apiServer) limitRequestBody(next http.Handler) http.Handler {
|
||||||
|
return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
|
||||||
|
if r.Body != nil {
|
||||||
|
r.Body = http.MaxBytesReader(w, r.Body, a.maxRequestBody())
|
||||||
|
}
|
||||||
|
next.ServeHTTP(w, r)
|
||||||
|
})
|
||||||
|
}
|
||||||
|
|
||||||
|
// writeBodyTooLarge emits the 413 response for a body rejected by
|
||||||
|
// MaxBytesReader.
|
||||||
|
func writeBodyTooLarge(w http.ResponseWriter) {
|
||||||
|
writeErr(w, http.StatusRequestEntityTooLarge, "request body too large")
|
||||||
|
}
|
||||||
|
|
||||||
|
// isBodyTooLarge reports whether err came from http.MaxBytesReader.
|
||||||
|
func isBodyTooLarge(err error) bool {
|
||||||
|
var mbe *http.MaxBytesError
|
||||||
|
return errors.As(err, &mbe)
|
||||||
|
}
|
||||||
|
|
||||||
|
// checkContent validates paste content: rejects whitespace-only content
|
||||||
|
// (400) and content over the byte cap (413). Returns (0, "") when valid.
|
||||||
|
func checkContent(content string, maxBytes int64) (int, string) {
|
||||||
|
if strings.TrimSpace(content) == "" {
|
||||||
|
return http.StatusBadRequest, "content is required"
|
||||||
|
}
|
||||||
|
if int64(len(content)) > maxBytes { // #40/#68: admin-tunable cap
|
||||||
|
return http.StatusRequestEntityTooLarge,
|
||||||
|
fmt.Sprintf("content exceeds max %d bytes", maxBytes)
|
||||||
|
}
|
||||||
|
return 0, ""
|
||||||
|
}
|
||||||
|
|
||||||
|
// parseLimit clamps the ?limit query param: missing/non-numeric/zero/negative
|
||||||
|
// or over-max values fall back to def. Zero intentionally maps to the default
|
||||||
|
// page size, matching the pre-existing `<= 0` clamp (#68).
|
||||||
|
func parseLimit(r *http.Request, def, max int) int {
|
||||||
|
n, err := strconv.Atoi(r.URL.Query().Get("limit"))
|
||||||
|
if err != nil || n <= 0 || n > max {
|
||||||
|
return def
|
||||||
|
}
|
||||||
|
return n
|
||||||
|
}
|
||||||
|
|
||||||
|
// parseOffset clamps the ?offset query param: missing/non-numeric or negative
|
||||||
|
// values become 0 (#68: negative offsets previously passed through to SQL).
|
||||||
|
func parseOffset(r *http.Request) int {
|
||||||
|
n, err := strconv.Atoi(r.URL.Query().Get("offset"))
|
||||||
|
if err != nil || n < 0 {
|
||||||
|
return 0
|
||||||
|
}
|
||||||
|
return n
|
||||||
|
}
|
||||||
|
|
||||||
|
// parseBurnAfterReads validates the burn_after_reads field (#68): negative
|
||||||
|
// values are rejected; zero/absent mean the default single read.
|
||||||
|
func parseBurnAfterReads(n int) error {
|
||||||
|
if n < 0 {
|
||||||
|
return errors.New("burn_after_reads must be a positive number")
|
||||||
|
}
|
||||||
|
return nil
|
||||||
|
}
|
||||||
Reference in New Issue
Block a user