🐛 bug: fix correctness issues across core, router, binder, client and middleware

[gaby]

Description

This PR is the result of a correctness audit of the codebase. It fixes 70+ distinct bugs across the app lifecycle, the router, Ctx/request/response handling, the binder, the REST client and twelve middlewares, together with the documentation that described the old (or intended-but-unimplemented) behavior.

Each fix has a regression test that reproduces the bug against the unfixed code. The branch adds 162 tests; the full suite (make test, race detector, shuffled) passes.

Correction, after @ReneWerner87's review. Three numbers in an earlier version of this description were wrong, and are corrected below: the Benchmark_equalFieldType and Benchmark_App_RebuildTree figures were branch-internal before/after pairs presented as comparisons against the merge base, and one claimed cache fix described a bug that did not exist on base. Their measurements were right in every case. Details in this comment.

No single issue tracks these; each fix is described below with the regression test that pins it.

Bugs fixed

App lifecycle, TLS and services

• ShutdownWithContext deadlocked against its own handlers. The app mutex was held for the whole connection drain, so an in-flight handler that takes the mutex itself (RebuildTree, RemoveRoute, Name) could never finish and every ShutdownWithTimeout ran to its deadline. The mutex is now released before draining. (Test_App_ShutdownWithTimeout_HandlerTakesAppMutex)
• …and two concurrent Shutdown callers then terminated every service twice, racing each other while doing it. The started list was handed out as a copy, and a service is only removed once it has terminated — too late to stop the second call. Shutdown now drains the list under its own mutex and puts back only what would not terminate, so a later shutdown still retries those. (Test_ShutdownServices_ConcurrentCallersTerminateOnce)
• Trusted proxies were silently untrusted. Proxies were stored exactly as written while lookups compare the canonical net.IP form, so 2001:DB8::1, 2001:0db8::1 or 0:0:0:0:0:0:0:1 never matched. Addresses are now stored canonically, with the IPv4-mapped spelling registered alongside. (Test_App_IsProxyTrusted_CanonicalIPForms, Test_App_IP_StripTrustedProxies_CanonicalIPForms)
• Ctx.ClientHelloInfo reported the wrong handshake. TLSHandler kept one app-wide ClientHelloInfo, so under concurrency a handler saw whichever TLS handshake happened most recently on any connection. The handler now records one per connection and releases it when the server reports the connection closed, via a ConnState hook that preserves a user-installed callback. (Test_Listener_TLS_ClientHelloInfo_PerConnection, Test_Listener_ConnState_UserCallbackPreserved)
• …and leaked one per handshake behind MaxConnsPerIP, because the pooled wrapper fasthttp installs for per-IP accounting is closed, cleared and recycled before the close is reported. A connection is now reported to the handler while it is still open, and the note taken then resolves its record at close. (Test_Listener_TLS_ClientHelloInfo_MaxConnsPerIP_NoLeak, Test_TLSHandler_ForgetsRecycledWrapper)
• …and leaked one per hijacked connection. StateHijacked is terminal in fasthttp — StateClosed never follows it — so every WebSocket upgrade over TLS retained a record. (Test_TLSHandler_ForgetsHijackedConnection) See Notes for reviewers for the remaining gap in this component and the redesign it needs.
• Shutdown hooks fired twice, or fired for a server that never started. With GracefulContext configured, an explicit Shutdown made Listen return, which woke the graceful-shutdown goroutine into a second shutdown; a Listen that failed before serving fired the hooks for nothing. (Test_Listen_GracefulShutdown_HooksOnce, Test_Listen_GracefulShutdown_NoHooksOnListenError)
• ShutdownTimeout was not applied as documented. The default is now applied on the built-in shutdown path, and a negative value disables the timeout and waits indefinitely. (Test_ListenConfigDefault_ShutdownTimeout, Test_GracefulShutdown_NegativeTimeoutWaitsIndefinitely)
• A half-configured TLS listener quietly served plaintext. Setting only one of CertFile/CertKeyFile fell through to the plain-HTTP branch. It now fails with ErrCertFileAndKeyRequired. (Test_Listen_TLS_PartialCertConfig)
• Services with duplicate names were dropped and never terminated, because services are keyed by name in State, and termination order was random since the started services were iterated from a map rather than in reverse start order. (Test_StartServices_DuplicateNames, Test_ShutdownServices_ReverseStartOrder)
• The error handler raced with timed-out handlers. Once fasthttp holds a timeout response, writes to the context are ignored anyway, while the abandoned handler goroutine may still be writing to it. App.ErrorHandler now returns without running the configured handler for such a request; the error still reaches outer middleware. (Test_App_ErrorHandler_SkipsCommittedTimeoutResponse, Test_Timeout_ErrorHandlerSkippedAfterTimeout)

Router

• Route constraints were corrupted by case-insensitive routing. With CaseSensitive off the whole pattern was lowercased before parsing, constraint names and arguments included: regex([A-Z]{2}) matched lowercase only, \D became \d, a datetime layout lost its T and Z, and a mixed-case custom constraint was dropped altogether. Constraints are now adopted from the pattern as written. (Test_Router_Constraints_CaseInsensitiveRouting)
• …which then stopped enforcing mixed-case built-in names. Adopting the pattern as written meant <INT> and <Guid> resolved to no handler at all and matched anything. Built-in names now fold case; custom ones keep the exact match, so two customs differing only in case stay distinct and a custom cannot capture a built-in name in another case. (Test_Router_Constraints_MixedCaseNames)
• Method and path overrides skipped routes. After middleware overrode the method (or the path into another tree bucket), Next resumed at the old bucket's index inside the new tree, skipping the endpoints before it. Routes now carry a registration id so their position can be found in the other tree. (Test_Router_MethodOverride_ContinuesInNewMethodTree, Test_Router_PathOverride_ContinuesInNewBucket)
• An escaped star registered as the catch-all route. /\* is a literal * in the path; the star flag is now derived from the pattern with its escapes intact. (Test_Router_EscapedStar_IsLiteral)
• StrictRouting was not applied to parametric routes, and not applied at all to the routes of a mounted app. /a/:id/ matched /a/x, while the equivalent static route correctly did not. (Test_Router_StrictRouting_ParamTrailingSlash, Test_App_Mount_StrictRouting)
• UnescapePath turned a literal + into a space, because the path was decoded with fasthttp's form-argument decoder. In a path + is an ordinary character (RFC 3986 §3.3); malformed escapes are now also left as-is instead of mangled. (Test_Router_UnescapePath_PlusIsLiteral, Test_UnescapePath_MalformedEscape)
• Name applied to only one route of a multi-method Add. (Test_App_Add_MultipleMethods_Name)
• Use with a prefix slice and a sub-app mounted only the first prefix — in App.Use, and in Group.Use and domainRouter.Use, which the first round missed. (Test_App_Use_MultiplePrefixes_MountsEachPrefix, Test_Group_Use_MultiplePrefixes_MountsEachPrefix, Test_Domain_Use_MultiplePrefixes_MountsEachPrefix)
• Mounting an app onto itself deadlocked at startup; it now panics with a clear message. (Test_App_Mount_Self_Panics)
• Automatic HEAD routes were missing in three cases: routes added at runtime and published with RebuildTree; a sub-app mounted onto an already-mounted app (discovered only after the auto-HEAD pass had run); and after a panicking OnRoute hook, because the "scan complete" memo was written from a defer and recorded the aborted scan, leaving HEAD at 405 for the lifetime of the process. (Test_App_RebuildTree_RegistersAutoHead, Test_Mount_Nested_AutoHead_SingleStartup, Test_App_AutoHead_PanickingHookLeavesScanIncomplete)

Ctx, request and response

