Consolidate sanitization/normalization helpers and remove double processing
Core and middleware carry around 40 duplicated or divergent transform sites (origin/host normalization, comma-list scanning, ETag parsing, control-char handling, quoted-string production, URL composition guards), grouped into 13 clusters below. Goal: one implementation per concern, at most one pass per value, and the fasthttp trust boundary written down at the call sites that depend on it. Suggested order: clusters 1, 2 and 5 first (most call sites).
New helpers go to internal/ packages (no new public API); only generic byte primitives belong in gofiber/utils.
Fix first
Three small bug-ish fixes that can land as standalone PRs before any consolidation:
- router.go
buildRouteURLsubstitutes path params with no escaping;urlnorm.RootedPathafterwards defuses a leading//, but a param containing?or#still restructures the URL, whileRedirect().Route()escapes query values viautils.AppendQueryEscape. Escape substituted params with path-segment rules (or reject structure-altering values). - client/hooks.go
parserRequestURLsubstitutes path params via unanchoredstrings.ReplaceAll(uri, ":"+key, val)::idalso matches inside:idx, and unescaped values containing/,?or#restructure the request target. Anchor at segment boundaries and percent-encode values. - extractors/extractors.go
FromParamrunsurl.PathUnescapeonc.Params(). WithUnescapePath: false(default) that is the single decode; withUnescapePath: truethe path was already decoded once inconfigDependentPaths, so params get decoded twice (%2520arrives as a space). Decide which layer decodes and make the count independent of the config flag.
What fasthttp v1.73.0 already guarantees (server path)
- Every exported header-value write (
Set,Add,SetCanonical,SetCookie,SetContentType, special setters) funnels throughinitHeaderValueBytes->removeNewLines: CR and LF are replaced with a space, silently; header keys are CR/LF-filtered too; trailer keys are the one different shape (validated and rejected instead). No exported API bypasses the value filter. Fiber-side CRLF stripping before a header write is therefore browser-parity normalization at most, never injection defense. - Parsed request-header values reject C0 bytes except HTAB, and reject DEL; bytes >= 0x80 pass. A violating request is answered 400 and the connection closed; obs-folds are collapsed to one space. A request target containing any CTL byte is rejected too.
- Server-side
URI().Path()is always percent-decoded and normalized (rooted,//dedup,/./and/../resolved). Decoding runs before the collapsing, so an encoded%2Fbecomes a real separator inPath().PathOriginal()keeps the raw bytes. No server-side switch disables this; the exportedURI.DisablePathNormalizingfield only changesRequestURI()output, neverPath(). URI().Host()is lowercased, unescaped and validated at parse time;URI.SetHostonly lowercases;Request.Header.Host()stays raw.- Cookie writes strip CR/LF and replace
;with a space (SetPathadditionally normalizes); cookie values are never percent-coded by fasthttp; invalid request-cookie values are silently dropped at parse.
Gaps fiber must keep covering itself: NUL and other C0/DEL bytes pass the header write filter (values are mutated silently, never rejected); on Unix \ is not a path separator for fasthttp's normalization, so %5C sequences survive into Path(); RequestCtx.Redirect appends fragments raw.
Clusters
- 1. Origin toolkit (cors/csrf):
normalizeOriginis near-identical in middleware/cors/utils.go and middleware/csrf/helpers.go (sole delta: csrf rejects non-http(s) schemes), the wildcard-subdomain matcher is character-identical in both, csrf additionally carries a test-onlysubdomain.match(dead production code), config-timeTrimSpaceand the redaction closure are duplicated too. The compare paths diverge: csrf uses internal/schemehost.Match (default-port folding), cors a lowercased map lookup without port folding, and the csrf referer check builds an origin string a third way. Target: one internal origin package consumed by both; today's deltas (scheme allowlist, port folding, idempotence check) become documented policy instead of accidents. - 2. Host normalization and IDNA: three comparators disagree (middleware/hostauthorization
normalizeHost: lowercase, port strip, bracket strip, trailing-dot strip, punycode; internal/schemehostnormalizeHostPort: lowercase plus default-port folding; proxy DomainForward: bareEqualFoldon the raw Host header, no port strip), plus the fold pairs in domain.go/Subdomainsand threeToLower(host)copies in client/cookiejar. IDNA alone runs with three profiles in three directions: hostauthorizationidna.Lookup.ToASCII, req.goSubdomainsidna.Lookup.ToUnicode, middleware/redirect a custom lenientidna.Newprofile. Target: one tiered helper (Fold / Strip / Canonical) with IDNA as a named policy; prefer fasthttp's already-cleanURI().Host()over re-folding the raw header (note: only the parse path validates,SetHostdoes not). - 3. Route pattern preprocessing: the trio "lowercase when !CaseSensitive, trim trailing
/when !StrictRouting, RemoveEscapeChar" exists three times in router.go (addPrefixToRoute,normalizePath,register) and again in path.goRoutePatternMatch;RemoveEscapeCharhas string and bytes twins.getGroupPathtrims the prefix and mount.go, domain.go and router.go trim its return value again. Target: oneNormalizePatternhelper;configDependentPathsstays the single request-time pass, with a comment stating that fiber routes onPathOriginal()(raw) by design, thatUnescapePathis the single path decode (see fix-first item 3 for the extractor interplay), and that fasthttp's normalizedPath()is only consumed by the fasthttp.FS flows. - 4. Location composition: the guard level depends on the entry point.
Back()normalizes via inline copies in redirect.gonormalizeRefererURL(verbatim re-implementation ofurlnorm.AsBrowserReads/StripTabCRLF; middleware/redirect and router.go already use the shared helpers, redirect.go is the lone holdout) plus backslash->slash and a leading-slash-run collapse to two slashes, whereurlnorm.RootedPathcollapses to one - a policy split to reconcile.To()andRes.Location()write raw;Redirect().Route()runsRootedPathbut not the strip pair. middleware/redirect normalizes rule targets at config time and again per request (the second pass is needed for substituted request data; split the passes instead of deleting one). middleware/rewrite composes with no guards; that is safe because fasthttp'sURI().SetPathre-runsnormalizePath, and that mechanism deserves the comment. The two glob->regex helpers anchor differently ("^"+k+"$"vs"^(?:"+k+")$", so alternation rules behave differently) and the twocaptureTokensdiverge on trailing-slash trimming. Cleanup: a redirect.go comment still referencesasRoutePath, which no longer exists. Target: internal/urlnorm becomes the single pipeline with one policy table; one shared glob->regex + captureTokens helper; document the strip-vs-reject split against the client side (cluster 13). - 5. Comma-list header scanning: three real scanners in core (helpers.go
getSplicedStrList, ctx.goheaderListContainsToken, res.goheaderContainsValue) plus the join/dedupe wrappers around them (joinHeaderValues,peekJoinedResponseHeader,appendUniqueValues), and four more scanners in middleware (adaptorhasCloseToken, proxydelConnectionListedHeaders, compresshasTokenfor Cache-Controlno-transformand Vary, cacheparseVary) plus cachejoinedHeader. Target: internal/headerlist with Contains/ForEach/Join/AppendUnique plus a quote-aware iterator for the consumers that need it. - 6. ETag: middleware/etag
isNoneMatchsplits If-None-Match naively on commas (its own comment admits commas inside opaque-tags mis-parse) while coreisEtagStaleis quote-aware;etagWeakMatchduplicates corematchEtag/matchEtagStrong, which additionally factor throughnormalizeEtag. Target: internal/etag Parse/Match/Format used by bothFresh()and the middleware. - 7. Media-type parameters: three quote/escape-aware scanners (helpers.go
unescapeHeaderValue+forEachMediaRange/getOffer, req.goCharsetwith its own state machine, the param-strip+fold pair inMediaTypeand etagisEventStream), plus middleware/cacheunquoteCacheDirectiveas an orphan third quoted-pair unescaper. internal/mediatypeNormalizeRequestContentTypefolds fasthttp's header buffer in place; the aliasing contract is documented at the function, the risk is call-site discipline, so keep one entry point. Target: internal/mediatype owns param iteration, quoted-pair unescape and param-strip, including the Cache-Control consumer. - 8. Forwarded headers: the per-element trim for X-Forwarded-For exists three times (
extractIPsFromHeader,extractIPFromHeaderforward and reverse), X-Forwarded-Host/-Proto parse separately; middleware/limiter's default key is a raw, unboundedc.IP()even when ProxyHeader-derived (contrast cache'sboundKeySegment, which bounds and escapes). Target: one forwarded-header iterator; bounded-key guidance for storage keys built from client-controlled values. - 9. Control-char handling: six SWAR masks, not two: helpers.go
quoteEscapeMask, internal/logtemplatecontrolScrubMask, proxyctlOrDELMask, basicauthasciiCTLMask(byte-identical to proxy's), basicauthvalidHeaderMask, requestidvisibleASCIIMask(a strict subset of the previous). Beyond the mask primitive, the policies disagree: the same class of user-controlled value is deleted (res.gosanitizeFilename, viaunicode.IsControl, so C1 too), replaced with a space (logtemplate), rejected (basicauth, requestid, proxy, sse) or percent-encoded (quoteRawString), with HTAB exempt in some and not others. Target: one parameterized mask primitive plus an explicit policy table (which value class gets which treatment); logtemplate stays the single log scrubber. - 10. Quoted-string production: basicauth Go-quotes Realm and Charset via
strconv.Quote, keyauth uses%qfor five params (realm, error, error_description, error_uri, scope); both emit Go string syntax (\\\uXXXX), not RFC 9110 quoted-string. helpers.goquoteRawStringis the correct dialect with two callers (Content-Disposition and theLinkrel=value in res.go), beside res.goencodeExtValuefor RFC 8187. Layering note: for Attachment/Download the CR/LF/C0 branches ofquoteRawStringare dead code,sanitizeFilenamealready deleted every control rune, and fasthttp scrubs a third time; the branches are live only forLinkrel=. Target: one shared producer with one documented layering. - 11. SameSite and cookie layering: res.go
Cookieholds two mappings (string -> http.SameSite -> fasthttp) and validates viahttp.Cookie.Valid()before fasthttp's silent CR/LF+semicolon mutation could trigger - that ordering is the contract, currently undocumented. middleware/session's own mapping recognizes only Strict and None and silently maps everything else to Lax, including the validCookieSameSiteDisabledthat res.go honors; it also overwritesCookieSecurein the non-None branch and never validates the config value at init. Target: one mapper honoring all four values; session validates at init; layering comment at res.Cookie. - 12. Filesystem path guards: middleware/static's stack in order:
bytesToPathStringfolds literal\to/before decoding, so the later backslash reject only fires on%5C-revealed ones; fixpointurl.PathUnescapeloop (double-encoding defense on an already-decoded input); segment-wise..scan; NUL reject; Windows volume/UNC checks;fs.ValidPath; the/__fiber_invalid__sentinel. res.go SendFile is smaller:path.Cleanonly in the fs.FS branch ofsendFileContentLength, the os.Stat branch runsfilepath.FromSlashwith no clean. Percent-decode currently has four dialects in the tree (static's fixpoint loop, middleware/redirect's lenientpercentDecode, extractors' stricturl.PathUnescape, client/cookiejar'sescapePercenton the encode side); the shared guards should settle the stray-%policy. **Target:** shared guard helpers plus a layer table documenting which check exists because of which fasthttp behavior (the backslash guard stays because\is not a separator on Unix). - 13. Client URL building: fix-first item 2 covers the substitution bug; the query side already delegates to fasthttp.Args and stays. Document (or align, see cluster 4) the philosophy split: the client rejects CTL bytes in upstream Locations while the server-side composer strips them. Related, same shape: middleware/paginate
encodeQueryValuesre-implementsurl.Values.Encodeallocation-free on top of the sharedutils.AppendQueryEscape; fold or document. Target: anchored, escaped substitution; one documented client/server policy split.
Acceptance
- Each cluster ends with exactly one implementation; the duplicates are deleted, not deprecated.
- No value is sanitized/normalized/escaped twice on any request or response path; every call site that relies on a fasthttp guarantee carries a comment naming it.
- Outputs stay byte-identical unless a divergence was a bug; behavior changes are listed per PR.
- The three fix-first items land as standalone PRs ahead of the clusters.
Non-issue, noted to prevent re-filing: the append on the package-level weakPrefix in middleware/etag/etag.go GenerateWeak looks racy but is not; a static []byte("W/") literal is initialized with cap == len, so every append reallocates.
Source: gofiber/fiber