Skip to content

Commit 6a0968c

Browse files
committed
add some guardrails and notes to the example code
1 parent f52af20 commit 6a0968c

File tree

2 files changed

+59
-23
lines changed

2 files changed

+59
-23
lines changed

examples/custom_middleware/example/jsonenforcer.go

Lines changed: 30 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,15 @@ import (
99
)
1010

1111
// ResponseWrapper captures response data for transformation
12+
//
13+
// This is a simple example for demonstration purposes and is not intended for
14+
// production use. Limitations:
15+
// - Does not preserve optional HTTP interfaces (http.Hijacker, http.Flusher, http.Pusher)
16+
// - Not safe for concurrent writes from multiple goroutines within the same request
17+
// - No memory limits on buffered content
18+
//
19+
// Each request gets its own ResponseWrapper instance, so different requests won't
20+
// interfere with each other.
1221
type ResponseWrapper struct {
1322
original httpserver.ResponseWriter
1423
buffer *bytes.Buffer
@@ -63,9 +72,8 @@ func (rw *ResponseWrapper) Size() int {
6372

6473
// transformToJSON wraps non-JSON content in a JSON response
6574
func transformToJSON(data []byte) ([]byte, error) {
66-
// First check if the data is valid JSON regardless of content type
67-
var test any
68-
if json.Unmarshal(data, &test) == nil {
75+
// Use json.Valid for efficient validation without unmarshaling
76+
if json.Valid(data) {
6977
return data, nil // Valid JSON, return as-is
7078
}
7179

@@ -91,6 +99,25 @@ func New() httpserver.HandlerFunc {
9199

92100
// RESPONSE PHASE: Transform response to JSON
93101
originalData := wrapper.buffer.Bytes()
102+
statusCode := wrapper.Status()
103+
if statusCode == 0 {
104+
statusCode = http.StatusOK
105+
}
106+
107+
// Copy headers to original writer
108+
originalWriter := wrapper.original
109+
for key, values := range wrapper.Header() {
110+
for _, value := range values {
111+
originalWriter.Header().Add(key, value)
112+
}
113+
}
114+
115+
// Check if this status code should have no body per HTTP spec
116+
// 204 No Content and 304 Not Modified MUST NOT have a message body
117+
if statusCode == http.StatusNoContent || statusCode == http.StatusNotModified {
118+
originalWriter.WriteHeader(statusCode)
119+
return
120+
}
94121

95122
// Transform captured data to JSON
96123
if len(originalData) == 0 && wrapper.status == 0 {
@@ -104,22 +131,10 @@ func New() httpserver.HandlerFunc {
104131
jsonData = []byte(`{"error":"Unable to encode response"}`)
105132
}
106133

107-
// Copy headers to original writer
108-
originalWriter := wrapper.original
109-
for key, values := range wrapper.Header() {
110-
for _, value := range values {
111-
originalWriter.Header().Add(key, value)
112-
}
113-
}
114-
115134
// Ensure JSON content type
116135
originalWriter.Header().Set("Content-Type", "application/json")
117136

118137
// Write status and transformed data
119-
statusCode := wrapper.Status()
120-
if statusCode == 0 {
121-
statusCode = http.StatusOK
122-
}
123138
originalWriter.WriteHeader(statusCode)
124139
if _, err := originalWriter.Write(jsonData); err != nil {
125140
// Response is already committed, cannot recover from write error

examples/custom_middleware/example/jsonenforcer_test.go

Lines changed: 29 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -103,11 +103,11 @@ func TestJSONEnforcerMiddleware(t *testing.T) {
103103
)
104104
})
105105

106-
t.Run("handles empty response body", func(t *testing.T) {
106+
t.Run("handles 204 No Content correctly", func(t *testing.T) {
107107
req := httptest.NewRequest("GET", "/test", nil)
108108
rec := httptest.NewRecorder()
109109

110-
// Handler that writes nothing
110+
// Handler that returns 204 No Content
111111
emptyHandler := func(w http.ResponseWriter, r *http.Request) {
112112
w.WriteHeader(http.StatusNoContent)
113113
}
@@ -123,13 +123,34 @@ func TestJSONEnforcerMiddleware(t *testing.T) {
123123
route.ServeHTTP(rec, req)
124124

125125
assert.Equal(t, http.StatusNoContent, rec.Code, "status code should be preserved")
126-
assert.Equal(t, "application/json", rec.Header().Get("Content-Type"))
127-
assert.Equal(
128-
t,
129-
`{"response":""}`,
130-
rec.Body.String(),
131-
"empty response should be wrapped",
126+
assert.Empty(t, rec.Body.String(), "204 responses must have no body per HTTP spec")
127+
assert.Equal(t, 0, rec.Body.Len(), "content length should be 0")
128+
})
129+
130+
t.Run("handles 304 Not Modified correctly", func(t *testing.T) {
131+
req := httptest.NewRequest("GET", "/test", nil)
132+
rec := httptest.NewRecorder()
133+
134+
// Handler that returns 304 Not Modified
135+
notModifiedHandler := func(w http.ResponseWriter, r *http.Request) {
136+
w.WriteHeader(http.StatusNotModified)
137+
// Even if handler writes, middleware should not include body
138+
_, err := w.Write([]byte("should be ignored"))
139+
assert.NoError(t, err)
140+
}
141+
142+
route, err := httpserver.NewRouteFromHandlerFunc(
143+
"test",
144+
"/test",
145+
notModifiedHandler,
146+
New(),
132147
)
148+
assert.NoError(t, err)
149+
150+
route.ServeHTTP(rec, req)
151+
152+
assert.Equal(t, http.StatusNotModified, rec.Code, "status code should be preserved")
153+
assert.Empty(t, rec.Body.String(), "304 responses must have no body per HTTP spec")
133154
})
134155

135156
t.Run("transforms HTML response to JSON", func(t *testing.T) {

0 commit comments

Comments
 (0)