Brings the package to 96.8% statement coverage. Adds:
- TestResetAllSRV: full-cache drop
- TestSRVFilteredRecords: LookupSRV partial-error semantics (usable
records returned, error downgraded to a warning)
- TestSRVCacheBound / TestACacheBound: eviction keeps the cache bounded
at maxCacheEntries
- TestSRVConcurrentRefreshDeduplicates / TestAConcurrentRefreshDeduplicates:
two goroutines missing the read-lock fast path trigger only one lookup
(verified under -race)
The grace-period and filtered-records logs now use a level-enabled logger
so the log-write branches execute. The only uncovered statements are the
two double-checked-locking re-check returns, which are reachable solely in
a narrow race window (the same untested idiom as reverseproxy's SRV/A).
Move the SRV and A/AAAA resolution + caching logic out of reverseproxy
into a new, transport-neutral package (dynamicupstreams) that returns
neutral targets instead of *reverseproxy.Upstream. reverseproxy's
SRVUpstreams and AUpstreams now delegate to it and build their own
upstreams from the targets; behavior is unchanged.
This lets other proxies (e.g. third-party layer4 proxies) reuse the same
DNS discovery + caching instead of copying it, reducing duplication and
maintenance burden. reverseproxy/upstreams.go shrinks substantially.
RFC for the de-duplication discussed in caddyserver/caddy-l4#429.
* Patch GHSA-vcc4-2c75-vc9v in stripHTML
templates: fix funcStripHTML bypass via depth counter
The previous false-start approach allowed XSS bypass via inputs like <<>img src=x onerror=alert(1)> and failed on stacked angle brackets.
Replace the tagStart/inTag state machine with a depth counter that mirrors PHP strip_tags behaviour: each '<' increments depth, each '>' decrements it, and text is only emitted at depth zero. Quoted attribute values (both single and double) are tracked so '>' inside href values does not prematurely close a tag.
Signed-off-by: JM Sanchez <77505889+jmrcsnchz@users.noreply.github.com>
* Update tplcontext_test.go
Templates: expand TestStripHTML with attack path coverage
Signed-off-by: JM Sanchez <77505889+jmrcsnchz@users.noreply.github.com>
---------
Signed-off-by: JM Sanchez <77505889+jmrcsnchz@users.noreply.github.com>
Parse each matcher segment individually using NewDispenser(segment) instead
of DispenseDirective(dir), which coalesced all same-name segments into one
token stream. This caused the second definition name to be misinterpreted
as a matcher module name, producing 'module not registered: http.matchers.@name'
instead of the correct 'matcher is defined more than once' error.
By parsing segments individually, the existing duplicate check in
parseMatcherDefinitions naturally catches the duplicate on the second pass.
Signed-off-by: Brunotlps <brunoteixlps@gmail.com>
* feat: drop headers with underscore in their names
* feat: Caddyfile binding and tests for underscore-in-header drop
Add the `allow_underscore_in_headers` global server option, refine the
doc comment, and cover the filter end-to-end: server-level unit tests
(drop, opt-out, debug log, RFC-7230 space rejection), a fastcgi unit
test for the trimmed header name replacer, and forward_auth integration
tests for both the default-drop and opt-out paths.
* remove allow_underscore_in_headers option for now
* fix(encode): prioritize zstd and br over gzip in content negotiation
* test(encode): update unit tests to reflect new default priority ties
* fix(encode): move default preferences to dynamic encode handler and restore generic negotiation helper
* test(encode): call real Provision function in served-response test
* test(encode): rename served-response test to TestServeHTTPDefaultEncodingPreference
* refactor(encode): use slices.SortStableFunc and httptest.NewRecorder as recommended
* refactor(encode): simplify sorting with cmp.Compare and check request error in test
* test(encode): fix variable redeclaration in TestServeHTTPDefaultEncodingPreference
Fix 'no new variables on left side of :=' error by changing 'err :=' to 'err ='
on line 347, since err was already declared on line 332.
This fixes the build failure in the encode module tests.
SNI is always ASCII on the wire (RFC 6066), and most config
patterns are also ASCII. For pure ASCII input, idna.ToASCII
only validates and lowercases, which is equivalent to a simple
strings.ToLower. Add a fast path to avoid the overhead of
idna.ToASCII in the common case.
When the rewrite URI template ends with a literal '?' and contains a placeholder that expands to client-controlled bytes (e.g. {http.request.header.X-Fwd}), those bytes flow into buildQueryString which runs a second Replacer pass. If the bytes contain placeholder syntax such as {env.SECRET}, that placeholder is evaluated, allowing disclosure of environment variables, files (via {file./path}), or internal request vars through the rewritten request URI.
Escape '{' and '}' in the injected query before assigning it to the query variable, so the second pass cannot find any placeholder syntax to evaluate. Operator-written placeholders in the rewrite template are already expanded by the first pass on the path component, so the only '{' or '}' surviving into the injected query must have come from replacement values.
Fixes GHSA-j8px-rmrx-76h9.
Includes three regression tests mirroring the 'is not re-expanded' tests in modules/caddyhttp/vars_test.go.
Co-authored-by: Matt Holt <mholt@users.noreply.github.com>
* caddytls: fix data race in session ticket key rotation
stayUpdated copies the map header (configs := s.configs) under the
lock, then iterates the original map after releasing it. Concurrent
calls to register/unregister mutate the same map.
Hold the lock for the entire iteration instead.
* caddytls: fix data race in AllMatchingCertificates
AllMatchingCertificates reads the package-level certCache without
acquiring certCacheMu, while Cleanup sets certCache to nil under
the write lock. The adjacent HasCertificateForSubject correctly
acquires certCacheMu.RLock.
Add the missing RLock/RUnlock to match.
* caddytls: fix ECH key rotation stopping permanently on error
When rotateECHKeys returns an error, the rotation goroutine returns
immediately, stopping all future key rotation for the lifetime of
the process.
Change return to continue, matching the error handling for
publishECHConfigs two lines below.
* metrics: Add nil check for metricsHandler in AdminMetrics.serveHTTP
Prevents panic when the admin metrics endpoint is accessed before
the module is fully provisioned. Returns a proper API error instead
of crashing.
* admin: provision router modules before registering routes
Instead of adding a nil check for metricsHandler, address the root
cause by provisioning admin router modules before calling Routes().
This ensures all handler state is initialized before routes are
registered on the mux.
Merge newAdminHandler and provisionAdminRouters into a single step,
removing the two-phase setup where routes were registered first and
modules provisioned later. The AdminConfig.routers field is no longer
needed since provisioning happens inline.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
* fix: go fmt admin.go
---------
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
* caddytls: Fix wildcard race in auto-HTTPS launch
When evaluating whether to skip managing an individual subdomain
due to an existing wildcard configuration, we now explicitly consult
the automate loader.
Because Caddy apps can start in any order, relying strictly on the
TLS app's internal management state was non-deterministic if the
HTTP app started first. Checking the automate loader guarantees
predictable behavior since it is fully populated during the
Provision phase, well before any apps are started.
* respond to review comments
1. update requested comment
2. remove personal domain from test
3. add regression test
* remove unnecessary mutex lock
* refactor: -integration test, +explicit cases
* refactor: remove redundant test, add comment
* rename file and add header
* update copyright year
* tls: add alpn to managed HTTPS records
* tls: centralise HTTPS RR ALPN defaults and registration
Reuse shared protocol defaults instead of repeating the default HTTP protocol list, unify server name registration to carry ALPN in one experimental API and reuse the TLS default ALPN ordering for HTTPS RR publication
* http: centralise effective protocol resolution for HTTPS RR ALPN
Both fallbacks in splitPos relied on golang.org/x/text/search with
search.IgnoreCase, which performs Unicode equivalence matching far beyond
ASCII case folding. Combined with the validated-ASCII guarantee on every
SplitPath entry, that fallback turned non-PHP filenames into PHP scripts:
- when the inner loop hit a non-ASCII byte and the IndexString fallback
returned -1, the loop broke without resetting match=false, so a stale
match=true caused a non-existent .php to be reported (PoC:
"/name.<U+00A1>.txt").
- search.IgnoreCase folded fullwidth, mathematical and circled letters
onto ASCII, so "/shell.<math sans-serif php>",
"/shell.<fullwidth p>hp", "/shell.<circled php>" were all detected as
".php" files.
Replace the fallback with strict byte-level ASCII case-insensitive
matching: any byte >= utf8.RuneSelf in the path can never be part of a
match, since SplitPath entries are validated ASCII-only and lower-cased
in Provision(). This keeps the hot path branch-light and removes the
x/text/search dependency from the main module.
Reported against FrankenPHP as GHSA-3g8v-8r37-cgjm and
GHSA-v4h7-cj44-8fc8. The vulnerable function in this module was adapted
from the same FrankenPHP code.
* caddytls: Expand ACME credentials
This allows using global placeholders such as {file./run/secrets/key_id}
when setting up the tls configuration.
* chore(formatting): gofmt on acmeissuer_test
* Delete old unix domain socket files on Windows
While Windows doesn't have the need to reuse a socket file descriptor
by dup()ing it on config reloads, there still is a valid need for an
equivalent to the `syscall.Unlink()` call in listen_unix.go (also in
`reuseUnixSocket`).
If a previous Caddy instance didn't terminate properly, the chances it
will leave behind a socket file are very high, breaking all subsequent
starting attempts. Other than for regular files, Windows seemingly has
no way for a process to flag a UNIX domain socket file with `FILE_DELETE_ON_CLOSE`,
which means this scenario can never be avoided entirely (e.g. in the case
of crashes).
For the long comment on `isAbstractUnixSocket`: the logic itself is likely
of dubious value, but I thought it better to explicitly reference the
issue, as I have just spent half an hour searching the web to figure out
whether abstract names will work or not on Windows. At least, the logic
as-is should now do the sensible thing if these are ever implemented
properly (and it matches what the Golang standard library does internally).
* Add a dial attempt to check for active server processes
As @steadytao pointed out (thanks!), the previous code didn't have
solid proof that an existing unix socket file had really been orphaned,
as it's also possible that there's another server process (still running).
This would still give the Windows implementation parity with the unix
one (as that one also unlinks the socket file without further checks),
but I've performed a couple of small tests and found this way of handling
socket files still problematic at least problematic if Caddy is used as
a reverse proxy in real world scenarios.
In tests with a simple Caddyfile that only declares an admin socket,
starting two caddy instances with the same Caddyfile works and behaves
like one would expect: the second instance removes the first instance's
socket file and "wins" the race.
When Caddy is used as a reverse proxy, though, what'll happen is more
complicated: While the second instance wins the race for the admin
socket, as long as the Caddyfile specifies a TCP downstream socket,
the second process will not be able to take this one over from the first
(also to be expected, that's how socket binding usually works).
This results in a rather broken state: The first process still holds on
to its TCP listening sockets, the second process fails to start because
of the error in its listening attempt, leaving an orphaned admin socket
file in the file system. Afterwards, the second process won't be running
and the first _will_ be running but unable to be controlled because its
admin socket has been replaced. This leaves the system in another state
that is bad from an ops perspective.
With this new change, we try first to connect to any unix socket that
isn't already covered by our current process (with a very low timeout)
and can easily decide if the socket is still in use by another process:
- If the connection is accepted, there's obviously a server process.
- If Windows returns WSACONNREFUSED [^1], there is either no active
server process for the socket file anymore, or the socket file does
not exist.
- Any other errors are likely a sign that there still is a server process
(e.g. a timeout would indicate that it's just slow in accepting new
connection attempts).
[^1]: https://learn.microsoft.com/en-us/windows/win32/winsock/windows-sockets-error-codes-2#wsaeconnrefused
* chore: tidy Windows unix socket reuse helper
---------
Co-authored-by: Zen Dodd <mail@steadytao.com>
* reverseproxy: Add ability to clear dynamic upstreams cache during retries
This is an optional interface for dynamic upstream modules to implement if they cache results.
TODO: More documentation; this is an experiment.
* Add some godoc
* Export interface; update godoc