• SendFile panicked on an uncomparable fs.FS. The handler cache compared fs.FS interface values with ==, which panics for a map-backed type such as fstest.MapFS on the second call. Such values are now compared by the identity of what they reference — including the length of a slice-backed one, since two prefixes of one array share an element pointer — and a func-backed file system is treated as distinct rather than risk serving the wrong root. (Test_Ctx_SendFile_UncomparableFS, Test_SameFS, Test_SameFS_SliceLength)
• SendFile deleted the request's Accept-Encoding, so middleware running after it (compress, logger) no longer saw what the client accepts. (Test_Ctx_SendFile_KeepsAcceptEncoding, Test_Ctx_SendFile_CompressedByMiddleware)
• AutoFormat answered text/html with no Accept header, where it is documented to fall back to text/plain. (Test_Ctx_AutoFormat_NoAcceptHeader)
• Content negotiation ignored range specificity. Per RFC 9110 §12.5.1 the most specific matching range determines an offer's weight, so a broad range with a higher weight (* or text/*) must not outrank a specific range that gives the offer a lower one — or rejects it with q=0. The resolution is now specificity-aware, with a fast path for uniform-weight headers and a first-match shortcut. (Test_Ctx_Accepts_MostSpecificRangeSetsQuality, Test_Utils_GetOffer_QualityZeroRejection)
• IPv4-mapped IPv6 proxies were reported as the client. Dual-stack proxies forward IPv4 clients as ::ffff:a.b.c.d (RFC 4291 §2.2); the validator rejected that spelling because it carries both . and :, so X-Forwarded-For was discarded and the proxy itself was returned as c.IP(). (Test_Ctx_IP_IPv4MappedIPv6InProxyHeader)
• Custom contexts were not passed to Format handlers or custom binders, so a c.(*MyCtx) assertion that holds in a route handler failed there. (Test_Ctx_Format_CustomCtx, Test_Bind_CustomBinder_CustomCtx)
• Multi-coding request bodies could read pooled memory. The identity step of a coding chain re-set the request body with a slice aliasing it, which under ReduceMemoryUsage returned the buffer to fasthttp's pool while the next decoder was still reading from it. (Test_Ctx_Body_With_Compression_IdentityChain)
• Flash messages were lost on programmatic requests. The flash-cookie prefilter only inspected the raw request headers, which a request built by middleware/adaptor or app.Handler on a hand-made RequestCtx does not carry. (Test_Redirect_Messages_ProgrammaticCookieHeader)
• The binder split scalar values. Whether a comma-separated value is split was decided by a coarse "does any field one level down have this kind" check, so a scalar field that merely shared a struct with a slice was split. Key resolution now walks the actual struct (own fields, embedded aliases, promoted fields with ambiguity detection, slice indices, dot/bracket notation), and only a destination the decoder could fill is considered. (11 Test_EqualFieldType_* / Test_QueryBinder_Bind_Splitting_* / Test_HeaderBinder_* tests, Test_fieldName, Test_StructKeyKind_NestedPaths)

Client

• A cookie fasthttp could not fully parse failed the whole request. A negative Max-Age or an Expires in another date format aborted cookie parsing; RFC 6265 §5.2 says an unparsable attribute is ignored, so the cookie's name and value are now kept. (Test_Client_UnparsableCookieAttributesIgnored, Test_Client_ResponseCookie_UnparsableAttribute)
• A failing response hook released the caller's Request, so the documented AcquireRequest/ReleaseRequest pattern released it twice — while a request the client itself created for the call was stranded when a hook failed. Requests a client helper creates are now tracked separately from caller-owned ones. (Test_Client_ResponseHookError_KeepsCallerRequest, Test_Request_Reset_ClearsClient, Test_Client_PreHookError_ReleasesOwnedRequest, Test_Client_AfterHookError_ReleasesOwnedRequest)
• The cookie jar ignored Max-Age. A Max-Age=0 logout kept sending the cookie and a short Max-Age never expired it. Max-Age now takes precedence over Expires (RFC 6265 §5.2.2), while a value that is not an integer is ignored rather than treated as an immediate expiry. (Test_CookieJar_MaxAge, Test_CookieJar_MaxAgePrecedence)
• …and resolved a repeated Max-Age by the wrong one, overflowed on a large one, and truncated one on 32-bit. fasthttp keeps the last occurrence while the attribute scan stopped at the first; Max-Age=10000000000 fits an int but not the nanoseconds of a time.Duration, so a cookie asking for three centuries of life was given an expiry in the past; and parsing into int truncates on 386 and arm, where a value past two billion silently downgraded a persistent cookie to a session cookie. (Test_CookieJar_MaxAgePrecedence, Test_CookieJar_MaxAgeBeyond32Bit)
• A canceled context raced with the request. The transport goroutine read fields from the caller's request after Send had already returned ErrTimeoutOrCancel, by which time the caller may have released it. (Test_Request_CancelledContext_NoRace, race detector)
• Request headers did not override client headers, they were appended to them; and re-sending a request accumulated every previous send's values. (Test_Client_RequestHeaderOverridesClientHeader, Test_Request_Resend_DoesNotAccumulateHeaders)
• Debug logging drained streamed bodies, consuming them before the caller could read them. (Test_Client_Debug_DoesNotDrainStream, Test_Client_Logger_StreamedBodiesLogHeadersOnly)
• Response.Header returned a string aliasing pooled memory, invalid after the response was released. (Test_Response_Header_CopiesValue)
• Query parameters lost their insertion order (sort.Sort is not stable). (Test_Request_Params_KeepInsertionOrder)
• An uppercase scheme was rejected as malformed. URL schemes are case-insensitive (RFC 3986 §3.1). (Test_Client_UppercaseScheme)

Middleware

cache

• A backend that dropped the body entry while keeping the metadata made every request for that key fail until the metadata expired, instead of being treated as a miss. (Test_Cache_StorageMissingBodyIsMiss, Test_Cache_OrphanedMetadataWithoutBody, Test_Cache_BodyFetchFailure)
• Refreshing an entry through a no-cache request left the old heap node behind: it popped in a later eviction and deleted the live entry, while its bytes stayed counted against MaxBytes forever. (Test_Cache_NoCacheRefresh_KeepsAccounting)
• An entry untracked before its replacement was stored was lost when the replacement failed to store, leaving it in the backend counting toward nothing and never expiring; it is now restored. (Test_Cache_VaryManifestStoreFailureUnreservesSpace)
• Eviction ran before the decision to store, so a response the middleware then declined to cache had already emptied the cache of valid entries. (Test_Cache_UncacheableResponseDoesNotEvict)
• A 206 Partial Content was cached and replayed — status, range body and all — to the next request for the whole representation. (Test_Cache_PartialContentNotStored)
• A clock stepping back past an entry's receipt time underflowed its resident time into a maximal Age. (Test_CacheAgeClockStepsBackwards)
• CacheInvalidator marked the shared in-memory item expired under the lock while concurrent hits read its expiry after dropping it — a data race the detector reports. (Test_CacheInvalidator_SharedEntryNoDataRace)

limiter / storage — a positive Expiration below one second fell back to the one-minute default; the in-memory storages truncated TTLs to whole seconds, so a sub-second TTL was stored already expired and 1.9s expired after one. TTLs now round up. (Test_Limiter_SubSecondExpirationIsOneSecondWindow, Test_Memory_TTLRoundsUp, Test_Memory_SubSecondTTLExpires, Test_Storage_Memory_Set_ExpirationRoundsUp, Test_Storage_Memory_GCInterval_SubSecond)

session — Reset wiped the absolute expiration along with the data, so a rotated session never expired absolutely. (Test_Session_Reset_KeepsAbsoluteTimeout)

etag — 304 Not Modified was returned for methods other than GET and HEAD, where RFC 9110 §13.1.2 does not define it and the handler has already run. (Test_ETag_IfNoneMatchOnlyForGetHead)

keyauth — the WWW-Authenticate challenge was skipped when the ErrorHandler returned a *fiber.Error for the app to write instead of setting the status itself. (Test_ErrorHandlerReturnsErrorChallenge)

timeout — the default path returned nil, so outer middleware recorded a timed-out request as a success; and a *fiber.Error returned by OnTimeout was dropped in favor of the default 408. (Test_Timeout_DefaultReturnsErrRequestTimeout, Test_Timeout_OnTimeoutReturnedErrorShapesResponse)

proxy — after Forward the app's own request still carried the upstream Host; and DomainForward ended the chain with an empty 200 for a non-matching host instead of passing it on, while a Host carrying a port never matched at all. (Test_Proxy_Forward_RestoresHost, Test_Proxy_DomainForward_NonMatchingHostContinues, Test_Proxy_HostWithoutPort)

static — one handler registered on two routes captured the first request's prefix and stripped it from every route; the Download attachment header was set before the file lookup and leaked onto the fallthrough response of a missing file; Download derived Content-Type from the URL extension, blanking the type detected for an extension-less file; and MaxAge was applied to error responses such as 416. (Test_Static_SameHandlerUnderTwoPrefixes, Test_Static_Download_NotFoundLeavesNoAttachment, Test_Static_Download_KeepsDetectedContentType, Test_Static_MaxAge_NotOnErrorResponses)

compress — negotiation used fasthttp's exact token matching, ignoring weights, wildcards and lists without spaces (so gzip;q=0 still compressed with gzip, and * matched nothing); an element carrying parameters other than the weight, or an unparsable q, was read as a rejection instead of taking the default weight of 1 per RFC 9110 §12.4.2; recomputing a strong ETag drained a streamed body into memory; and the client's own Accept-Encoding header was left mutated. (Test_Compress_AcceptEncoding_Negotiation, Test_Compress_StreamedBody_KeepsETagWeak, Test_Compress_RequestHeaderUntouched)

