diff --git a/bind_test.go b/bind_test.go index e33298b39..14061077f 100644 --- a/bind_test.go +++ b/bind_test.go @@ -834,7 +834,7 @@ func TestDefaultBinder_BindToStructFromMixedSources(t *testing.T) { givenURL: "/api/real_node/endpoint?node=xxx", givenContent: strings.NewReader(`{`), expect: &Opts{ID: 0, Node: "node_from_path"}, // query binding has already modified bind target - expectError: "code=400, message=Bad Request, err=unexpected EOF", + expectError: "code=400, message=Bad Request, err=unexpected end of JSON input", }, { name: "nok, GET with body bind failure when types are not convertible", @@ -1004,7 +1004,7 @@ func TestDefaultBinder_BindBody(t *testing.T) { givenContentType: MIMEApplicationJSON, givenContent: strings.NewReader(`{`), expect: &Node{ID: 0, Node: ""}, - expectError: "code=400, message=Bad Request, err=unexpected EOF", + expectError: "code=400, message=Bad Request, err=unexpected end of JSON input", }, { name: "ok, XML POST bind to struct with: path + query + empty body", diff --git a/json.go b/json.go index a969ccb8c..d5fc9294c 100644 --- a/json.go +++ b/json.go @@ -4,12 +4,25 @@ package echo import ( + "bytes" "encoding/json" + "sync" ) // DefaultJSONSerializer implements JSON encoding using encoding/json. type DefaultJSONSerializer struct{} +// jsonBufPool reuses buffers for reading request bodies during JSON +// deserialization, avoiding the per-request decoder and its internal read +// buffer that json.NewDecoder allocates. +var jsonBufPool = sync.Pool{New: newJSONBuf} + +func newJSONBuf() any { return new(bytes.Buffer) } + +// maxPooledJSONBuf caps the capacity of buffers returned to jsonBufPool so a +// single large request body cannot pin an oversized buffer in the pool. +const maxPooledJSONBuf = 1 << 16 // 64 KiB + // Serialize converts an interface into a json and writes it to the response. // You can optionally use the indent parameter to produce pretty JSONs. func (d DefaultJSONSerializer) Serialize(c *Context, target any, indent string) error { @@ -21,8 +34,29 @@ func (d DefaultJSONSerializer) Serialize(c *Context, target any, indent string) } // Deserialize reads a JSON from a request body and converts it into an interface. +// +// The body is read into a pooled buffer and decoded with json.Unmarshal rather +// than streaming through json.NewDecoder. This avoids allocating a decoder and +// its internal read buffer on every request. json.Unmarshal does not retain a +// reference to the input slice, so the buffer is safe to reuse afterwards. +// +// Note: the full request body is read into memory before decoding. As with any +// JSON parser, decoding untrusted input can allocate large amounts of memory; +// guard such endpoints with middleware.BodyLimit (or http.MaxBytesReader), +// which makes the body read here fail fast once the limit is exceeded. func (d DefaultJSONSerializer) Deserialize(c *Context, target any) error { - if err := json.NewDecoder(c.Request().Body).Decode(target); err != nil { + buf := jsonBufPool.Get().(*bytes.Buffer) + buf.Reset() + defer func() { + // Do not return oversized buffers to the pool — they would pin memory. + if buf.Cap() <= maxPooledJSONBuf { + jsonBufPool.Put(buf) + } + }() + if _, err := buf.ReadFrom(c.Request().Body); err != nil { + return ErrBadRequest.Wrap(err) + } + if err := json.Unmarshal(buf.Bytes(), target); err != nil { return ErrBadRequest.Wrap(err) } return nil diff --git a/json_test.go b/json_test.go index 1804b3e82..c369e41da 100644 --- a/json_test.go +++ b/json_test.go @@ -4,13 +4,26 @@ package echo import ( - "github.com/stretchr/testify/assert" + "errors" + "fmt" "net/http" "net/http/httptest" "strings" + "sync" "testing" + + "github.com/stretchr/testify/assert" ) +// deserializeJSON decodes body into target via the default serializer using a +// fresh context. It does not touch *testing.T so it is safe to call from +// goroutines (used by the concurrency test). +func deserializeJSON(e *Echo, body string, target any) error { + req := httptest.NewRequest(http.MethodPost, "/", strings.NewReader(body)) + c := e.NewContext(req, httptest.NewRecorder()) + return DefaultJSONSerializer{}.Deserialize(c, target) +} + // Note this test is deliberately simple as there's not a lot to test. // Just need to ensure it writes JSONs. The heavy work is done by the context methods. func TestDefaultJSONCodec_Encode(t *testing.T) { @@ -98,3 +111,106 @@ func TestDefaultJSONCodec_Decode(t *testing.T) { assert.EqualError(t, err, "code=400, message=Bad Request, err=json: cannot unmarshal number into Go struct field .id of type string") } + +// TestDefaultJSONCodec_Decode_RejectsTrailingData documents an intentional +// behavior change: Deserialize uses json.Unmarshal, which (unlike a streaming +// json.Decoder) rejects any content after the first top-level JSON value. +func TestDefaultJSONCodec_Decode_RejectsTrailingData(t *testing.T) { + e := New() + for _, body := range []string{ + userJSON + `{"id":2,"name":"second"}`, // a second JSON object + userJSON + ` trailing garbage`, // trailing non-JSON + userJSON + `,`, // trailing token + } { + var u user + err := deserializeJSON(e, body, &u) + if assert.Error(t, err, "body %q should be rejected", body) { + assert.IsType(t, &HTTPError{}, err) + assert.Equal(t, http.StatusBadRequest, err.(*HTTPError).Code) + } + } +} + +// TestDefaultJSONCodec_Decode_PooledBufferReuse guards against stale bytes +// bleeding between requests through the reused pooled buffer: a long body +// followed by a short one must each decode to exactly their own input. +func TestDefaultJSONCodec_Decode_PooledBufferReuse(t *testing.T) { + e := New() + for i := 0; i < 50; i++ { + longName := strings.Repeat("x", 1000+i) + var long user + err := deserializeJSON(e, fmt.Sprintf(`{"id":%d,"name":%q}`, i, longName), &long) + assert.NoError(t, err) + assert.Equal(t, user{ID: i, Name: longName}, long) + + var short user + err = deserializeJSON(e, `{"id":7,"name":"a"}`, &short) + assert.NoError(t, err) + assert.Equal(t, user{ID: 7, Name: "a"}, short) + } +} + +// TestDefaultJSONCodec_Decode_PooledBufferConcurrent exercises the pooled +// buffer from many goroutines at once; run under -race it catches any aliasing +// or missing-reset regression that would let one request's body corrupt another. +func TestDefaultJSONCodec_Decode_PooledBufferConcurrent(t *testing.T) { + e := New() + const n = 64 + var wg sync.WaitGroup + errs := make([]error, n) + got := make([]user, n) + for i := 0; i < n; i++ { + wg.Add(1) + go func(i int) { + defer wg.Done() + body := fmt.Sprintf(`{"id":%d,"name":%q}`, i, strings.Repeat("n", i+1)) + errs[i] = deserializeJSON(e, body, &got[i]) + }(i) + } + wg.Wait() + for i := 0; i < n; i++ { + assert.NoError(t, errs[i]) + assert.Equal(t, user{ID: i, Name: strings.Repeat("n", i+1)}, got[i]) + } +} + +// TestDefaultJSONCodec_Decode_LargeBodyThenNormal covers the buffer-cap path: a +// body larger than maxPooledJSONBuf must decode correctly, and its oversized +// buffer (dropped from the pool rather than retained) must not affect the next +// normal-sized request. +func TestDefaultJSONCodec_Decode_LargeBodyThenNormal(t *testing.T) { + e := New() + bigName := strings.Repeat("z", 100*1024) // 100 KiB > 64 KiB cap + var big user + err := deserializeJSON(e, fmt.Sprintf(`{"id":1,"name":%q}`, bigName), &big) + assert.NoError(t, err) + assert.Equal(t, user{ID: 1, Name: bigName}, big) + + var small user + err = deserializeJSON(e, userJSON, &small) + assert.NoError(t, err) + assert.Equal(t, user{ID: 1, Name: "Jon Snow"}, small) +} + +// errReader is an io.ReadCloser whose Read always fails, used to exercise the +// body-read error branch of Deserialize. +type errReader struct{} + +func (errReader) Read([]byte) (int, error) { return 0, errors.New("read failed") } +func (errReader) Close() error { return nil } + +// TestDefaultJSONCodec_Decode_BodyReadError verifies a failing request body read +// surfaces as a 400, matching the pre-existing decoder behavior. +func TestDefaultJSONCodec_Decode_BodyReadError(t *testing.T) { + e := New() + req := httptest.NewRequest(http.MethodPost, "/", http.NoBody) + req.Body = errReader{} + c := e.NewContext(req, httptest.NewRecorder()) + + var u user + err := DefaultJSONSerializer{}.Deserialize(c, &u) + if assert.Error(t, err) { + assert.IsType(t, &HTTPError{}, err) + assert.Equal(t, http.StatusBadRequest, err.(*HTTPError).Code) + } +} diff --git a/perf_bench_test.go b/perf_bench_test.go index b6ff0f640..8d6fecafd 100644 --- a/perf_bench_test.go +++ b/perf_bench_test.go @@ -111,6 +111,14 @@ type bindTarget struct { Active bool `json:"active" query:"active"` } +// nopReadCloser adapts a Reader to a ReadCloser without allocating, so the +// request body can be reset and reused across benchmark iterations instead of +// rebuilding the request (and its httptest machinery) inside the loop. +type nopReadCloser struct{ r *strings.Reader } + +func (n nopReadCloser) Read(p []byte) (int, error) { return n.r.Read(p) } +func (nopReadCloser) Close() error { return nil } + func BenchmarkBind_JSON(b *testing.B) { e := New() body := `{"id":1,"name":"Jon Snow","email":"jon@winterfell.north","age":24,"active":true}` @@ -118,17 +126,64 @@ func BenchmarkBind_JSON(b *testing.B) { var t bindTarget return c.Bind(&t) }) + // Build the request once and reset its body each iteration so the benchmark + // measures Echo's routing+binding cost, not httptest.NewRequest allocations. + r := strings.NewReader(body) + req := httptest.NewRequest(http.MethodPost, "/", r) + req.Header.Set(HeaderContentType, MIMEApplicationJSON) + req.Body = nopReadCloser{r} + w := &nopResponseWriter{} b.ReportAllocs() b.ResetTimer() - w := &nopResponseWriter{} for i := 0; i < b.N; i++ { w.h = nil - req := httptest.NewRequest(http.MethodPost, "/", strings.NewReader(body)) - req.Header.Set(HeaderContentType, MIMEApplicationJSON) + r.Reset(body) e.ServeHTTP(w, req) } } +// BenchmarkJSONSerialize/Deserialize exercise the DefaultJSONSerializer directly, +// isolating JSON encode/decode cost from routing and request construction. +func BenchmarkJSONDeserialize(b *testing.B) { + e := New() + body := `{"id":1,"name":"Jon Snow","email":"jon@winterfell.north","age":24,"active":true}` + r := strings.NewReader(body) + req := httptest.NewRequest(http.MethodPost, "/", r) + req.Body = nopReadCloser{r} + c := e.NewContext(req, &nopResponseWriter{}) + s := DefaultJSONSerializer{} + b.ReportAllocs() + b.ResetTimer() + for i := 0; i < b.N; i++ { + r.Reset(body) + var t bindTarget + if err := s.Deserialize(c, &t); err != nil { + b.Fatal(err) + } + } +} + +func BenchmarkJSONSerialize(b *testing.B) { + e := New() + type payload struct { + ID int `json:"id"` + Name string `json:"name"` + Tags []string + } + p := payload{ID: 1, Name: "Jon Snow", Tags: []string{"a", "b", "c"}} + w := &nopResponseWriter{} + c := e.NewContext(httptest.NewRequest(http.MethodGet, "/", nil), w) + s := DefaultJSONSerializer{} + b.ReportAllocs() + b.ResetTimer() + for i := 0; i < b.N; i++ { + w.h = nil + if err := s.Serialize(c, p, ""); err != nil { + b.Fatal(err) + } + } +} + func BenchmarkBind_Query(b *testing.B) { e := New() e.GET("/", func(c *Context) error {