Skip to content

Commit 571d095

Browse files
committed
fix: restore full request body for large payloads (closes #76)
1 parent d3f918c commit 571d095

File tree

3 files changed

+91
-1
lines changed

3 files changed

+91
-1
lines changed

CHANGELOG.md

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,10 @@ All notable changes to this project will be documented in this file.
55
The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
66
and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
77

8+
## [v0.1.5] - 2025-12-08
9+
### Fixed
10+
- Fixed critical bug where POST request bodies were lost or truncated by using `io.MultiReader` to restore the full body stream (fixes #76).
11+
812
## [v0.1.4] - 2025-12-06
913

1014
### Security

request.go

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -215,7 +215,17 @@ func (rve *RequestValueExtractor) extractBody(r *http.Request, target string) (s
215215
rve.logger.Error("Failed to read request body", zap.Error(err))
216216
return "", fmt.Errorf("failed to read request body for target %s: %w", target, err)
217217
}
218-
r.Body = io.NopCloser(strings.NewReader(string(bodyBytes))) // Restore body for next read
218+
// Restore body for next read, verifying if we need to combine with remaining body
219+
// We use io.MultiReader to concatenate the bytes we read with the *remaining* bytes in the original body.
220+
// This ensures that even if we hit the limit, the downstream consumer can read the full body.
221+
// We also ensure the original Closer is preserved.
222+
r.Body = &struct {
223+
io.Reader
224+
io.Closer
225+
}{
226+
Reader: io.MultiReader(strings.NewReader(string(bodyBytes)), r.Body),
227+
Closer: r.Body,
228+
}
219229

220230
// SOTA Pattern: Zero-Copy (avoid allocation for string conversion)
221231
if len(bodyBytes) == 0 {

request_body_test.go

Lines changed: 76 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,76 @@
1+
package caddywaf
2+
3+
import (
4+
"bytes"
5+
"io"
6+
"net/http/httptest"
7+
"strings"
8+
"testing"
9+
10+
"github.com/stretchr/testify/assert"
11+
"go.uber.org/zap"
12+
)
13+
14+
func TestMiddleware_RequestBodyRestoration(t *testing.T) {
15+
// Setup middleware
16+
// Create extractor
17+
logger := zap.NewNop()
18+
rve := NewRequestValueExtractor(logger, false, 1024*1024) // 1MB limit
19+
20+
t.Run("Body < MaxSize", func(t *testing.T) {
21+
bodyContent := "small body"
22+
req := httptest.NewRequest("POST", "/", strings.NewReader(bodyContent))
23+
24+
// Extract body (simulates WAF inspecting it)
25+
extracted, err := rve.ExtractValue(TargetBody, req, nil)
26+
assert.NoError(t, err)
27+
assert.Equal(t, bodyContent, extracted)
28+
29+
// Verify body is restored and readable again
30+
restoredBody, err := io.ReadAll(req.Body)
31+
assert.NoError(t, err)
32+
assert.Equal(t, bodyContent, string(restoredBody))
33+
})
34+
35+
t.Run("Body > MaxSize", func(t *testing.T) {
36+
// Max size is 1MB. Let's send 2MB.
37+
size := 2 * 1024 * 1024
38+
bodyContent := make([]byte, size)
39+
// Fill with some data
40+
for i := 0; i < size; i++ {
41+
bodyContent[i] = 'a'
42+
}
43+
44+
req := httptest.NewRequest("POST", "/", bytes.NewReader(bodyContent))
45+
46+
// Extract body
47+
extracted, err := rve.ExtractValue(TargetBody, req, nil)
48+
assert.NoError(t, err)
49+
50+
// Extracted should be truncated to 1MB
51+
assert.Equal(t, 1024*1024, len(extracted))
52+
53+
// Verify body restoration
54+
// If the implementation is naive, it might only restore the 1MB we read,
55+
// and the rest is lost because LimitReader consumed the prefix.
56+
// OR, if it restores using LimitReader's underlying reader, maybe it's fine?
57+
// Wait, LimitReader wraps the original request body.
58+
// We read from LimitReader.
59+
// If we replace req.Body with a new reader containing key read bytes...
60+
// The original req.Body (the socket/buffer) has been advanced by 1MB.
61+
// If we set req.Body = NewReader(readBytes), subsequent consumers will read 1MB and then EOF.
62+
// The remaining 1MB in the original req.Body is skipped/lost!
63+
64+
restored, err := io.ReadAll(req.Body)
65+
assert.NoError(t, err)
66+
67+
// This assertion is expected to FAIL if the bug exists for large bodies.
68+
// Use NotEqual or expect failure if we want to demonstrate the bug?
69+
// User says "POST request's body gone". They didn't specify size.
70+
// But let's see what happens.
71+
if len(restored) != size {
72+
t.Logf("Bug confirmed: Expected %d bytes, got %d", size, len(restored))
73+
}
74+
assert.Equal(t, size, len(restored))
75+
})
76+
}

0 commit comments

Comments
 (0)