sse — a reconnection delay below one millisecond was written as retry: 0, telling the client to reconnect immediately. (Test_SSE_Retry_SubMillisecondRoundsUp)

adaptor — the context-value walk only recursed into a field literally named Context, so every value below a WithTimeout, WithDeadline, WithoutCancel or AfterFunc context was lost; a depth bound meant to stop a cyclic chain silently dropped the outermost values of a deeper middleware stack, so the walk now follows the chain to its end and stops only where it would repeat, keyed on each context's pointer with its type; a request net/http served over TLS was seen as plaintext by Fiber; only RequestURI was copied back after middleware ran, so a rewrite of r.URL such as http.StripPrefix was ignored; and the guard installing that rewritten path compared the decoded path against the raw one the router matched, so any escaped or non-canonical request re-bucketed the tree and resumed the chain past middleware that had already run — an auth guard mounted before the adaptor never ran for /api%2Fadmin/secret and four other spellings. (Test_CopyContextToFiberContext_DerivedContexts, Test_CopyContextToFiberContext_DeepChain, Test_CopyContextToFiberContext_Cycle, Test_HTTPHandler_TLSPropagated, Test_HTTPHandler_TLSConnectionStateCarried, Test_HTTPMiddleware_URLRewrite, Test_HTTPMiddleware_NoRewriteKeepsChainPosition, Test_HTTPMiddleware_NextCalledAfterReturn)

retry (addon) — ExponentialBackoff mutated its own currentInterval, so a second Retry call on the same value started at MaxBackoffTime and concurrent callers corrupted each other's backoff. (Test_ExponentialBackoff_Retry_StartsFromInitialInterval, Test_ExponentialBackoff_Retry_Concurrent)

log — Fatal returned before exiting when the logger level suppressed its output, so a fatal condition let the program continue. (Test_DefaultLogger_FatalExitsAboveLevel, Test_DefaultLogger_FatalExitsAfterWrite)

paginate — with no SortKey configured the middleware still read c.Query(""), letting a nameless ?=-name parameter control the sort order; and NextPageURL could overflow. (Test_Paginate_EmptySortKey_IgnoresQuery, Test_PageInfo_NextPageURL_NoOverflow)

favicon — the read limit was computed as MaxBytes+1, which overflowed for math.MaxInt64 and served the icon as an empty response. (Test_Favicon_MaxBytes_MaxInt64)

Changes introduced

• Benchmarks: two benchmarks moved, both measured against the merge base (b192a98) on one machine.

  Benchmark_Utils_GetOffer — the negotiation fix is on a hot path. Medians of -count=5, allocations unchanged in every case:

  case	base	this PR
  simple	71.5 ns	75.0 ns	+5.0%
  6_offers	192.5 ns	202.7 ns	+5.3%
  1_parameter	305.9 ns	315.4 ns	+3.1%
  2_parameters	452.7 ns	442.0 ns	-2.4%
  3_parameters	592.1 ns	586.4 ns	-1.0%
  10_parameters	1813.0 ns	1803.0 ns	-0.6%
  6_offers_w/params	647.9 ns	646.0 ns	-0.3%
  mime_extension (×4)	392–844 ns	403–864 ns	-0.7% … +9.5%
  web_browser	279.8 ns	398.3 ns	+42.4%

  Uniform-weight headers take a fast path and are unchanged within noise. The web_browser case is the one that genuinely costs more: mixed weights require checking whether a more specific range demotes the first match, which is exactly the bug being fixed. @ReneWerner87 has sketched a way to recover roughly half of that; it is on the follow-up list below.

  Benchmark_equalFieldType — ~1.06x against the merge base after 51c0085 (interleaved medians on one machine: 150.5 → 160 ns/op), allocations unchanged at 0. The correct walk down the struct that replaced main's single map lookup first cost 1.60x; profiling put most of that not in the walk but around it — the sync.Map holding each type's field information, whose interface-keyed hash ran up to four times per call, plus reflect.Value construction and repeated Kind() calls. A small direct-mapped cache keyed by the type descriptor's address now sits in front of the map (a collision only falls through to it), the per-tag cache is resolved once per call rather than per level, and the type is walked instead of a reflect.Value. The residual is within the benchmark's own run-to-run spread, and the CI bot's re-measurement on 51c0085 agrees: no significant change across all 1692 compared benchmarks, so the gate is green and Benchmark_equalFieldType is no longer flagged.

  Benchmark_App_RebuildTree — 22888 → 33050 B/op (+44.4%), 75 → 84 allocs, deterministic. The cause is legitimate: the auto-HEAD fix takes the route count from 239 to 381, so this is the price of registering the companions that were previously missing.

  One earlier regression the CI bot flagged was a real bug, since fixed and back to parity: Benchmark_ShutdownServices_.../successful-completion (121 ms → 61 ms — App.init already starts configured services, so a second startServices registered each twice and shutdown terminated them twice).

• Patch coverage: the fixes add guard and fallback branches, so the branch's own new lines were measured and the reachable ones covered: 57 → 14 uncovered statements. Measuring them turned up the compress default-weight bug above. The 14 that remain are unreachable without corrupt state or two simultaneous storage faults.

• Documentation Update: 21 documentation pages were corrected to match the fixed behavior. This also documents three previously undocumented behaviors found during the audit: ShutdownTimeout semantics, that GracefulContext and Shutdown* act only on the process they run in under prefork, and that Response.Close releases the request only when the client created it.

• Changelog/What's New: Bug fixes only; no API additions.

• Migration Guide: Not required — no API changes. Behavior changes are corrections toward the documented and RFC-specified behavior. One deserves a release note: proxy.DomainForward now passes a non-matching host on to the routes behind it instead of ending the chain, which removes a host-isolation property some deployments may lean on.

• API Alignment with Express: No new API surface.

• API Longevity: No exported signatures changed. The only new exported symbol is the ErrCertFileAndKeyRequired error value.

• Examples: No new features to demonstrate.

Type of change

• Documentation update (changes to documentation)
• Code consistency (non-breaking change which improves code reliability and robustness)

The template offers no "bug fix" option; this PR is bug fixes plus the documentation corrections that go with them.

Checklist

• Followed the inspiration of the Express.js framework for new functionalities. (No new functionality.)
• Conducted a self-review of the code and provided comments for complex or critical parts.
• Updated the documentation in the /docs/ directory.
• Added or updated unit tests to validate the effectiveness of the changes. (162 new tests. Each fix has a test that reproduces its bug; running the new tests against unfixed production code, @ReneWerner87 measured 104 of 141 failing at the time, with 25 not compiling against base because they reference symbols this branch introduces.)
• Ensured that new and existing unit tests pass locally with the changes. (make test: 5769 tests, race detector on, -shuffle=on.)
• Verified that any new dependencies are essential. (No new dependencies.)
• Aimed for optimal performance with minimal allocations in the new code.
• Provided benchmarks for the new code to analyze and improve upon. (See Benchmarks above.)

Notes for reviewers

Deliberately not changed here, called out rather than silently left:

• Prefork graceful shutdown stops only the process whose context is canceled. fasthttp's prefork master exposes no shutdown API, so this is documented instead of worked around.
• A single-byte delimiter parameter swallows /. Pinned by existing tests; changing it would be a breaking routing change and belongs in its own PR.
• SkipUnmatchedRoutes answers 405 before the middleware chain, so a method-override middleware never runs for those requests. This is the documented short-circuit; a test now pins it.
• etag does not answer an unsafe method with 412. RFC 9110 §13.1.2 calls for 412 where a matching If-None-Match arrives on a method other than GET or HEAD, but that rule assumes the precondition is evaluated before the method is applied — §15.5.13 defines 412 by that purpose. This middleware runs after c.Next(), over a 200 OK the handler already produced, so answering 412 would tell the client its request was not performed when it was. docs/middleware/etag.md now says this.
• The TLS ClientHelloInfo map still has one hole: records are retained when fasthttp refuses a connection at the per-IP limit before StateNew fires. The design assumes StateNew fires for every connection that completes a handshake, which is not true on every path. What removes the class rather than patching it again is storing the info on a Fiber-owned listener wrapper so cleanup rides on Close(). That is a redesign and is proposed as a follow-up.

Follow-ups from @ReneWerner87's review, agreed and not in this diff: the sameFS(x, x) SendFile store/goroutine leak for non-comparable structs, a per-mount Route.id remap, the compress/client allocation regressions and the Benchmark_Compress fixture reusing one RequestCtx, and the negotiation fast-path recovery. Two pre-existing issues they found on both trees will be filed separately: the req.go:192 SetBodyRaw aliasing race, and root-package test-order instability under -shuffle=on.

make betteralign reports two findings (routeParser in path.go, Route in router.go) that reproduce identically on the merge base. Both structs carry existing "field order is load-bearing" comments and //nolint:govet // fieldalignment directives, so their layout was left alone.

