* caddyhttp: presize buffers in hot paths to reduce allocations
Right-size three per-request allocations that previously grew from a
nil/empty backing array, causing repeated reallocation:
- CleanPath: pre-Grow the strings.Builder used when slash collapsing is
disabled, instead of letting it reallocate as runes are appended
- headers: presize the rewritten value slice in HeaderOps.ApplyTo's Set
loop instead of appending from nil
- reverseproxy: presize the weights slice in WeightedRoundRobinSelection
.Select
This commit also adds benchmarks exercising each path. Measured with benchstat
with count=20 and GOMAXPROCS=1, with p=0.000:
sec/op B/op allocs/op
CleanPathNoCollapse 1013.4n -> 828.6n (-18.23%) 120->64 (-46.67%) 4->1 (-75.00%)
HeaderOpsApplyToSet 1262.0n -> 955.8n (-24.27%) 255->152 (-40.39%) 9->6 (-33.33%)
WeightedRoundRobinSelect 215.8n -> 200.3n ( -7.20%) 112->96 (-14.29%) 2->2 (N/A)
* caddyhttp: presize strings.Builder in query rewrite and path matcher
Two builders whose output size is bounded by a known input length were
growing from empty, causing repeated reallocation. Seed each with a
capacity hint:
- rewrite.buildQueryString: pre-Grow to len(qs) (runs per request when
rewrite query manipulation is configured)
- MatchPath.matchPatternWithEscapeSequence: pre-Grow to len(escapedPath)
(reached only when a path pattern contains % escape sequences)
Adds a benchmark for buildQueryString, measured with benchstat:
BuildQueryString:
- sec/op 965.2n -> 743.3n (-22.99%)
- B/op 248->80 (-67.74%)
- allocs/op 5->1 (-80.00%)
The builder previously reallocated four times per call. The matcher path
is cold and not separately benchmarked.
* chore: add benchmark headers
---------
Signed-off-by: Julien Voisin <jvoisin@users.noreply.github.com>
* httpcaddyfile: reject conflicting client_auth when merging TLS policies
* httpcaddyfile: use host and host:port site addresses in client auth conflict test
* httpcaddyfile: enhance client auth consolidation test to handle conflicting configurations
* httpcaddyfile: remove unnecessary blank line in conflict error handling
* caddyhttp: lower-case path patterns with placeholders at request time
Patterns with placeholders were lower-cased at provisioning, which
changed case-sensitive placeholder names such as {env.FOO}, while the
replaced values were never lower-cased and so could not match the
lower-cased request path. Leave such patterns untouched at provisioning
and lower-case the expanded pattern at request time.
Fixes#6387
* caddyhttp: restore the encoded separator path matcher tests
* reverseproxy: reduce lock contention in dynamic SRV and A upstreams (#7946)
The shared SRV and A/AAAA caches held a global write lock across the DNS
lookup, so one slow or timing-out record blocked every unrelated
upstream source.
Lookups now run without the cache lock held, deduplicated per key with
singleflight. A per-key generation (stored on the cache entry, so it is
bounded by the cache cap) and a cache-wide epoch keep a lookup that was
in flight during ResetSRV/ResetAllSRV from writing its result back, and
are part of the singleflight key so callers after a reset do not join a
pre-reset lookup.
The shared lookup is detached from the request that started it and
bounded by its own 30s timeout; each caller still returns as soon as its
own context is done. Distinct lookups across both caches are capped by a
shared gate (64 running, 256 queued); beyond that they are shed, serving
a stale SRV entry if a grace period is set and one is cached.
SRV and A now share one cache implementation. AUpstreams also implements
CachingUpstreamSource (via new ResetA/ResetAllA), so retries purge its
cache entry like SRVUpstreams already did.
Rebased onto the move of this code into modules/internal/network (#7790,
#8252).
Fixes#7946
Co-authored-by: Francis Lavoie <lavofr@gmail.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* network: backdate cache entries in TestShedLookups instead of a 1ns refresh
On Windows two close time.Now() calls can report no elapsed time, so an
entry seeded with a 1ns refresh could still be fresh and skip the shed
path entirely.
* network: move the shared cache and lookup gate into cache.go
dynamicupstreams.go keeps the SRV/A API; the generic cache, its
singleflight/generation handling and the lookup gate are not specific to
either record type. No functional change.
* network: preserve cache reset and grace semantics
---------
Co-authored-by: Francis Lavoie <lavofr@gmail.com>
Co-authored-by: steadytao <mail@steadytao.com>
HTTPBasicAuth.correctPassword built its hash cache key with
append(account.password, plaintextPassword...). account.password is
shared by all requests and can have spare capacity, so short passwords
were written into the stored hash's backing array, causing a data race
between concurrent requests and potentially mixed-up cache keys.
Build the key in a fresh buffer instead, and add regression tests.
* requestbody: reject oversized Content-Length before the next handler
When max_size is set and the request declares a Content-Length above it, respond 413 right away instead of letting the next handler start and fail mid-read. Bodies of unknown length are still limited by MaxBytesReader. Both paths return the same error, so handle_errors sees no difference.
* caddytest: send unknown-length bodies in max_size placeholder tests
request_body now answers 413 for a declared Content-Length over max_size before the placeholder reads the body, so these tests no longer covered the read path and the direct respond one failed.
* requestbody: keep the body limited for error routes
Error routes get the same request, so wrap the body before the early 413 to stop them from reading past max_size.
{args[-1]} and {args.-1} parsed as -1 and indexed the args slice
directly, panicking with index out of range. Treat negative indices as
out of bounds, like indices past the end.
* admin: authorize remote requests matching any permission entry
enforceAccessControls rejected a request as soon as one permission entry
did not match its method or path, so only the first entry was usable.
Allow the request if any entry permits both, and deny only after all fail.
* admin: check every access entry that holds the client key
A key listed in several access control entries was only checked
against the first one; deny only after all of them are tried.
Use the chosen file info, not a zero size, to tell whether a candidate
was chosen, so smallest_size keeps an empty file and largest_size can
choose one.
When a handle_response status code did not evaluate to an integer, the
handler returned a 500 without closing the upstream response body,
leaking the connection.
* caddyauth: Support third-party http.authentication.hashes modules
basic_auth and hash-password only accepted the built-in bcrypt and
argon2id algorithms, even though the http.authentication.hashes
namespace is documented as extensible. Look up any other algorithm
name among the installed hash modules instead, and list the available
ones when the name is not recognized.
The built-in algorithms keep their own hash-password flags; other
modules are provisioned and must implement Hasher to generate hashes.
Closes#6670
* caddyauth: Base64-encode non-MCF hashes from hash-password modules
hash-password printed module hashes as raw bytes, but basic_auth
base64-decodes any password that isn't in Modular Crypt Format, so the
output couldn't be used as-is. Encode non-MCF hashes as base64 and add
a round-trip test that provisions HTTPBasicAuth with the command output.
* caddyauth: always base64-encode third-party hash output
* reverseproxy: Test that WebSockets + unix-sockets works
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* Close temp file before reusing its path for the socket
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
* Strengthen unix socket WebSocket test, reuse tempSocketName helper
Check that upstream bytes sent with the 101 are delivered, echo a payload larger than any buffer, and verify the upstream connection closes when the client disconnects. Reuse the existing tempSocketName helper for all unix socket tests, which also closes the temp file descriptor the older tests leaked.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
* caddyhttp: let handler read deadlines cap the idle read timeout
IdleTimeoutReader sets the read deadline before every body read, so a
deadline set by a handler with http.ResponseController.SetReadDeadline()
was replaced before it could take effect. Since #7913, handlers can't
bound a stalled request body more tightly than read_body.
A handler's read deadline is now routed to the IdleTimeoutReader, which
applies it right away and keeps it as a ceiling for every idle reset and
for the deadline put back between HTTP/2 reads. A handler can make the
idle timeout stricter but can't loosen it, and a zero deadline removes
the ceiling, as #8145 does for write deadlines.
* caddyhttp: keep the hard read deadline and the drain deadline with handler deadlines
SetReadDeadline installed the handler's deadline as is, so a later or zero
deadline lifted read_timeout. It now installs the earlier of the two.
HandlerDone cleared a deadline set by the handler before arming the HTTP/1
drain deadline, leaving the post-handler drain unbounded.
* caddyhttp: keep the idle deadline of a read in flight on handler deadlines
SetReadDeadline replaced the idle deadline armed for a blocked read with the
handler's ceiling, so a zero or later deadline lifted the idle timeout for
that read. It now installs the earlier of the two until the read returns.
* caddyhttp: keep the expired drain deadline of a timed-out body on handler deadlines
After a body read timed out on HTTP/1, a zero or later deadline set by the
handler replaced the expired one kept to bound net/http's post-handler drain.
* perf: bypass header key normalization for canonical literals
http.Header's Get, Set, Add and Del run textproto.CanonicalMIMEHeaderKey
on every call. When the key is a canonical literal the result is the key
itself, so the scan is pure overhead. Access the map directly instead.
Only hot, allocation-free paths are converted, and only where the key is
a canonical literal. Call sites taking a key from config, a template or
the request itself still go through the normalizing methods, since they
need it for correctness.
allHeaderValues and lastHeaderValue dropped their http.CanonicalHeaderKey
call for the same reason: all callers pass canonical literals.
benchstat, Apple M5 Pro, go1.27.1:
PrepareRequest-15 659.4n ± 2% -> 457.7n ± 2% -30.58% (p=0.000 n=12)
AcceptedEncodings-15 330.8n ± 5% -> 309.4n ± 7% -6.44% (p=0.019 n=10)
ResponseWriterWriteHeader/event-stream-15 1007.5n ± 3% -> 732.5n ± 1% -27.30%
ResponseWriterWriteHeader/informational-15 60.0n ± 3% -> 48.0n ± 2% -20.01%
ResponseWriterWriteHeader/not-modified-15 214.3n ± 6% -> 175.3n ± 4% -18.20%
ResponseWriterWriteHeader/connect-15 60.2n ± 3% -> 51.9n ± 4% -13.85%
ResponseWriterWriteHeader/html-15 206.5n ± 4% -> 197.9n ± 2% -4.19%
geomean -14.38%
Allocations are unchanged.
* tests: verify http.Header map keys are canonical
Indexing http.Header directly skips the canonicalization Get/Set/Add/Del
perform, which also puts the key outside what CI checks. canonicalheader
only inspects method calls. staticcheck's SA1008 covers reads, but stops
descending as soon as an assignment writes to a header, so it misses the
write, the delete builtin and the read-modify-write append. A key that is
not canonical in those positions is a silent lookup miss, not a compile
error.
Walk the loaded packages and verify every string constant used as a key in
an http.Header map operation matches http.CanonicalHeaderKey. A helper that
takes the key from its caller would hide the literal again, so a parameter
named with a canonical prefix opts its arguments into the same check.
The WebSocket headers Caddy deliberately writes with RFC 6455 casing are
allowed explicitly; they are only ever written out, never looked up against
a parsed header map.
Covers 64 operations, runs in under a second, and promotes golang.org/x/tools
from an indirect to a direct dependency at the version already in the module
graph.
* headercheck: resolve aliases of http.Header
* headercheck: check http.Header composite literal keys
* headercheck: note overlap with canonicalheader
* headercheck: drop parameter-name check
Helpers now take the indexed values, so every key sits in an index expression the analyzer and the linter already check.
* headercheck: scope RFC 6455 exceptions to their call sites
A key-based allow-list also accepted reads such as h["Sec-WebSocket-Key"], which miss because parsed headers are canonical. Honor //nolint:canonicalheader on the line instead, which the linter will use too.
* encode: read every Accept-Encoding field line
AcceptedEncodings read only the first field line, so a client that split
the list across several lines had the rest ignored. RFC 9110 section 5.3
treats repeated list-field lines as one comma-joined list. Iterate over
the items of every line instead of joining them, and skip empty items,
which used to yield an empty encoding. file_server's precompressed
selection uses the same function and is fixed with it.
Fixes https://github.com/caddyserver/caddy/issues/8272
Co-authored-by: Kağan Küçük <kaankck@gmail.com>
* encode: read every Cache-Control and Content-Encoding field line
Only the first field line was read, but RFC 9110 section 5.3 makes
multiple lines of a list-based field equivalent to one comma-joined
value:
- Cache-Control: no-transform on a later line was ignored, so the
request or response was compressed anyway.
- Content-Encoding: an empty first line followed by a real coding let
the handler encode the body a second time.
* encode: parse Cache-Control no-transform per RFC 9111
The substring match missed No-Transform, though RFC 9111 section 5.2 compares directive names case-insensitively, and fired on text that is not the directive: longer tokens such as x-no-transform-ext, quoted arguments such as community="no-transform", and semicolon-separated values, which are not valid Cache-Control syntax. Split the field on commas outside quoted strings and match only a well-formed no-transform element.
* encode: keep accepted encoding preferences on the stack
AcceptedEncodings counted commas to size the preference slice, and the variable capacity put it on the heap. A constant capacity covers typical Accept-Encoding headers and lets the compiler keep the slice on the stack; append still moves it to the heap for longer ones. BenchmarkAcceptedEncodings: 151.8ns -> 120.5ns (-21%), 192 -> 64 B/op, 2 -> 1 allocs/op (n=8).
* encode: build the encoded ETag without fmt
fmt.Sprintf boxed both arguments and parsed the format on every encoded response with a strong ETag. Concatenation: 47ns -> 16.5ns, 56 -> 24 B, 3 -> 1 allocs.
* reverseproxy: access response Server and Via headers directly
finalizeResponse runs on every proxied response; skip the key normalization as prepareRequest already does on the request side.
* encode: access headers directly in Write, WriteHeader and Close
These run once per response; convert the remaining literal-key Get, Set and Add calls like the rest of the handler.
* encode: match Cache-Control no-transform with httpguts
httpguts.HeaderValuesContainsToken already compares list elements case-insensitively across field lines, so the hand-written list parser is unnecessary. A no-transform inside a comma-separated quoted argument now also disables encoding, which only errs toward not transforming.
* encode: inline the no-transform check
isEncodeAllowed only wrapped one httpguts call. Its unit test now drives the response directive through ServeHTTP instead.
---------
Co-authored-by: Kağan Küçük <kaankck@gmail.com>
* caddytls: keep ACME issuer network proxy across re-provisioning
LoadModule clears NetworkProxyRaw, so when the HTTP app provisions an
ACME issuer a second time (fillInACMEIssuer), the rebuilt template has
no HTTPProxy. Since #7510 that template is used via SetConfig, and ACME
requests bypass the configured proxy.
Keep the loaded proxy func on the issuer, like the DNS solver.
Fixes#8283
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
(cherry picked from commit 2366825f2380383dacd8adeb1508e34c3e25634d)
* caddytls: test that ACME requests use the proxy after RebuildCertMagic
Replace the provision-twice check with a regression for the path the
HTTP app takes: Provision, re-provision in fillInACMEIssuer, then
RebuildCertMagic. The test issues against a local proxy and asserts the
ACME request goes through it.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
---------
Co-authored-by: k4lleanka <257361691+k4lleanka@users.noreply.github.com>
Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
Kévin Dunglas
Julien Voisin
Zen Dodd
Steffen Busch
dependabot[bot]
sam-kota
D'Ali Ben Macha
Venkatesh Panda
Nguyen Thanh Dat
Krish Singh Huraandsteadytao
EINIER FREYRE CORONA
a
Kartik K
hsdfat
jasper
tannevaledandZen Dodd
bwisn
Akinyemi Toluwalase