The consumed-stream retry guard treated any read of a streaming request
body as consumption. Go's transport probes unknown-length bodies of GET,
HEAD, and similar requests before sending them, and HTTP/3 reports
bodyless GET and HEAD requests with an unknown length, so every such
request lost its retries once an upstream connection was established.
A first read that returns io.EOF without data no longer counts as
consumption. Later attempts send http.NoBody, while bodies that yield
data, fail, or are interrupted mid-read remain non-retryable.
Follow-up to de1bcf6d-457c-4348-9d4e-5eeb41cc120b.
Code written by GitHub Copilot (GPT-6.1 Sol, Claude Opus 5.5); reviewed by mholt.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Follow up report dff6e149-9cd3-47fb-8e21-b4d3d4bad807: make AST handling exhaustive and preallocate map entries. Verify regex and URL-pattern factories do not expand runtime env/file placeholders while preserving their existing pattern grammar and static behavior.
Code written by GitHub Copilot (GPT-6.1 Sol, Claude Opus 5.5); reviewed by mholt.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Fix report dff6e149-9cd3-47fb-8e21-b4d3d4bad807 by retaining string provenance through CEL matcher factories. Runtime strings must not become templates or globs on a second pass; static leaves retain their existing matcher syntax and caching.
Code written by GitHub Copilot (GPT-6.1 Sol, Claude Opus 5.5); reviewed by mholt.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Reject opaque HTTP request targets before either normal or error routes can interpret them. Clear URL.Opaque defensively on proxy clones. Verify HTTP/1 parsing, path authentication, supported target forms and outbound serialization without contacting an upstream.
Addresses scan report e17d2fcd-d50c-4d68-82d1-96519a437a24.
Code written by GitHub Copilot (GPT-6.1 Sol, Claude Opus 5.5); reviewed by mholt.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Follow up report de1bcf6d-457c-4348-9d4e-5eeb41cc120b. Expect one transport attempt for consumed partial streams while keeping unread dial retries and fully buffered trailer replay covered.
Validated the entire reverseproxy package with offline race-short tests in a loopback-only network namespace.
Code written by GitHub Copilot (GPT-6.1 Sol, Claude Opus 5.5); reviewed by mholt.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Fix report de1bcf6d-457c-4348-9d4e-5eeb41cc120b. Release pooled buffers exactly once, serialize reads with release, and retain safe replay cursors for fully buffered requests. Retire each stream attempt so consumed bodies cannot be retried while unread dial failures remain retryable.
Add deterministic concurrent ownership checks, boundary coverage, local transport failure cases, and synthetic canary tests.
Code written by GitHub Copilot (GPT-6.1 Sol, Claude Opus 5.5); reviewed by mholt.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Apply the existing header-alias rules before method and host validation, so early errors cannot carry spoofed identity aliases into user-defined error routes. Verify all early error paths with synthetic authentication headers.
Addresses scan report 3ebe3dfa-8541-44a7-8f1f-cd5071d55a67. The supplied checker has a malformed Authorization header; regression tests use independently constructed requests.
Code written by GitHub Copilot (GPT-6.1 Sol, Claude Opus 5.5); reviewed by mholt.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Canonicalize the path before capturing and substituting its components, not only afterward. Retain repeated slashes, query strings and valid alternate encodings; cover the supplied depth-changing traversal and encoded dot segments.
Addresses scan report 34013c71-0786-4179-8403-eacc143a1d89 (marked duplicate, but not fixed by the prior post-replacement safeguard).
Code written by GitHub Copilot (GPT-6.1 Sol, Claude Opus 5.5); reviewed by mholt.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Use filepath.Rel rather than prefix trimming for both existing and fallback candidates. Preserve dotfiles and directory suffixes, and always expose a slash-prefixed path so rewrites and downstream path guards agree.
Addresses scan report 3188624e-2157-49e6-b3e0-8c5049b72269.
Code written by GitHub Copilot (GPT-6.1 Sol, Claude Opus 5.5); reviewed by mholt.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Lowercasing and slash-preserving path cleaning rewrote invalid UTF-8
bytes as U+FFFD, so the path matcher could evaluate a different path
than the file server opens. Preserve the original bytes in both, keeping
case-insensitive matching for valid Unicode.
Addresses scan report 18c02f60a5b43529.
Code written by GitHub Copilot (GPT-6.1 Sol, Claude Opus 5.5); reviewed by mholt.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
When an escape sequence was followed by a wildcard in the pattern, the
path cursor had already skipped to the last hex digit, so an encoded
slash could be swallowed into the wildcard span. Advance the cursor by
the escape width only when the character is consumed.
Addresses scan report 0e63a0fe7fea5ef7.
Code written by GitHub Copilot (GPT-6.1 Sol, Claude Opus 5.5); reviewed by mholt.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Decoded path placeholders (orig_uri.path, path components and file
matcher values) were inserted raw into the rewritten URI, so an
encoded '%' or '?' could be decoded a second time. Escape all decoded
path placeholders in path context in a single replacement pass; escaped
and whole-URI placeholders keep their URI syntax.
Addresses scan report 9e3a082391f793d1.
Code written by GitHub Copilot (GPT-6.1 Sol, Claude Opus 5.5); reviewed by mholt.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Like prefix/suffix stripping, substring and regexp path replacement can
produce a relative path or expose dot segments that later path-scoped
handlers interpret differently. Apply the existing canonicalizePath
safeguard whenever these operations change the path.
Addresses scan report 65568b0d767b733c.
Code written by GitHub Copilot (GPT-6.1 Sol, Claude Opus 5.5); reviewed by mholt.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
With per_host metrics, HTTPS servers automatically observed catch-all
hosts, and port/case variants of configured hosts produced distinct
labels. Require explicit observe_catchall_hosts for unconfigured hosts,
normalize the host label, and also collect hosts from subroutes, error
routes and named routes (scanned after handlers are provisioned).
Behavior change: unconfigured hosts on HTTPS servers are now labeled
"_other" unless observe_catchall_hosts is enabled.
Addresses scan report 457dcf090b421495.
Code written by GitHub Copilot (GPT-6.1 Sol, Claude Opus 5.5); reviewed by mholt.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
A placeholder-dependent search_regexp was recompiled for every matching
header value. Compile it once per replacement operation, keep skipping
invalid expanded expressions, and merge the duplicated "*" code path.
Addresses scan report 6c26a7b4c2e5ddc3.
Code written by GitHub Copilot (GPT-6.1 Sol, Claude Opus 5.5); reviewed by mholt.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Failed comparisons for missing accounts all used the same fake hash, so
their cached results returned quickly while failures for real accounts
did not, leaking which usernames exist. Cache only successful
comparisons and scope cache keys to the username.
Addresses scan report 430e037dbf6a4fd3.
Code written by GitHub Copilot (GPT-6.1 Sol, Claude Opus 5.5); reviewed by mholt.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
IdleDeadline.next computed the min_rate allowance as
transferred * time.Second / MinRate, which overflows int64 once a
request body or response passes about 9.2 GB. The credit goes negative,
the deadline lands in the past, and the connection is cut even though
the client is far above the configured rate.
Compute the credit with a 128-bit intermediate and saturate instead.
FileServer.Provision hides the data directory when it is relative (#8327),
because a relative data directory sits under the working directory and
holds private keys. The config directory falls back to the same relative
"./caddy" when no config home can be resolved, and it holds autosave.json,
which is the loaded configuration.
The two directories are resolved independently, so an absolute data
directory did not cover a relative config directory: with XDG_DATA_HOME set
and no HOME or XDG_CONFIG_HOME, the data directory is absolute while the
config directory is "./caddy" under the served root.
Both directories are now hidden through the existing fallback, each entry
added only once.
Refs #8381
* fileserver: apply slash rules to fallback entry
The literal candidate built for the final entry of first_exist_fallback cleaned the path without the rules of #8326 and #8352. With "{path}" as that entry, "/.env/" and "/.env/./" matched the regular file "/.env", bypassing path matchers that see the trailing slash.
Both candidate builders now share the same cleaning and splitting.
* fileserver: handle Windows separators in file matcher
php_fastcgi used first_exist_fallback, which since v2.11.8 matches its final entry even when the file is absent. On a site without index.php, "/", directories with an index.html and missing files were all rewritten to the missing index.php, so file_server never served them.
php_fastcgi now uses first_exist, as v2.11.7 effectively did since globbing skipped absent files. Explicitly configured first_exist_fallback keeps its semantics. The transport normalizes the path itself, so the dot-segment protection doesn't depend on this rewrite.
Fixes#8367
The transport split the path at the first split extension anywhere, so "/admin.php.php" and "/admin.phpx/y.php" ran "/admin.php" with the rest as PATH_INFO. Route matchers judged a different file: "path /admin.php /admin.php/*" didn't match, and with php_fastcgi the file matcher rewrote "/a.phpx/b.php" to that file while PHP ran "/a.php". With "index off", or try_files not ending in a PHP file, nothing rewrites the request, so the protected script runs.
A match must now end the path or be followed by "/", as in the file matcher.
On case-insensitive filesystems (APFS and NTFS by default), a request using another letter case opens a hidden file anyway: the Caddyfile auto-hidden by httpcaddyfile is served for /caddyfile.
Entries without glob characters now match regardless of letter case. Glob patterns keep their case-sensitive matching. On case-sensitive filesystems, a literal entry also hides names that differ from it only by case.
wrapperFs embeds fs.FS, which hides the StatFS, ReadDirFS and
ReadFileFS methods of the wrapped filesystem. fs.Stat on
DefaultFileSystem did open, fstat and close instead of one stat,
for every fs.Stat in the file matcher and file_server, including the
literal-path case of fs.Glob.
GlobFS is not forwarded: OsFS.Glob uses filepath.Glob, which differs
from fs.Glob (Lstat for literal patterns, OS path separators).
Stat now succeeds on files and directories that cannot be opened, and
no longer blocks on FIFOs. The file matcher matches unreadable files
instead of skipping them, file_server picks an unreadable index file
(403) instead of trying the next one, and browse lists symlinks to
unreadable directories as directories.
BenchmarkFileMatcher, Apple M5 Pro, macOS, count=10:
before after delta
existing file 18.90µs 1.570µs -91.70%
front controller 20.85µs 3.040µs -85.42%
existing file 936 B 728 B 22 -> 18 allocs
front controller 1312 B 1520 B 35 -> 33 allocs
* fileserver: don't serve regular file when path ends by slash
With canonical URIs disabled, file_server trimmed the trailing slash before
stat and served the file, while path matchers see "/secret.env/" and skip
rules such as "path *.env". Return 404 (or pass through) instead.
* fileserver: trim trailing OS separator before stat
On Windows, SanitizedPathJoin appends a backslash for a path ending in "/",
which the "/" trim left in place. Stat then failed on a regular file, so
"/file/" returned 400 instead of the canonical redirect or a 404.
* fileserver: keep trailing separator of a filesystem root
Trimming it turned a root of `C:\` into `C:`, the current directory of the
drive, and a root of `\` or `/` into an empty path, so requests for "/"
listed or indexed the wrong directory, or failed.
* fileserver: log request path on trailing slash 404
The logged filename has its trailing separator trimmed, so it didn't show the slash that caused the 404.
* fileserver: check trailing slash on matcher path
Closing one side of a tunnel ends its copy loops, which close the other
side. closeConnectionsLocked sent each close message and closed that
connection before moving on, in map order, so when the client side came
first the upstream was often closed before its close message went out.
Send every close message in the batch first, then close the connections.
The second close then regularly finds the connection already closed.
Treat net.ErrClosed as expected instead of returning it, so reloads with
open websockets no longer log "use of closed network connection" at
ERROR level.
The upstream close message test goes back to running the real cleanup,
which it can now do reliably.
* reverseproxy: set a write deadline on websocket close messages
Cleanup sends a Close control message to every attached websocket while
holding the tunnel tracker lock, during a config reload. If the peer has
stopped reading, the tunnel's copy goroutine is blocked writing to it and
the close message waits behind that write forever. The reload never
finishes and holds the config lock, so every later /load and /config
request hangs until Caddy is restarted.
Setting a write deadline before the close message unblocks both writes.
The connection is closed right after, so the deadline is never cleared.
* Add bot disclosure
* reverseproxy: make close control timeout a const, harden stalled-peer test
* Delete BOT_DISCLOSURE.md
Signed-off-by: Francis Lavoie <lavofr@gmail.com>
* reverseproxy: bound each batch of websocket close messages by one deadline
The close loops are serial and hold the tunnel tracker lock, so a
per-connection write timeout let N stalled peers hold a reload for N
times the timeout. Compute one absolute deadline per batch instead,
and skip the best-effort close frame on connections that cannot set a
write deadline rather than risk the hang.
Extended CONNECT streams gain SetWriteDeadline through
http.ResponseController. For HTTP/1 upgrades, net/http returns the
upstream socket inside a body that hides its deadline methods, so the
upstream close message was never bounded; record the connection from
the request's ClientTrace and write the close message to that socket.
Restores the TODO about being the exclusive writer, per review.
* reverseproxy: make websocket close tests deterministic
Per review, the stalled-peer tests use net.Pipe instead of real TCP:
it is unbuffered and lets one write through at a time, so a close
message waits behind a stuck copy write without large allocations or
sleeps. The batch test checks that graceful closes share one deadline
instead of timing cleanup.
The HTTP/1 upstream close test now runs a real upgrade through
ServeHTTP and the transport, so it fails if the GotConn hook is
removed. The choice of upstream close writer moves into
upstreamCloseWriter, with a table test for the HTTP/2 and HTTP/3 case.
* reverseproxy: record the upstream conn with atomic.Pointer
Per review, store the connection from the GotConn hook in an
atomic.Pointer[net.Conn] instead of a mutex-guarded holder type.
Rename upstreamCloseWriter to upstreamWriteCloser. It returns
io.ReadWriteCloser, since both the body and the recorded socket are
one, so gracefulClose keeps its original io.ReadWriteCloser parameter.
* reverseproxy: share one close deadline across detached trackers
Per review, an upstream removal notifies every stopped tracker in
turn, and each close loop holds its own lock, so a deadline per
tracker let N trackers with stalled peers hold a reload for N times
the timeout. Compute one absolute deadline in
notifyDetachedTunnelTrackersOfUpstreamRemoval and pass it through to
closeConnectionsForUpstream.
Add a test with two stopped trackers holding stalled net.Pipe peers
that asserts both graceful closes receive the same deadline.
* reverseproxy: simplify the websocket close deadline change
Fold the two tunnel close loops into closeConnectionsLocked, so the
shared-deadline rule and its rationale live in one place. Replace the
per-tracker closeWriteTimeout field, which production never varied,
with a package variable that tests shorten.
Record the upstream socket only for websocket requests, since only
their graceful close reads it, and store a copy of the connection
rather than the address of the hook's parameter. Use the package's
caddy.CtxKey convention for the context key.
Share the upgrade tunnel fixture between the half-close test and the
upstream close message test.
When downgrading an HTTP/2 or HTTP/3 extended CONNECT WebSocket request
to HTTP/1.1, delete any parsed client-supplied Sec-WebSocket-Key header
prior to installing Caddy's generated 16-byte random key. This prevents
request header operations from canonicalizing and combining both keys
into duplicate Sec-WebSocket-Key headers upstream.
Signed-off-by: duongynhi000005-oss <duongynhi000005@gmail.com>
* 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>
* 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.
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.
Matthew Holt
Jae Birdsall
PerryLink
Zen Dodd
Kévin Dunglas
zjubiology
Chris Oliver
Suganraj
duongynhi000005-oss
Julien Voisin
Steffen Busch
D'Ali Ben Macha
Venkatesh Panda