Skip to content

Commit fbf15bd

Browse files
vishrclaude
andcommitted
review: fix stale Reset comment, document dsw self-reference invariant, test cached bind errors
- context: correct newContext comment (Reset clears the store map, no longer nils it); document that only json()'s nested-guard may point the response at &c.dsw - test: deterministic cold-then-warm bind ensures the per-type cache preserves field-name conversion errors regardless of suite ordering Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent c8a1a93 commit fbf15bd

2 files changed

Lines changed: 35 additions & 2 deletions

File tree

bind_cache_test.go

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,31 @@
1+
// SPDX-License-Identifier: MIT
2+
// SPDX-FileCopyrightText: © 2015 LabStack LLC and Echo contributors
3+
4+
package echo
5+
6+
import (
7+
"net/http"
8+
"net/http/httptest"
9+
"testing"
10+
11+
"github.com/stretchr/testify/assert"
12+
)
13+
14+
// TestBindCachedMetaPreservesFieldNameError ensures the per-type bind metadata cache preserves the
15+
// field-name prefix in conversion errors on BOTH the cold (first) and warm (cached) bind of a type.
16+
// DTO is declared locally so its reflect.Type is independent of suite ordering, making the second
17+
// bind a deterministic cache hit (the bindMetaFor Load branch).
18+
func TestBindCachedMetaPreservesFieldNameError(t *testing.T) {
19+
type DTO struct {
20+
Number int `query:"number"`
21+
}
22+
bind := func() error {
23+
e := New()
24+
req := httptest.NewRequest(http.MethodGet, "/?number=10a", nil)
25+
var dto DTO
26+
return e.NewContext(req, httptest.NewRecorder()).Bind(&dto)
27+
}
28+
29+
assert.ErrorContains(t, bind(), "number", "cold cache: error must carry field name")
30+
assert.ErrorContains(t, bind(), "number", "warm cache: error must still carry field name")
31+
}

context.go

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -78,7 +78,9 @@ type Context struct {
7878
handler HandlerFunc
7979

8080
// dsw is reused by json() so that each JSON response does not heap-allocate a delayedStatusWriter.
81-
// It lives on the pooled Context; &c.dsw is a stable, allocation-free pointer.
81+
// It lives on the pooled Context; &c.dsw is a stable, allocation-free pointer. Only json() may point
82+
// the response at &c.dsw, and only via the nested-call guard there — aliasing it to itself (wrapping
83+
// &c.dsw around &c.dsw) would make the response writer reference itself.
8284
dsw delayedStatusWriter
8385

8486
store map[string]any
@@ -105,7 +107,7 @@ func NewContext(r *http.Request, w http.ResponseWriter, opts ...any) *Context {
105107
}
106108

107109
func newContext(r *http.Request, w http.ResponseWriter, e *Echo) *Context {
108-
// store is created lazily by Set (and reset to nil by Reset), so we deliberately do not allocate a map here.
110+
// store is created lazily by Set and cleared (not freed) by Reset, so we deliberately do not allocate a map here.
109111
c := &Context{
110112
pathValues: nil,
111113
echo: e,

0 commit comments

Comments
 (0)