From 7f7b2db786839697ec8c53a696f509c62a8d2e1e Mon Sep 17 00:00:00 2001 From: sachhg Date: Thu, 10 Sep 2026 18:36:26 -0700 Subject: [PATCH 1/2] fix(rfc9457): do not mutate a shared ProblemError --- rfc9457.go | 20 ++++++++++++-------- rfc9457_test.go | 30 ++++++++++++++++++++++++++++++ 2 files changed, 42 insertions(+), 8 deletions(-) diff --git a/rfc9457.go b/rfc9457.go index 90db0ac06..2719b536e 100644 --- a/rfc9457.go +++ b/rfc9457.go @@ -104,23 +104,27 @@ func ProblemDetailsHTTPErrorHandler(exposeError bool) HTTPErrorHandler { } } - if pe.Status == 0 { - pe.Status = http.StatusInternalServerError + // The problem can be a value the application reuses, for example a package + // level sentinel error, so the defaults are applied to a copy. Writing them + // back would race between requests and would permanently alter the error. + problem := *pe + if problem.Status == 0 { + problem.Status = http.StatusInternalServerError } - if pe.Type == "" { - pe.Type = "about:blank" + if problem.Type == "" { + problem.Type = "about:blank" } - if pe.Title == "" { - pe.Title = http.StatusText(pe.Status) + if problem.Title == "" { + problem.Title = http.StatusText(problem.Status) } c.Response().Header().Set(HeaderContentType, MIMEApplicationProblemJSON) var cErr error if c.Request().Method == http.MethodHead { // Issue #608 - cErr = c.NoContent(pe.Status) + cErr = c.NoContent(problem.Status) } else { - cErr = c.JSON(pe.Status, pe) + cErr = c.JSON(problem.Status, &problem) } if cErr != nil { c.Logger().Error("echo RFC 9457 error handler failed to send error to client", "error", cErr) // truly rare case. ala client already disconnected diff --git a/rfc9457_test.go b/rfc9457_test.go index 5902e11bf..1d9c0d801 100644 --- a/rfc9457_test.go +++ b/rfc9457_test.go @@ -9,6 +9,7 @@ import ( "log/slog" "net/http" "net/http/httptest" + "sync" "testing" "github.com/stretchr/testify/assert" @@ -183,3 +184,32 @@ func TestProblemError_StatusCode(t *testing.T) { pe := &ProblemError{Status: http.StatusTeapot} assert.Equal(t, http.StatusTeapot, pe.StatusCode()) } + +func TestProblemDetailsHTTPErrorHandler_DoesNotMutateSharedProblem(t *testing.T) { + // A package level sentinel is the idiomatic way to express a reusable error, + // so serving one must neither race nor leave the value altered. + sentinel := &ProblemError{Status: http.StatusNotFound, Detail: "no such widget"} + + e := New() + e.Logger = slog.New(slog.DiscardHandler) + e.Any("/path", func(c *Context) error { return sentinel }) + e.HTTPErrorHandler = ProblemDetailsHTTPErrorHandler(false) + + var wg sync.WaitGroup + for i := 0; i < 50; i++ { + wg.Add(1) + go func() { + defer wg.Done() + rec := httptest.NewRecorder() + e.ServeHTTP(rec, httptest.NewRequest(http.MethodGet, "/path", nil)) + assert.Equal(t, http.StatusNotFound, rec.Code) + assert.Equal(t, `{"type":"about:blank","title":"Not Found","status":404,"detail":"no such widget"}`+"\n", rec.Body.String()) + }() + } + wg.Wait() + + assert.Equal(t, "", sentinel.Type) + assert.Equal(t, "", sentinel.Title) + assert.Equal(t, http.StatusNotFound, sentinel.Status) + assert.Equal(t, "no such widget", sentinel.Detail) +} From 5efa193be26fa11ced07f8eedf458145e22139b2 Mon Sep 17 00:00:00 2001 From: sachhg Date: Thu, 10 Sep 2026 21:55:05 -0700 Subject: [PATCH 2/2] test(rfc9457): check the sentinel after a single request --- rfc9457_test.go | 25 +++++++------------------ 1 file changed, 7 insertions(+), 18 deletions(-) diff --git a/rfc9457_test.go b/rfc9457_test.go index 1d9c0d801..aa871e82c 100644 --- a/rfc9457_test.go +++ b/rfc9457_test.go @@ -9,7 +9,6 @@ import ( "log/slog" "net/http" "net/http/httptest" - "sync" "testing" "github.com/stretchr/testify/assert" @@ -187,29 +186,19 @@ func TestProblemError_StatusCode(t *testing.T) { func TestProblemDetailsHTTPErrorHandler_DoesNotMutateSharedProblem(t *testing.T) { // A package level sentinel is the idiomatic way to express a reusable error, - // so serving one must neither race nor leave the value altered. + // so serving one must not leave the value altered. sentinel := &ProblemError{Status: http.StatusNotFound, Detail: "no such widget"} + original := *sentinel e := New() e.Logger = slog.New(slog.DiscardHandler) e.Any("/path", func(c *Context) error { return sentinel }) e.HTTPErrorHandler = ProblemDetailsHTTPErrorHandler(false) - var wg sync.WaitGroup - for i := 0; i < 50; i++ { - wg.Add(1) - go func() { - defer wg.Done() - rec := httptest.NewRecorder() - e.ServeHTTP(rec, httptest.NewRequest(http.MethodGet, "/path", nil)) - assert.Equal(t, http.StatusNotFound, rec.Code) - assert.Equal(t, `{"type":"about:blank","title":"Not Found","status":404,"detail":"no such widget"}`+"\n", rec.Body.String()) - }() - } - wg.Wait() + rec := httptest.NewRecorder() + e.ServeHTTP(rec, httptest.NewRequest(http.MethodGet, "/path", nil)) - assert.Equal(t, "", sentinel.Type) - assert.Equal(t, "", sentinel.Title) - assert.Equal(t, http.StatusNotFound, sentinel.Status) - assert.Equal(t, "no such widget", sentinel.Detail) + assert.Equal(t, http.StatusNotFound, rec.Code) + assert.Equal(t, `{"type":"about:blank","title":"Not Found","status":404,"detail":"no such widget"}`+"\n", rec.Body.String()) + assert.Equal(t, original, *sentinel) }