fix(api): enforce api-body-limit before parsing multipart requests

pkg/modules/api/context.go

@@ -166,13 +166,17 @@ func newContext(echoCtx *echo.Context, logger *slog.Logger, fs *gotenberg.FileSy
 	// downloaded remotely ("download from" feature).
 	var totalBytesRead atomic.Int64
 
+	bodyLimitErr := func() error {
+		return WrapError(
+			fmt.Errorf("body limit reached (> %d)", bodyLimit),
+			NewSentinelHttpError(http.StatusRequestEntityTooLarge, "The request body exceeds the configured size limit. Increase it with --api-body-limit, or send a smaller request."),
+		)
+	}
+
 	addReadBytes := func(n int64) error {
 		newTotal := totalBytesRead.Add(n)
 		if bodyLimit != 0 && newTotal > bodyLimit {
-			return WrapError(
+			return bodyLimitErr()
-				fmt.Errorf("body limit reached (> %d)", bodyLimit),
-				NewSentinelHttpError(http.StatusRequestEntityTooLarge, "The request body exceeds the configured size limit. Increase it with --api-body-limit, or send a smaller request."),
-			)
 		}
 		return nil
 	}
@@ -217,8 +221,25 @@ func newContext(echoCtx *echo.Context, logger *slog.Logger, fs *gotenberg.FileSy
 		}
 	}()
 
+	// The multipart parser reads the whole request and spools file parts to
+	// disk before the counter above sees a single byte, so the limit must
+	// also bound the raw body itself. Content-Length allows an early
+	// rejection, but it may be absent or lie, hence the reader.
+	if bodyLimit > 0 {
+		req := echoCtx.Request()
+		if req.ContentLength > bodyLimit {
+			return nil, cancel, fmt.Errorf("check content length: %w", bodyLimitErr())
+		}
+		req.Body = http.MaxBytesReader(echoCtx.Response(), req.Body, bodyLimit)
+	}
+
 	form, err := echoCtx.MultipartForm()
 	if err != nil {
+		var maxBytesErr *http.MaxBytesError
+		if errors.As(err, &maxBytesErr) {
+			return nil, cancel, fmt.Errorf("get multipart form: %w", bodyLimitErr())
+		}
+
 		if errors.Is(err, http.ErrNotMultipart) {
 			return nil, cancel, WrapError(
 				fmt.Errorf("get multipart form: %w", err),

pkg/modules/api/context_test.go

@@ -6,6 +6,7 @@ import (
 	"encoding/json"
 	"errors"
 	"fmt"
+	"io"
 	"log/slog"
 	"mime/multipart"
 	"net/http"
@@ -902,3 +903,100 @@ func TestNewContext_DownloadFromRedirectVerdictStaysGeneric(t *testing.T) {
 		}
 	}
 }
+
+type countingReadCloser struct {
+	r io.Reader
+	n int64
+}
+
+func (c *countingReadCloser) Read(p []byte) (int, error) {
+	n, err := c.r.Read(p)
+	c.n += int64(n)
+	return n, err
+}
+
+func (*countingReadCloser) Close() error { return nil }
+
+// The body limit must stop the multipart parser itself. It used to apply
+// only after the parser had read the whole request and spooled file parts
+// to disk.
+func TestNewContext_BodyLimitBoundsIngress(t *testing.T) {
+	body := new(bytes.Buffer)
+	writer := multipart.NewWriter(body)
+	err := writer.WriteField("foo", "bar")
+	if err != nil {
+		t.Fatalf("write multipart field: %v", err)
+	}
+	part, err := writer.CreateFormFile("files", "large.bin")
+	if err != nil {
+		t.Fatalf("create multipart file: %v", err)
+	}
+	_, err = part.Write(bytes.Repeat([]byte("A"), 2<<20))
+	if err != nil {
+		t.Fatalf("write multipart file: %v", err)
+	}
+	err = writer.Close()
+	if err != nil {
+		t.Fatalf("close multipart writer: %v", err)
+	}
+	size := int64(body.Len())
+
+	for _, tc := range []struct {
+		scenario         string
+		bodyLimit        int64
+		knownLength      bool
+		expectErr        bool
+		maxBytesConsumed int64
+	}{
+		{scenario: "known length over the limit is rejected before reading", bodyLimit: 1024, knownLength: true, expectErr: true, maxBytesConsumed: 0},
+		{scenario: "unknown length over the limit stops at the limit", bodyLimit: 1024, knownLength: false, expectErr: true, maxBytesConsumed: 1024 + 1},
+		{scenario: "unknown length one byte over the limit", bodyLimit: size - 1, knownLength: false, expectErr: true, maxBytesConsumed: size},
+		{scenario: "body exactly at the limit", bodyLimit: size, knownLength: true, expectErr: false},
+		{scenario: "body exactly at the limit with unknown length", bodyLimit: size, knownLength: false, expectErr: false},
+		{scenario: "no limit", bodyLimit: 0, knownLength: false, expectErr: false},
+	} {
+		t.Run(tc.scenario, func(t *testing.T) {
+			counted := &countingReadCloser{r: bytes.NewReader(body.Bytes())}
+			req := httptest.NewRequest(http.MethodPost, "/forms/pdfengines/merge", nil)
+			req.Body = counted
+			req.ContentLength = -1
+			if tc.knownLength {
+				req.ContentLength = size
+			}
+			req.Header.Set("Content-Type", writer.FormDataContentType())
+
+			echoCtx := echo.New().NewContext(req, httptest.NewRecorder())
+			logger := slog.New(slog.DiscardHandler)
+			fs := gotenberg.NewFileSystem(new(gotenberg.OsMkdirAll))
+
+			_, cancel, err := newContext(echoCtx, logger, fs, 10*time.Second, tc.bodyLimit, downloadFromConfig{disable: true})
+			if cancel != nil {
+				defer cancel()
+			}
+
+			if !tc.expectErr {
+				if err != nil {
+					t.Fatalf("newContext returned error: %v", err)
+				}
+				if counted.n != size {
+					t.Fatalf("consumed %d bytes, want %d", counted.n, size)
+				}
+				return
+			}
+
+			if err == nil {
+				t.Fatal("expected the body limit to reject the request")
+			}
+			var httpErr HttpError
+			if !errors.As(err, &httpErr) {
+				t.Fatalf("expected an HttpError, got: %v", err)
+			}
+			if status, _ := httpErr.HttpError(); status != http.StatusRequestEntityTooLarge {
+				t.Fatalf("status = %d, want %d", status, http.StatusRequestEntityTooLarge)
+			}
+			if counted.n > tc.maxBytesConsumed {
+				t.Fatalf("consumed %d/%d bytes before rejection, want at most %d", counted.n, size, tc.maxBytesConsumed)
+			}
+		})
+	}
+}