fix(middleware): stop BodyLimit handing out more than the limit

middleware/body_limit.go

@@ -81,9 +81,30 @@ func (config BodyLimitConfig) ToMiddleware() (echo.MiddlewareFunc, error) {
 }
 
 func (r *limitedReader) Read(b []byte) (n int, err error) {
+	// Once the limit is known to be exceeded, stay refused. io.Reader's
+	// contract invites callers to process the n>0 bytes of a failed read and
+	// carry on, so a reader that keeps serving data after the first refusal
+	// hands out the whole body to anyone following that advice.
+	if r.read > r.LimitBytes {
+		return 0, echo.ErrStatusRequestEntityTooLarge
+	}
+
+	// Never read further than one byte past the limit: that byte is what
+	// proves the body is too large, and anything beyond it is data the caller
+	// asked for but is not allowed to have.
+	if max := r.LimitBytes - r.read + 1; int64(len(b)) > max {
+		b = b[:max]
+	}
+
 	n, err = r.reader.Read(b)
 	r.read += int64(n)
 	if r.read > r.LimitBytes {
+		// Hand back only what fits. The byte past the limit was read to prove
+		// the body is too large, not to be delivered, and a caller that
+		// processes n>0 before the error must not receive it.
+		if over := int(r.read - r.LimitBytes); over <= n {
+			n -= over
+		}
 		return n, echo.ErrStatusRequestEntityTooLarge
 	}
 	return

middleware/body_limit_test.go

@@ -93,6 +93,89 @@ func TestBodyLimitAfterDecompressUsesDecodedSize(t *testing.T) {
 	assert.Equal(t, body, rec.Body.String())
 }
 
+func TestBodyLimitReaderStopsAtTheLimit(t *testing.T) {
+	const limit = 5
+	body := bytes.Repeat([]byte("x"), 10*limit)
+
+	reader := &limitedReader{
+		BodyLimitConfig: BodyLimitConfig{Skipper: DefaultSkipper, LimitBytes: limit},
+		reader:          io.NopCloser(bytes.NewReader(body)),
+	}
+
+	// io.Reader asks callers to process the n>0 bytes of a read before
+	// treating its error as fatal, so keep reading the way such a caller
+	// would. No more than the limit may be handed over however long it goes on.
+	buf := make([]byte, 64)
+	var total int
+	for range 20 {
+		n, err := reader.Read(buf)
+		total += n
+		if n == 0 && err != nil {
+			break
+		}
+	}
+
+	assert.Equal(t, limit, total)
+}
+
+// countingReader records how much was asked of the source, which is not
+// visible from what the caller receives.
+type countingReader struct {
+	io.Reader
+	read  int64
+	calls int
+}
+
+func (c *countingReader) Read(b []byte) (int, error) {
+	c.calls++
+	n, err := c.Reader.Read(b)
+	c.read += int64(n)
+	return n, err
+}
+
+func (c *countingReader) Close() error { return nil }
+
+func TestBodyLimitReaderDoesNotOverdrawTheSource(t *testing.T) {
+	const limit = 5
+	src := &countingReader{Reader: bytes.NewReader(bytes.Repeat([]byte("x"), 1<<20))}
+
+	reader := &limitedReader{
+		BodyLimitConfig: BodyLimitConfig{Skipper: DefaultSkipper, LimitBytes: limit},
+		reader:          src,
+	}
+
+	buf := make([]byte, 64*1024)
+	_, _ = reader.Read(buf)
+	callsAtRefusal := src.calls
+
+	// One byte past the limit is enough to know the body is too large; a
+	// megabyte of it should never be pulled off the wire to find that out.
+	assert.LessOrEqual(t, src.read, int64(limit+1))
+
+	// And once refused, the source must not be touched again.
+	for range 5 {
+		_, _ = reader.Read(buf)
+	}
+	assert.Equal(t, callsAtRefusal, src.calls)
+}
+
+func TestBodyLimitReaderStaysRefused(t *testing.T) {
+	reader := &limitedReader{
+		BodyLimitConfig: BodyLimitConfig{Skipper: DefaultSkipper, LimitBytes: 2},
+		reader:          io.NopCloser(bytes.NewReader([]byte("Hello, World!"))),
+	}
+
+	_, err := io.ReadAll(reader)
+	he := err.(echo.HTTPStatusCoder)
+	assert.Equal(t, http.StatusRequestEntityTooLarge, he.StatusCode())
+
+	// Reading on after the refusal must not produce more of the body.
+	n, err := reader.Read(make([]byte, 8))
+	assert.Equal(t, 0, n)
+	he = err.(echo.HTTPStatusCoder)
+	assert.Equal(t, http.StatusRequestEntityTooLarge, he.StatusCode())
+}
+
 func TestBodyLimitReader(t *testing.T) {
 	hw := []byte("Hello, World!")