---

🤖 Generated with Claude Code

https://claude.ai/code/session_01NcsBE6AGr9Ey8cpaiELUs8

[chatgpt-codex-connector]

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review	Status	Commit	Review trigger
📝 Code Review	✅ Completed 2026-09-04T13:45:14.106165Z	8a56e9c	Manual request
🔒 Security Review	✅ Completed 2026-09-04T13:41:24.441641Z	8a56e9c	Manual request

Security findings

Advisory findings (1)

• 🟡 Remove wrapped TLS connections from the ClientHello map · Medium

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

[coderabbitai]

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

• @coderabbitai resume to resume automatic reviews.
• @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

• ▶️ Resume reviews
• 🔍 Trigger review

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Team

Run ID: 542aeb40-2c45-43c9-9992-b9c80af716ec

📥 Commits

Reviewing files that changed from the base of the PR and between 50f1f88 and 7cd34a9.

📒 Files selected for processing (2)

• binder/mapping_test.go
• client/client_test.go

🚧 Files skipped from review as they are similar to previous changes (1)

• client/client_test.go

Included review availability: Your plan provides up to 10 included reviews per hour; 8 remain after this review.

---

Walkthrough

This change set updates retry state, routing, request handling, middleware behavior, cache accounting, storage expiration, service shutdown, TLS handling, logging, and related documentation and tests.

Changes

Retry state

