fix(rfc9457): do not mutate a shared ProblemError

[sachhg]

Problem

ProblemDetailsHTTPErrorHandler fills in the zero-valued Type, Title and Status fields by writing them back into the *ProblemError it was handed:

if pe.Status == 0 {
    pe.Status = http.StatusInternalServerError
}
if pe.Type == "" {
    pe.Type = "about:blank"
}
if pe.Title == "" {
    pe.Title = http.StatusText(pe.Status)
}

pe comes from errors.As, so it points at whatever value the application returned. Returning a package level sentinel is the idiomatic way to express a reusable error, and the same pointer then reaches the handler on every request that returns it:

var ErrWidgetNotFound = &echo.ProblemError{Status: http.StatusNotFound, Detail: "no such widget"}

func handler(c *echo.Context) error {
    return ErrWidgetNotFound
}

Two things go wrong. Concurrent requests write to the same struct, which the race detector reports:

WARNING: DATA RACE
Write at 0x... by goroutine 10:
Previous read at 0x... by goroutine 33:

And the write is permanent. After the first request the sentinel is no longer the value the application declared; Type has become "about:blank" and Title has become "Not Found". An application that later inspects or re-serializes its own error sees fields it never set.

The ProblemErrorer path has the same shape whenever ProblemError() returns a shared pointer.

The change

The defaults are applied to a copy, so the response is unchanged and the caller's value is left alone. Nothing else about the handler moves.

Testing

TestProblemDetailsHTTPErrorHandler_DoesNotMutateSharedProblem serves one request that returns a sentinel, checks the response still carries the fully defaulted body, and then compares the sentinel with a copy taken before the request.

Against the current code it fails on the mutation:

expected: echo.ProblemError{Type:"", Title:"", Status:404, Detail:"no such widget", Instance:""}
actual  : echo.ProblemError{Type:"about:blank", Title:"Not Found", Status:404, Detail:"no such widget", Instance:""}

With this change go test -race ./... is clean across all three packages.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 95.73%. Comparing base (dcb05f0) to head (5efa193).
⚠️ Report is 13 commits behind head on master.

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #3094      +/-   ##
==========================================
+ Coverage   93.34%   95.73%   +2.38%     
==========================================
  Files          43       44       +1     
  Lines        4735     4034     -701     
==========================================
- Hits         4420     3862     -558     
+ Misses        192      172      -20     
+ Partials      123        0     -123     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[aldas]

LFTM