Layer / File(s)	Summary
Isolated retry backoff state addon/retry/*	Retry keeps interval state per invocation and supports concurrent reuse of one ExponentialBackoff instance.

Routing and request context

Layer / File(s)	Summary
Routing and context flow app.go, mount.go, path.go, req.go, res.go, router.go, ctx.go, redirect.go	Routing preserves constraint spelling, handles escaped stars and strict trailing slashes, supports route-tree changes after method or path overrides, mounts prefixes, registers automatic HEAD routes, canonicalizes proxy addresses, preserves programmatic flash cookies, and passes custom contexts to handlers.
Binder field resolution binder/*, bind_test.go	Binder resolution handles nested keys, embedded and promoted fields, indexed collections, recursive types, and slice-only splitting destinations.

Client and adaptor flow

Layer / File(s)	Summary
Client request ownership and cleanup client/*	Client helpers mark owned requests, response cleanup respects ownership, headers reset between sends, cookies honor Max-Age, streamed debugging preserves bodies, and response header values are copied.
HTTP adaptor flow middleware/adaptor/*	The adaptor propagates TLS state, copies derived context values, follows rewritten URLs, and blocks late continuation after flush or hijack.

Middleware behavior

Layer / File(s)	Summary
Response and protocol middleware middleware/compress/*, middleware/etag/*, middleware/proxy/*, middleware/static/*, middleware/timeout/*, middleware/keyauth/*	Compression negotiation, ETag conditions, proxy host restoration, static serving, timeout errors, and authentication challenge handling follow the updated behavior.
Boundary and duration handling middleware/limiter/*, middleware/paginate/*, middleware/sse/*, middleware/session/*, middleware/favicon/*, helpers.go	Duration conversion, pagination limits, session expiration, favicon limits, and content negotiation handle boundary values and specificity rules.

Cache, storage, and lifecycle

Layer / File(s)	Summary
Cache storage and accounting middleware/cache/*	Cache eviction, invalidation, accounting, missing bodies, partial responses, and clock rollback handling are updated.
Storage and service lifecycle internal/memory/*, internal/storage/memory/*, services.go, state.go, services_test.go	Memory expirations round up and saturate safely. Services validate duplicate names and terminate in reverse start order.

Listener and logging lifecycle

Layer / File(s)	Summary
Listener lifecycle and TLS validation listen.go, listen_test.go, app.go, app_test.go	TLS certificate pairs are validated, shutdown timeout semantics are updated, shutdown hooks are controlled by listener state, and connection cleanup integrates with TLS metadata.
Fatal logging termination log/default.go, log/default_test.go, hooks_test.go	Fatal logging uses a replaceable exit hook and exits even when the fatal message is filtered.

Documentation

Layer / File(s)	Summary
API and middleware documentation docs/api/*, docs/client/*, docs/guide/*, docs/middleware/*	Documentation describes the updated routing, context, client ownership, middleware, shutdown, storage, service, and protocol behavior.

Estimated code review effort: 5 (Critical) | ~120 minutes

Merge Risk: 🟡 Moderate · up to 7cd34

The change updates broad Fiber lifecycle and request behavior, but unresolved conditional-write, routing, mount-cycle, and client-request ownership concerns could cause incorrect request handling or availability problems in affected applications. These issues should be resolved or explicitly accepted before merge.

Poem

A rabbit checks the routes,
Backoff hops from thread to thread,
TLS stars guard the gate,
Cache clocks settle in the shade,
Tests nibble every edge.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 28.83% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 222 functions across 61 files.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Title check	✅ Passed	The title clearly summarizes the pull request as a broad correctness-fix pass across the core areas listed in the changeset. It is concise and related to the primary change, although it uses a broad s…
Description check	✅ Passed	The description is detailed and directly addresses the template requirements. It explains the purpose, lists the changes, documents tests and benchmarks, identifies documentation updates, covers API a…

• Fix all pre-merge checks with AI

✨ Finishing Touches 💡 1

📝 Generate docstrings 💡

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch claude/codebase-bug-audit-g0uzel

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[codecov]

Codecov Report

❌ Patch coverage is 97.16599% with 28 lines in your changes missing coverage. Please review.
✅ Project coverage is 94.18%. Comparing base (ac861c5) to head (51c0085).

Files with missing lines	Patch %	Lines
middleware/cache/cache.go	86.79%	10 Missing and 4 partials ⚠️
client/hooks.go	83.87%	3 Missing and 2 partials ⚠️
ctx.go	89.36%	3 Missing and 2 partials ⚠️
middleware/static/static.go	97.05%	2 Missing and 1 partial ⚠️
log/default.go	93.33%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #4652      +/-   ##
==========================================
+ Coverage   93.80%   94.18%   +0.38%     
==========================================
  Files         139      139              
  Lines       16229    16881     +652     
==========================================
+ Hits        15223    15899     +676     
+ Misses        636      627       -9     
+ Partials      370      355      -15     

Flag	Coverage Δ
unittests	94.18% <97.16%> (+0.38%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ 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.

[Copilot]

🟡 Changes recommended

State.deleteService currently compares Service interface values directly (started == srv), which can panic for uncomparable service implementations and should be replaced with a safe identifier comparison.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Pull request overview

This PR applies a broad correctness audit across Fiber core (app lifecycle, routing, ctx/request/response), the REST client, binders, and multiple middlewares, pairing behavior fixes with regression tests and documentation updates so the corrected semantics are pinned going forward.

Changes:

• Fixes service lifecycle correctness (duplicate-name validation, deterministic reverse shutdown order) and multiple router correctness issues (constraints preservation under case-insensitive routing, StrictRouting edge cases, method/path override continuity, auto-HEAD updates).
• Hardens request/response behaviors (safe SendFile FS comparison, preserves Accept-Encoding, correct AutoFormat fallback, path unescape semantics, proxy/IP validation, timeout/error-handler interactions).
• Updates client and middleware correctness (cookie parsing tolerance, request ownership on Response.Close, header merge semantics, compression negotiation, proxy forwarding host restoration) with extensive new tests and docs.

File summaries

File	Description
state.go	Track started services in-order with a mutex for deterministic shutdown.
services.go	Validate services slice upfront; shutdown in reverse start order.
services_test.go	Adds tests for reverse shutdown order and duplicate service names.
router.go	Route IDs for cross-tree Next() continuity; constraints adoption; auto-HEAD behavior updates.
res.go	Safer FS equality for SendFile; custom ctx propagation for Format; AutoFormat fallback; preserve Accept-Encoding.
req.go	Fix identity coding body aliasing; IPv4-mapped IPv6 validation; method/path override index carry-over; trusted proxy unmap.
redirect.go	Flash-cookie detection works for programmatic requests with empty raw headers.
redirect_test.go	Regression test for programmatic flash cookie parsing/clearing.
path.go	Correct path unescaping (+ literal, malformed escapes preserved); strict routing for param routes; constraint adoption.
mount.go	Prevent self-mount deadlock; collect nested sub-apps before startup processing.
middleware/timeout/timeout.go	Ensure timeout errors propagate; allow *fiber.Error from OnTimeout to shape response.
middleware/timeout/timeout_test.go	Tests for timeout error propagation, shaping, and skipping app ErrorHandler.
middleware/timeout/config.go	Clarifies OnTimeout semantics and concurrency expectations.
middleware/static/static.go	Per-route file handlers; avoid attachment leakage on misses; preserve detected Content-Type; MaxAge only on success.
middleware/static/static_test.go	Tests for download/header leakage, per-prefix correctness, and MaxAge on errors.
middleware/sse/sse.go	Round sub-millisecond retry up to 1ms.
middleware/sse/sse_test.go	Regression test for sub-millisecond retry rounding.
middleware/sse/event.go	Shared retry-ms rounding helper for event output.
middleware/session/session.go	Preserve/restore absolute expiration across session reset.
middleware/session/session_test.go	Regression test that rotated sessions still expire absolutely.
middleware/proxy/proxy.go	Restore original Host after forwarding; DomainForward continues on mismatch; match Host with optional port.
middleware/proxy/proxy_test.go	Tests for DomainForward mismatch continuation and Forward host restoration.
middleware/paginate/paginate.go	Ignore sort query when SortKey is empty to prevent nameless param control.
middleware/paginate/paginate_test.go	Tests for empty SortKey behavior and NextPageURL overflow handling.
middleware/paginate/page_info.go	Prevent integer overflow in next-page calculation.
middleware/limiter/limiter_test.go	Regression tests for sub-second expiration semantics and Retry-After behavior.
middleware/limiter/config.go	Treat any non-positive Expiration as default (preserve sub-second positives).
middleware/keyauth/keyauth.go	Add challenge when ErrorHandler returns *fiber.Error with 401/407.
middleware/keyauth/keyauth_test.go	Regression test for challenge behavior when ErrorHandler returns errors.
middleware/favicon/favicon.go	Avoid MaxBytes+1 overflow for math.MaxInt64.
middleware/favicon/favicon_test.go	Regression test for MaxBytes overflow behavior.
middleware/etag/etag.go	Restrict 304 behavior to GET/HEAD while still setting ETag for others.
middleware/etag/etag_test.go	Regression tests for method restrictions around If-None-Match/304.
middleware/compress/compress.go	Proper Accept-Encoding negotiation; avoid draining streams for ETag; restore request headers post-negotiation.
middleware/compress/compress_test.go	Tests for negotiation, weak ETag on streams, and request header preservation.
middleware/cache/utils.go	Prevent resident-time underflow when clock steps backwards.
middleware/cache/config.go	Adds internal accounting hook field (used by cache internals/tests).
middleware/adaptor/adaptor.go	Copy context values through derived contexts; propagate TLS state; respect URL rewrites; guard against post-flush next.
middleware/adaptor/adaptor_test.go	Tests for derived-context copying, TLS propagation, and URL rewrite routing.
log/default.go	Ensure Fatal exits even when log level suppresses output; add test hook via osExit.
log/default_test.go	Regression test for fatal exit behavior above log level.
listen.go	Default ShutdownTimeout applied; negative disables timeout; reject partial TLS keypair config; graceful shutdown goroutine stop signal.
listen_test.go	Tests for shutdown hooks once, no hooks on listen error, timeout defaults/negative semantics, partial TLS config failure, per-conn CHI.
internal/storage/memory/memory.go	Round expiration up to whole seconds; clamp; clarify non-positive semantics.
internal/storage/memory/memory_test.go	Tests for rounding-up expirations and sub-second GC intervals.
internal/storage/memory/config.go	Treat non-positive GCInterval as default (preserve sub-second positives).
internal/memory/memory.go	Round TTL up to whole seconds with clamp.
internal/memory/memory_test.go	Tests for TTL rounding and sub-second expiry.
hooks_test.go	Make tests that redirect global logger output non-parallel; restore output with cleanup.
helpers.go	RFC 9110 specificity-aware content negotiation resolution; new helper functions for offer selection.
docs/middleware/timeout.md	Documents timeout limitations: handler goroutine behavior and ErrorHandler skip semantics.
docs/middleware/static.md	Documents per-route prefix handling, Download header behavior, and MaxAge only on success.
docs/middleware/sse.md	Documents retry ms rounding behavior.
docs/middleware/proxy.md	Documents host matching with/without port and restoration after forwarding.
docs/middleware/logger.md	Corrects bytesSent/bytesReceived semantics timing and sentinel values.
docs/middleware/keyauth.md	Documents challenge behavior when ErrorHandler returns *fiber.Error.
docs/middleware/etag.md	Documents 304 limited to GET/HEAD; other methods pass through.
docs/middleware/compress.md	Documents encoding negotiation, header preservation, and weak ETag for streamed bodies.
docs/middleware/cache.md	Documents that 206 is not stored and why (range semantics).
docs/middleware/adaptor.md	Documents Locals vs Context value access, derived-context copying, TLS propagation, and rewrite behavior/limits.
docs/guide/routing.md	Documents escaped * literal behavior, constraint case handling, and auto-HEAD behavior with rebuild/mount nesting.
docs/client/rest.md	Documents helper-owned request lifecycle; scheme case-insensitivity; redirect constraints.
docs/client/response.md	Documents Header returns a copy; Close request-ownership semantics.
docs/client/request.md	Documents caller-owned requests; header override semantics; JSON/XML header behavior; redirect constraints.
docs/client/hooks.md	Documents header override semantics; tolerant cookie parsing; debug logging behavior with streams/nil logger.
docs/api/services.md	Documents uniqueness requirement and reverse shutdown order.
docs/api/log.md	Corrects default logger output stream description.
docs/api/fiber.md	Documents StrictRouting param routes; UnescapePath semantics; TLS config requirement; graceful shutdown semantics (incl. prefork).
docs/api/ctx.md	Documents Accept specificity rule; per-connection ClientHelloInfo; IP validation accepts IPv4-mapped IPv6; SendFile Accept-Encoding behavior.
docs/api/bind.md	Clarifies bound string lifetime; Immutable does not apply; ParserConfig SetAliasTag ignored.
docs/api/app.md	Documents multi-prefix mount behavior, self-mount panic, Name applies to multi-method registration, ErrorHandler skip after fasthttp timeout response, auto-HEAD on RebuildTree.
ctx.go	Per-connection ClientHelloInfo tracking; handler ctx helper; unescape path semantics; bind uses handler ctx.
ctx_test.go	Adds regression tests for SendFile FS comparability, Accept-Encoding preservation, AutoFormat fallback, IP validation, Accept specificity, custom ctx in Format, identity chain.
ctx_interface_gen.go	Updates interface docs and adds ctxForHandlers to generated interface.
client/response.go	Header returns copied string; Close releases request only if client-owned.
client/request.go	Stable sort for params/form data; reset clears client/clientOwned.
client/hooks.go	Case-insensitive scheme detection; clear/override header merge; tolerant cookie parsing; debug logging avoids draining streams and handles nil logger.
client/core.go	Capture request state before goroutine to avoid races on cancel; correct ownership cleanup on hook errors; redirect-method constraints.
client/cookiejar.go	Honor Max-Age precedence (incl. Max-Age=0/negative) with attribute-presence detection.
client/client.go	REST helpers use client-owned requests that Response.Close releases.
client/client_test.go	Regression tests for cookie parsing tolerance, request ownership, debug stream logging, Max-Age, cancellation race, header override, stable param order, Header copying, resend headers, uppercase scheme.
binder/query_test.go	Adds tests for correct splitting resolution across nested/embedded structures and invalid destinations.
binder/mapping.go	Reworks key-to-field resolution to avoid incorrect splitting; adds promoted/embedded handling and depth bounds.
binder/mapping_test.go	Expands tests for new splitting/key-resolution behavior and fieldName parsing.
binder/header_test.go	Tests map[string]string not being split while map[string][]string is.
bind_test.go	Regression test ensuring custom ctx reaches custom binder Parse and Body bind.
app.go	Canonicalize trusted proxy IP storage; Name applies to multi-method registrations; mount all prefixes; avoid ShutdownWithContext mutex deadlock; skip ErrorHandler after committed timeout response; hook ConnState for TLS handler cleanup; collect sub-apps before auto-HEAD.
app_test.go	Tests for shutdown mutex deadlock fix, canonical trusted proxies (incl. v4-mapped), ErrorHandler skip on committed timeout response.
addon/retry/README.md	Documents per-call backoff state semantics.
addon/retry/exponential_backoff.go	Make Retry state per-call (no shared mutation); clarify currentInterval semantics.
addon/retry/exponential_backoff_test.go	Tests for per-call reset and concurrency safety.

Review details

Files not reviewed (1)

• ctx_interface_gen.go: Generated file

• Files reviewed: 93/94 changed files
• Comments generated: 1
• Review effort level: Lite

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 15b0e60097

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 7cd34a9266

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[gaby]

@codex review

[chatgpt-codex-connector]

💡 Codex Security Review

Here are some automated security review suggestions for this pull request.

Reviewed commit: 8a56e9c4db

ℹ️ About Codex security reviews in GitHub

This is an experimental Codex feature. Security reviews are triggered when:

• You comment "@codex security review"
• A regular code review gets triggered (for example, "@codex review" or when a PR is opened), and you’re opted in so security review runs alongside code review

Once complete, Codex will leave suggestions, or a comment if no findings are found.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 8a56e9c4db

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[gaby]

Overlap check against the other open PRs

I went through all 14 other open PRs to see whether anything here already fixes them, so nothing gets merged twice or dropped. Nothing does — every one of them fixes a genuinely different bug, including the ones that touch the same files. Recording the checks so a reviewer does not have to redo them:

PR	Same ground?	Why it is still separate
#4590 — cache: delete stale entries after uncacheable revalidation	Same if hasPrivate || hasNoCache || varyHasStar block	Their bug is the persisted entry surviving when e was already released before revalidation (the oldHeapIdx fallback). This branch still carries the pre-fix if e != nil { / removeHeapEntry(key, e.heapidx) there. Ours were the orphaned-metadata miss, the stale heap node left by a no-cache refresh, and eviction running before the decision to store.
#4640 — binder: bound pooled decoder path cache	Same file, binder/mapping.go	Bounds the gofiber/schema decoder's internal request-path cache across pooled uses. This PR reworked equalFieldType / fieldInfo, a different cache — keyed per struct type and already bounded.
#4492 — cache: refactor lock flow	Same file, middleware/cache/cache.go	A chore closing #4334 (lock guard, dropping goto). The CacheInvalidator race fixed here is a different concern and does not remove the need for it.
#4648 — Bind().WithSplitting()	Same feature area	Purely additive: a per-call override of the global EnableSplittingOnParsers. This PR changed how the split destination is resolved, not whether the flag can be overridden per call.

The rest are clearly unrelated to anything here: #4653 (CORS/CSRF origin matching), #4642 (Vary case-insensitivity — Ctx.Vary is untouched here), #4636 (shared :name delimiters — adjacent to the delimiter behavior this PR deliberately left alone), #4629 (escaping values in generated URLs, not UnescapePath), #4628 (request charset parser), and the features #4596, #4594, #4593, #4404, #3702.

Merge-order heads-up

Three of the four above will conflict textually with this branch, so whoever merges second has to resolve it:

• 🧹 chore: refactor cache handler lock flow #4492 rewrites 213/197 lines of the same middleware/cache/cache.go and already reports mergeable_state: dirty.
• 🐛 fix: Delete stale entries after uncacheable revalidation #4590 edits the exact block where this PR changed removeHeapEntry to return (evictionCandidate, bool) — the call still compiles as a statement, but the two edits land on the same lines.
• 🐛 fix(binder): bound pooled decoder path cache #4640 restructures binder/mapping.go and binder/mapping_test.go around code this PR also changed, including Benchmark_equalFieldType.

Happy to rebase onto whichever of these lands first, or to hold this one — say which order you would like.

---

Generated by Claude Code

[ReneWerner87]

Went through this branch against 1783fe81. Everything below reproduces as a base-vs-head differential.

Three that look blocking

1. adaptor.HTTPMiddleware resumes past middleware that already ran (adaptor.go:513-527)

The guard compares the decoded, normalised path (freq.URI().Path()) against the raw path the router matched on (c.Path() = PathOriginal()). Those differ for any non-canonical path with no rewrite by the wrapped middleware, and c.Path(override) then re-buckets the tree so c.Next() resumes after the adaptor's own index.

app.Use("/api/admin", auth)
app.Use(adaptor.HTTPMiddleware(passthrough))
app.Get("/api/admin/secret", handler)

GET /api%2Fadmin/secret    base 404   head 200 "SECRET"  authRan=false
GET //admin/secret         base 404   head 200 "SECRET"  authRan=false
GET /./admin/secret        base 404   head 200 "SECRET"  authRan=false
GET /x/../admin/secret     base 404   head 200 "SECRET"  authRan=false
GET /adm%69n/secret        base 404   head 200 "SECRET"  authRan=false

Trigger set: any percent-escape including a no-op one (%41, %7E), any duplicate slash, any . or .. segment. Not +, ;params, backslash, case, query or fragment. Adaptor-before-guard is safe; guard-then-adaptor-then-route is the affected order.

2. Mixed-case constraint names stop enforcing (router.go:1053, router.go:844, path.go:243)

app.Get("/:id<INT>", h);  GET /abc     base 404   head 200  id="abc"
same for <Int> <iNt> <Bool> <Guid> <Alpha> <Float> <DateTime> <Range(5,10)> <Regex>
<int> and <minLen>/<maxLen>/<betweenLen> unaffected

Three entry points: register, addPrefixToRoute (mounted sub-apps), RoutePatternMatch. With CaseSensitive: true this is already broken on main; what changes here is the default config, which worked because the pattern was lowercased before parsing.

3. ClientHelloInfo retained per hijacked connection (ctx.go:129/:154, app.go:1769)

StateHijacked is terminal in fasthttp (server.go:3199, workerpool.go:237-241), so forget never runs.

hijacked TLS conns   50/100/200/300/600
clientHelloInfos     50/100/200/300/600     serverConns=0
StateNew=n  StateHijacked=n  StateClosed=0
non-hijacked control 0     ~2.4 KiB/entry

Every WebSocket upgrade over TLS hijacks. The one-line fix below covers only part of it: with MaxConnsPerIP set, 200 ordinary non-hijacked TLS requests still retain 192 records, because fasthttp refuses at the per-IP limit before StateNew fires. Given this component has produced three defects across three correction rounds, keying the record by the *tls.Conn rather than the pooled wrapper may be worth more than another patch.

Patch

Closes 1, 2, the sameFS slice collision, the SendFile allocation and the Group.Use gap, plus the hijack half of 3.

Verified: all six bypass spellings back to 404, http.StripPrefix still 200 (that support is new here, so it must not break), every constraint spelling including MİNLEN enforcing again, sameFS 2 allocs to 0, Group.Use 200/200, adaptor suite green under -race.

+25 -10 across 6 files

diff --git a/app.go b/app.go
index 7de60364..7134e843 100644
--- a/app.go
+++ b/app.go
@@ -1766,7 +1766,8 @@ func (app *App) hookConnState() {
 	app.connStateHooked = true
 	user := app.server.ConnState
 	app.server.ConnState = func(conn net.Conn, state fasthttp.ConnState) {
-		if state == fasthttp.StateNew || state == fasthttp.StateClosed {
+		// StateHijacked is terminal too: fasthttp never reports StateClosed for it.
+		if state == fasthttp.StateNew || state == fasthttp.StateClosed || state == fasthttp.StateHijacked {
 			app.mutex.Lock()
 			handler := app.tlsHandler
 			app.mutex.Unlock()
diff --git a/constraint.go b/constraint.go
index 2648a7d9..860746b3 100644
--- a/constraint.go
+++ b/constraint.go
@@ -11,6 +11,7 @@ import (
 	"time"
 	"unicode"
 
+	"github.com/gofiber/utils/v2"
 	"github.com/gofiber/utils/v2/swar"
 	"github.com/google/uuid"
 )
@@ -163,11 +164,13 @@ func findConstraintHandler(name string, regexHandler any, customs []CustomConstr
 			return &customConstraintWrapper{CustomConstraint: cc}
 		}
 	}
-	if name == ConstraintRegex {
+	// Built-in names are matched case-insensitively: the pattern is adopted as
+	// written, so a mixed-case spelling must still resolve.
+	if utils.EqualFold(name, ConstraintRegex) {
 		return regexConstraintType{regexHandler: regexHandler}
 	}
 	for _, bc := range builtinConstraints {
-		if bc.Name() == name {
+		if utils.EqualFold(bc.Name(), name) {
 			return bc
 		}
 	}
diff --git a/domain.go b/domain.go
index 11cd95d1..e7abc5d6 100644
--- a/domain.go
+++ b/domain.go
@@ -377,7 +377,8 @@ func (d *domainRouter) Use(args ...any) Router {
 
 	for _, prefix := range prefixes {
 		if subApp != nil {
-			return d.mount(prefix, subApp)
+			d.mount(prefix, subApp)
+			continue
 		}
 
 		wrapped := d.wrapHandlers(handlers)
diff --git a/group.go b/group.go
index 34cc324b..a9ef6e1c 100644
--- a/group.go
+++ b/group.go
@@ -96,7 +96,8 @@ func (grp *Group) Use(args ...any) Router {
 
 	for _, prefix := range prefixes {
 		if subApp != nil {
-			return grp.mount(prefix, subApp)
+			grp.mount(prefix, subApp)
+			continue
 		}
 
 		grp.app.register([]string{methodUse}, getGroupPath(grp.Prefix, prefix), grp, handlers...)
diff --git a/middleware/adaptor/adaptor.go b/middleware/adaptor/adaptor.go
index 71ff3eb7..b68ffb56 100644
--- a/middleware/adaptor/adaptor.go
+++ b/middleware/adaptor/adaptor.go
@@ -514,17 +514,22 @@ func HTTPMiddleware(mw func(http.Handler) http.Handler) fiber.Handler {
 			fhdr.SetMethod(r.Method)
 			// A rewrite of r.URL (http.StripPrefix) leaves RequestURI untouched; route the URL.
 			requestURI := r.RequestURI
+			newPath := ""
 			if r.URL != nil {
 				if rewritten := r.URL.RequestURI(); rewritten != "" && rewritten != requestURI {
 					requestURI = rewritten
+					// Clone now: r.URL aliases the storage SetRequestURI overwrites.
+					newPath = strings.Clone(r.URL.EscapedPath())
 				}
 			}
 			freq.SetRequestURI(requestURI)
 			freq.SetHost(r.Host)
 			fhdr.SetHost(r.Host)
-			// The router matches the path derived at request start, so install the rewritten one too.
-			if path := string(freq.URI().Path()); path != c.Path() {
-				c.Path(path)
+			// Only a genuine rewrite re-routes; the decoded path differs from the raw
+			// one for every escaped request, and overriding it there would resume the
+			// chain past middleware that already ran.
+			if newPath != "" && newPath != c.Path() {
+				c.Path(newPath)
 			}
 
 			// Remove all cookies before setting, see https://github.com/valyala/fasthttp/pull/1864
diff --git a/res.go b/res.go
index 0400f3a7..39e95b7e 100644
--- a/res.go
+++ b/res.go
@@ -116,12 +116,16 @@ func sameFS(a, b fs.FS) bool {
 	if va.Type() != vb.Type() {
 		return false
 	}
-	if va.Comparable() && vb.Comparable() {
+	if va.Type().Comparable() {
 		return a == b
 	}
 
 	switch va.Kind() {
-	case reflect.Map, reflect.Slice, reflect.Chan, reflect.Pointer, reflect.UnsafePointer:
+	case reflect.Slice:
+		// Pointer() is &elem[0] and ignores length, so two prefixes of one
+		// backing array would otherwise look like the same file system.
+		return va.Pointer() == vb.Pointer() && va.Len() == vb.Len()
+	case reflect.Map, reflect.Chan, reflect.Pointer, reflect.UnsafePointer:
 		return va.Pointer() == vb.Pointer()
 	default:
 		// A func's pointer is its code entry, shared by every closure over the

Two placements are load-bearing. The strings.Clone has to happen before SetRequestURI, since r.URL and r.RequestURI are b2s views into the storage that call overwrites (a clone taken afterwards yields esc="llollo" for /hello). And utils.EqualFold is deliberately in the builtins loop only: applying it to customs as well collapses two customs differing only in case, and lets a custom named INT capture <int>. It does not replace resolveConstraintName either, being ASCII-only, so MİNLEN still needs the ToLower path.

Not in the patch

• sameFS(x, x) is still false for a non-comparable struct (one with a map, slice or func field), so SendFile appends a store and starts a fasthttp.FS cleanup goroutine per request: 10,100 requests give 10,100 stores and 10,103 goroutines, and forced GC reclaims none because app.sendfiles is append-only. Not a regression, since base panics on those types, but the crash became a leak. Fixing it means separating "cannot decide identity" from "different" and skipping the cache append in the first case, which is more than a one-liner.
• Concurrent Shutdown() terminates each service once per caller (base [1,1,1] at every Terminate duration, head [2,2,2] at two callers, [4,4,4] at four). Please do not fix this by restoring the mutex: that release fixes a real deadlock, base takes 8s+ to return when an in-flight handler holds app.mutex and head takes 0.4s. Draining state.services under servicesMu instead of copying gives [1,1,1].
• Route.id is shared across a register and preserved by copyRoute, so one sub-app under two same-bucket prefixes gives two same-id routes and routeIndexInTree rewinds into middleware that already ran. Needs the overriding middleware inside the sub-app to reproduce. A fresh id per clone regresses Req.Method override to 405; a per-mount id remap fixes both.
• proxy.DomainForward non-match now calls c.Next(), so Host: evil.example.net reaches the routes behind it. Documented in proxy.md:70; worth a release-note line since it removes a host-isolation property.
• SendFile{Compress: false} no longer suppresses a downstream compress.New(). The field comment was updated; the example at docs/api/ctx.md:3745 still says // Disable compression.
• Two allocation regressions the repo's benchmarks cannot see. compress.go:199 utils.CopyBytes(line) escapes, adding one alloc to every compressed response, and Benchmark_Compress reuses one RequestCtx without resetting the response, so from iteration 1 the middleware early-returns (body sizes [10552, 37699, 37699, 37699], 2610 ns/op reported for a 37,699-byte README). Separately, AcquireRequest + resp.Close() goes 2 to 27 allocs/op since Close no longer returns caller-owned requests.
• Negotiation: uniformQuality replacing !hasRejections as the fast-path condition means any header with differing q values takes the rescan. Uniform-q control is +3.2% (n.s.), browser-shaped headers +35 to +106%, uniform-q ones 10-25% faster. About 56% is recoverable by returning the specificity from firstMatchingOffer, restricting the demotion scan to the strictly-lighter suffix, and breaking on first demotion.
• router.go:1189 writes the auto-HEAD memo from a defer, so a panicking OnRoute hook records an aborted scan as complete and HEAD stays 405 for the process lifetime. Base rescans.
• cookiejar.go:860 uses Atoi into int, so Max-Age=3000000000 silently downgrades a persistent cookie to a session cookie on 32-bit. Verified by building for GOARCH=386 and arm; there is no 32-bit CI job.

Four numbers I measure differently

• Benchmark_equalFieldType "273 to 261 ns/op": against the merge base with a byte-identical benchmark body I get 76-81 to 116-121 ns/op, matching the red gate at 1.65x. The quoted pair looks like a branch-internal before/after of 5f0c620f (its parent measures 130.0, it measures 119.8) placed under the "vs. the merge base" heading. The regression is real but narrow: one non-test caller, gated on EnableSplittingOnParsers (off by default) plus a comma in the value, so a realistic bind is +1-5%. Hoisting getFieldCache out of the walk and not calling cachedFieldInfo twice at mapping.go:476 should clear the gate.
• Benchmark_App_RebuildTree "back to parity": 22886 to 33054 B/op (+44.4%), 75 to 84 allocs, deterministic. The cause is legitimate (239 to 381 routes from the auto-HEAD twins), so it reads better as the price of the fix. The ShutdownServices parity claim does check out.
• The adaptor depth bound: there is none on base or head, and a 202-level WithValue chain copies all 202 on both. const maxContextDepth = 64 did live on this branch for 25 commits before the last commit removed it, which is presumably where docs/middleware/adaptor.md:302 came from. That line is now stale.
• Cache, "a store that failed after its space had been reserved leaked that reservation": base already unreserves in its own defer at cache.go:738-746, unchanged at every commit on the branch, and Test_Cache_VaryManifestStoreFailureUnreservesSpace passes on base.

On the test claim: running each new test in its own process against head-tests-over-base-production gives 104 of 141 failing on unfixed code, 12 passing, and 25 that do not compile against base at all (they reference sameFS, underlyingConn, ErrCertFileAndKeyRequired, Config.accounting, clientOwned and similar). 104 is a good number; the wording just covers more than it can.

Unrelated, but worth an issue

The root package is order-unstable under go test . -race -shuffle=on on both trees: base fails 4 of 6 runs, head 5 of 6, scattered across Test_Ctx_SendFile_*, Test_RoutePatternMatch_MatchesRouter, Test_Router_ScanMatchesReference and Test_App_BodyLimit_LargerThanDefault. Each passes in isolation, so it is test-order coupling rather than a defect in either tree. make test has no rerun while .github/workflows/test.yml:43 sets rerun-fails: '2', so CI stays green over it.

Also pre-existing and unrelated to this PR: req.go:192. With an all-identity Content-Encoding chain and ReduceMemoryUsage: true, Body() calls SetBodyRaw(originalBody), whose ResetBody returns req.body to the pool while the handler still holds a slice aliasing it, so the next request's socket read overwrites it. 64 goroutine pairs, no injected poisoning: roughly 150 to 2450 of 3200 victim handlers read another request's body, on head and base alike, with -race naming bufio.(*Reader).Read / fasthttp.appendBodyFixedSize against the handler's read. Happy to open that one separately.

A lot here is right, so for balance: the compress parser fixes real breakage (base did not compress at all for any header carrying a q or a *), the binder predicate matches gofiber/schema across 60+ differential pairs with zero disagreements, the client cancel-path copy removes 40+ genuine data races present on base, and the trusted-proxy canonicalisation fixes a case where a non-canonical Proxies entry did not match even its own spelling.

[gaby]

Thank you — this is the most useful review the PR has had, and the three blocking findings are all real. Fixed in d80d7e3, each reproduced as a failing test first and mutation-checked against the unfixed code afterwards.

The three that block

1. Adaptor resumed past middleware that already ran. Confirmed, and it is the worst thing in the PR: an auth guard mounted before the adaptor never ran. Your diagnosis is exact — the guard compared freq.URI().Path() (decoded, normalised) against c.Path() (PathOriginal()), which differ for every escaped or non-canonical request, so c.Path() re-bucketed the tree and c.Next() resumed after the adaptor's index. Took your fix including the strings.Clone placement; a clone after SetRequestURI does yield the corrupted read you describe. Test_HTTPMiddleware_NoRewriteKeepsChainPosition drives all five spellings and asserts 404 with authRan == false; Test_HTTPMiddleware_URLRewrite still passes, so http.StripPrefix support survives.

2. Mixed-case constraint names. Confirmed — 8 of the 10 spellings I tested returned 200 for input the constraint should reject, with <int> and the length constraints unaffected exactly as you said. Took your constraint.go fix, including keeping utils.EqualFold out of the customs loop for the two reasons you gave. Test_Router_Constraints_MixedCaseNames covers all three entry points (register, the mounted sub-app, RoutePatternMatch).

3. ClientHelloInfo on hijacked connections. Confirmed and fixed with your one-liner, plus Test_TLSHandler_ForgetsHijackedConnection.

On the deeper point — "this component has produced three defects across three correction rounds" — you are right and I am not going to patch it a fourth time on my own judgement. The remaining hole you name (records retained when fasthttp refuses at the per-IP limit before StateNew fires) is real precisely because my design assumes StateNew fires for every connection that completes a handshake, and there are paths where it does not. Keying by the *tls.Conn is not directly available either — GetClientInfo receives the raw conn, not the *tls.Conn. What removes the whole class is wrapping the raw conn in Fiber's own listener wrapper and storing the info on that wrapper, no map at all: cleanup then rides on Close(), which is universal and covers hijack, per-IP refusal and everything else. The catch is the public SetTLSHandler + user-supplied-listener path, where Fiber does not build the listener and would still need a fallback. I would rather do that as its own PR with your review than bolt it on here. Say the word if you want it in this one instead.

Four more from your list, also fixed

• Concurrent Shutdown double-terminates. Worse than reported: with four callers I get four terminations per service and a -race violation. Fixed your way — drained under servicesMu, not copied — and the mutex release stays exactly as it is. I kept the retry-on-failure property by putting back only the services that would not terminate, in start order. (Test_ShutdownServices_ConcurrentCallersTerminateOnce)
• Group.Use and domainRouter.Use mounted only the first prefix — the same bug I fixed for App.Use and missed twice. Both fixed, both tested.
• sameFS slice collision. Fixed with your va.Type().Comparable() + length check.
• Max-Age into a 32-bit int. Mine, introduced two days ago. Now parsed at 64 bits and carried through as int64. I cannot make this fail on amd64, so the test pins the contract rather than reproducing the platform bug; your GOARCH=386/arm runs are the real evidence.
• Auto-HEAD memo written from a defer. Fixed — recorded on the normal exits only, so a panicking OnRoute hook no longer marks an aborted scan complete. (Test_App_AutoHead_PanickingHookLeavesScanIncomplete, which fails if the defer is put back.)

Your four numbers: you are right on all of them

I checked each against the merge base rather than argue.

• Benchmark_equalFieldType. You are right that "273 → 261 ns/op" is not a base-vs-head figure. My own measurement is base 115.5 → head 177 ns/op, a 1.53x ratio matching both your 76-81 → 116-121 and the bot's 1.65x. The PR body presented a branch-internal before/after under a "vs. the merge base" heading. That is my error and the body is now corrected.
• Benchmark_App_RebuildTree. Measured: base 22888 B/op, 75 allocs → head 33050 B/op, 84 allocs, +44.4%. Your number, not mine; "back to parity" was wrong for the same reason — 42563 was a branch-internal intermediate. Corrected, and reframed as the price of the auto-HEAD fix, which is how you put it.
• The adaptor depth bound. Correct — maxContextDepth was removed in 9d1dca9 when cycle detection replaced it, and docs/middleware/adaptor.md:302 was left behind describing it. Fixed.
• Cache reservation leak. Correct, and this is the one that matters most: the merge base already unreserves in its own defer at cache.go:738-746, and I confirmed Test_Cache_VaryManifestStoreFailureUnreservesSpace passes against base production code with only the branch-only accounting seam stripped. That bullet described a bug that did not exist. What this branch actually adds there is the replaced-entry restore (4246585, bc930b6), which is a different and real fix. The claim is removed from the body.

Also fair on the test wording: "each written before its fix" covers more than it can, and 104-of-141 is the honest figure.

Still open, and I am not claiming otherwise

Taking these in a follow-up rather than growing this diff further, unless you would rather they land here:

• sameFS(x, x) false for a non-comparable struct → the SendFile store/goroutine leak. Needs "cannot decide identity" separated from "different", as you say.
• Route.id per-mount remap.
• proxy.DomainForward non-match reaching the routes behind it — agreed this needs a release-note line, since it removes a host-isolation property.
• The two allocation regressions, and the Benchmark_Compress fixture reusing one RequestCtx without resetting the response.
• The negotiation fast path — your ~56% recovery sketch (return the specificity from firstMatchingOffer, restrict the demotion scan to the strictly-lighter suffix, break on first demotion) is more concrete than anything I had.
• Your getFieldCache hoist for equalFieldType. I tried a reflect.TypeOf-based walk earlier and it made things worse (177 → 207 ns), so I will measure yours rather than assume.
• The req.go:192 SetBodyRaw aliasing race and the -shuffle=on order-instability — both pre-existing on base, and yes please, separate issues. I will not touch them here.

Full suite green under -race -shuffle=on, lint 0 issues, gofumpt clean. I also merged the main you brought in (#4653) and re-validated on top of it.

---

Generated by Claude Code

[gaby]

Benchmark_equalFieldType: 1.60x → ~1.06x in 51c0085

@ReneWerner87 — you asked for the gate to be cleared rather than explained, and you were right that it could be. The bug fix (the real walk down the struct) is unchanged; the cost was mostly around it, not in it.

What the profile actually said. With the walk in place, the sync.Map holding each type's field information was ~37% of the benchmark: its interface-keyed hash (nilinterhash → typehash → efaceeq) ran up to four times per call. Another ~20% was reflect.Value construction in equalFieldType and repeated Kind() interface calls in structKeyKind. The string-map lookups (~30%) are at parity with base — it did the same four.

What changed.

• A 64-slot direct-mapped cache now sits in front of the sync.Map, keyed by the address of the type descriptor — stable and unique for the life of the program. A hit is an atomic load and an integer compare; a collision only falls through to the map, so nothing is ever wrong, just slower. The address is read straight from the interface's data word (one unsafe line, commented and nolint-annotated): reflect.ValueOf(t).Pointer() returns the same word and, measured in isolation, cost more than the lookup it was serving (~13 ns/iter of the total).
• Your getFieldCache hoist: the per-tag cache is resolved once per equalFieldType call and passed down, not re-resolved per level.
• equalFieldType walks the type rather than a reflect.Value; a Value is only taken for the one question the type cannot answer, whether a map passed by value is nil. structKeyKind carries each level's kind instead of asking the type again.

Numbers, interleaved medians against the merge base on one machine, allocations unchanged at zero:

	base	head	ratio
before	150.5 ns	240 ns	1.60x
front cache	150.5 ns	210 ns	1.37x
+ type walk, kind tracking	150.5 ns	176 ns	1.17x
+ direct descriptor read	150.5 ns	160 ns	1.06x

The residual is within the benchmark's own run-to-run spread here (base alone ranged 148–159 across rounds). The bot's re-measurement on its own hardware is the arbiter; I am not claiming parity from one machine again.

One behavioral note. A typed nil pointer to a struct now gets the answer main gives it — what kind the named field has — rather than false. That nil check was this branch's addition, cost a reflect.Value walk on every call, and decided a case the decoder rejects anyway.

Race-clean, lint 0 issues, full suite green. Two tests that poison the type cache to exercise the mismatch path now also clear the slot in front of it, so they keep testing what they claim to.

---

Generated by Claude Code

[github-actions]

✅ No significant benchmark change.

_{51c0085 vs main@ac861c5 · 1692/1692 results compared · retest: 1 reported re-checked · noise-aware thresholds · full results}