* reverseproxy: Make shallow-ish clone of the request
* Refactor request cloning into separate function
Co-authored-by: Matthew Holt <mholt@users.noreply.github.com>
* reverseproxy: Adjust defaults, document defaults
Related to some of the issues in https://github.com/caddyserver/caddy/issues/4245, a complaint about the proxy transport defaults not being properly documented in https://caddy.community/t/default-values-for-directives/14254/6.
- Dug into the stdlib to find the actual defaults for some of the timeouts and buffer limits, documenting them in godoc so the JSON docs get them next release.
- Moved the keep-alive and dial-timeout defaults from `reverseproxy.go` to `httptransport.go`. It doesn't make sense to set defaults in the proxy, because then any time the transport is configured with non-defaults, the keep-alive and dial-timeout defaults are lost!
- Sped up the dial timeout from 10s to 3s, in practice it rarely makes sense to wait a whole 10s for dialing. A shorter timeout helps a lot with the load balancer retries, so using something lower helps with user experience.
* reverseproxy: Make keepalive interval configurable via Caddyfile
* fastcgi: DialTimeout default for fastcgi transport too
* caddyhttp: Sanitize scheme and host on incoming requests
* reverseproxy: Sanitize the URL scheme and host before proxying
* Apply suggestions from code review
Co-authored-by: Matt Holt <mholt@users.noreply.github.com>
Co-authored-by: Matt Holt <mholt@users.noreply.github.com>
From reading through the code, I think this code path is now obsoleted by the changes made in https://github.com/caddyserver/caddy/pull/4266.
Basically, `h.flushInterval()` will set the flush interval to `-1` if we're in a bi-directional stream, and the recent PR ensured that `h.copyResponse()` properly flushes headers immediately when the flush interval is non-zero. So now there should be no need to call Flush before calling `h.copyResponse()`.
Also split the Caddyfile subdirective keepalive_idle_conns into two properties so the conns and conns_per_host can be set separately.
This is technically a breaking change, but probably anyone who this breaks already had a broken config anyway, and silently fixing it won't help them fix their configs.
Turns out this was an oversight, we assumed we could use `{http.response.header.*}` but that doesn't work because those are grabbed from the response writer, and we haven't copied any headers into the response writer yet.
So the fix is to set all the response headers into the replacer at a new namespace before running the handlers.
This adds the `{http.reverse_proxy.header.*}` replacer.
See https://caddy.community/t/empty-http-response-header-x-accel-redirect/12447
* reverseproxy: Add `handle_response` blocks to `reverse_proxy` (#3710)
* reverseproxy: complete handle_response test
* reverseproxy: Change handle_response matchers to use named matchers
reverseproxy: Add support for changing status code
* fastcgi: Remove obsolete TODO
We already have d.Err("transport already specified") in the reverse_proxy parsing code which covers this case
* reverseproxy: Fix support for "4xx" type status codes
* Apply suggestions from code review
Co-authored-by: Matt Holt <mholt@users.noreply.github.com>
* caddyhttp: Reorganize response matchers
* reverseproxy: Reintroduce caddyfile.Unmarshaler
* reverseproxy: Add comment mentioning Finalize should be called
Co-authored-by: Maxime Soulé <btik-git@scoubidou.com>
Co-authored-by: Matt Holt <mholt@users.noreply.github.com>
* reverseproxy: Implement health_uri, replaces health_path, supports query
Also fixes a bug with `health_status` Caddyfile parsing , it would always only take the first character of the status code even if it didn't end with "xx".
* reverseproxy: Rename to URI, named logger, warn in Provision (for JSON)
* reverseproxy: Add duration/latency placeholders (close#4012) (and #2268)
Adds 4 placeholders, one is actually outside reverse proxy though:
{http.request.duration} is how long since the server decoded the HTTP request (headers).
{http.reverse_proxy.upstream.latency} is how long it took a proxy upstream to write the response header.
{http.reverse_proxy.upstream.duration} is total time proxying to the upstream, including writing response body to client.
{http.reverse_proxy.duration} is total time spent proxying, including selecting an upstream and retries.
Obviously, most of these are only useful at the end of a request, like when writing response headers or logs.
See also: https://caddy.community/t/any-equivalent-of-request-time-and-upstream-header-time-from-nginx/11418
* Add new placeholders to documentation
Proxy response bodies can now be buffered, and the size of the request body and
response body buffer can be limited. Any remaining content that doesn't fit in the
buffer will remain on the wire until it can be read; i.e. bodies are not truncated,
even if the buffer is not big enough.
This fulfills a customer requirement. This was made possible by their sponsorship!
* fix(caddy): Avoid "operation was canceled" errors
- Also add error handling for StatusGatewayTimeout
* revert(caddy): Revert 504 handling
- This will potentially break load balancing and health checks
* Handle client cancellation as different error
Co-authored-by: Matthew Holt <mholt@users.noreply.github.com>
* ci: Use golangci's github action for linting
Signed-off-by: Dave Henderson <dhenderson@gmail.com>
* Fix most of the staticcheck lint errors
Signed-off-by: Dave Henderson <dhenderson@gmail.com>
* Fix the prealloc lint errors
Signed-off-by: Dave Henderson <dhenderson@gmail.com>
* Fix the misspell lint errors
Signed-off-by: Dave Henderson <dhenderson@gmail.com>
* Fix the varcheck lint errors
Signed-off-by: Dave Henderson <dhenderson@gmail.com>
* Fix the errcheck lint errors
Signed-off-by: Dave Henderson <dhenderson@gmail.com>
* Fix the bodyclose lint errors
Signed-off-by: Dave Henderson <dhenderson@gmail.com>
* Fix the deadcode lint errors
Signed-off-by: Dave Henderson <dhenderson@gmail.com>
* Fix the unused lint errors
Signed-off-by: Dave Henderson <dhenderson@gmail.com>
* Fix the gosec lint errors
Signed-off-by: Dave Henderson <dhenderson@gmail.com>
* Fix the gosimple lint errors
Signed-off-by: Dave Henderson <dhenderson@gmail.com>
* Fix the ineffassign lint errors
Signed-off-by: Dave Henderson <dhenderson@gmail.com>
* Fix the staticcheck lint errors
Signed-off-by: Dave Henderson <dhenderson@gmail.com>
* Revert the misspell change, use a neutral English
Signed-off-by: Dave Henderson <dhenderson@gmail.com>
* Remove broken golangci-lint CI job
Signed-off-by: Dave Henderson <dhenderson@gmail.com>
* Re-add errantly-removed weakrand initialization
Signed-off-by: Dave Henderson <dhenderson@gmail.com>
* don't break the loop and return
* Removing extra handling for null rootKey
* unignore RegisterModule/RegisterAdapter
Co-authored-by: Mohammed Al Sahaf <msaa1990@gmail.com>
* single-line log message
Co-authored-by: Matt Holt <mholt@users.noreply.github.com>
* Fix lint after a1808b0dbf209c615e438a496d257ce5e3acdce2 was merged
Signed-off-by: Dave Henderson <dhenderson@gmail.com>
* Revert ticker change, ignore it instead
Signed-off-by: Dave Henderson <dhenderson@gmail.com>
* Ignore some of the write errors
Signed-off-by: Dave Henderson <dhenderson@gmail.com>
* Remove blank line
Signed-off-by: Dave Henderson <dhenderson@gmail.com>
* Use lifetime
Signed-off-by: Dave Henderson <dhenderson@gmail.com>
* close immediately
Co-authored-by: Matt Holt <mholt@users.noreply.github.com>
* Preallocate configVals
Signed-off-by: Dave Henderson <dhenderson@gmail.com>
* Update modules/caddytls/distributedstek/distributedstek.go
Co-authored-by: Mohammed Al Sahaf <msaa1990@gmail.com>
Co-authored-by: Matt Holt <mholt@users.noreply.github.com>
* reverseproxy: Fix dial placeholders, SRV, active health checks
Supercedes #3776
Partially reverts or updates #3756, #3693, and #3695
* reverseproxy: add integration tests
Co-authored-by: Mohammed Al Sahaf <msaa1990@gmail.com>
* reverseproxy: fix breakage in handling SRV lookup introduced by 3695
* reverseproxy: validate against incompatible config options with lookup_srv
* reverseproxy: add integration test cases for validations involving lookup_srv
* reverseproxy: clarify the reason for skipping an iteration
* grammar.. Oxford comma
Co-authored-by: Francis Lavoie <lavofr@gmail.com>
Co-authored-by: Francis Lavoie <lavofr@gmail.com>
Fixes#3753
* reverseproxy: construct active health-check transport from scratch (Fixes#3691)
* reverseproxy: do upstream health-check on the correct alternative port
* reverseproxy: add integration test for health-check on alternative port
* reverseproxy: put back the custom transport for health-check http client
* reverseproxy: cleanup health-check integration test
* reverseproxy: fix health-check of unix socket upstreams
* reverseproxy: skip unix socket tests on Windows
* tabs > spaces
Co-authored-by: Francis Lavoie <lavofr@gmail.com>
* make the linter (and @francislavoie) happy
Co-authored-by: Francis Lavoie <lavofr@gmail.com>
* One more lint fix
Co-authored-by: Francis Lavoie <lavofr@gmail.com>
Co-authored-by: Francis Lavoie <lavofr@gmail.com>
* reverse proxy: Support more h2 stream scenarios (#3556)
* reverse proxy: add integration test for better h2 stream (#3556)
* reverse proxy: adjust comments as francislavoie suggests
* link to issue #3556 in the comments
We already restore them within the retry loop, but after successful
proxy we didn't reset them, so as handlers bubble back up, they would
see the values used for proxying.
Thanks to @ziddey for identifying the cause.
Correct behavior is not well defined because this is a non-standard
header field. This could be a "hop-by-hop" field much like
X-Forwarded-For is, but even our X-Forwarded-For implementation
preserves prior entries. Or, it could be best to preserve the original
value from the first hop, representing the protocol as facing the
client.
Let's try it the other way for a bit and see how it goes.
See https://caddy.community/t/caddy2-w-wordpress-behind-nginx-reverse-proxy/8174/3?u=matt
Either Dial or LookupSRV will be set, but if we rely on Dial always
being set, we could run into bugs.
Note: Health checks don't support SRV upstreams.
Adds `Alt-Svc` to the list of headers that get removed when proxying
to a backend.
This fixes the issue of having the contents of the Alt-Svc header
duplicated when proxying to another Caddy server.
Previously, all matchers in a route would be evaluated before any
handlers were executed, and a composite route of the matching routes
would be created. This made rewrites especially tricky, since the only
way to defer later matchers' evaluation was to wrap them in a subroute,
or to invoke a "rehandle" which often caused bugs.
Instead, this new sequential design evaluates each route's matchers then
its handlers in lock-step; matcher-handlers-matcher-handlers...
If the first matching route consists of a rewrite, then the second route
will be evaluated against the rewritten request, rather than the original
one, and so on.
This should do away with any need for rehandling.
I've also taken this opportunity to avoid adding new values to the
request context in the handler chain, as this creates a copy of the
Request struct, which may possibly lead to bugs like it has in the past
(see PR #1542, PR #1481, and maybe issue #2463). We now add all the
expected context values in the top-level handler at the server, then
any new values can be added to the variable table via the VarsCtxKey
context key, or just the GetVar/SetVar functions. In particular, we are
using this facility to convey dial information in the reverse proxy.
Had to be careful in one place as the middleware compilation logic has
changed, and moved a bit. We no longer compile a middleware chain per-
request; instead, we can compile it at provision-time, and defer only the
evaluation of matchers to request-time, which should slightly improve
performance. Doing this, however, we take advantage of multiple function
closures, and we also changed the use of HandlerFunc (function pointer)
to Handler (interface)... this led to a situation where, if we aren't
careful, allows one request routed a certain way to permanently change
the "next" handler for all/most other requests! We avoid this by making
a copy of the interface value (which is a lightweight pointer copy) and
using exclusively that within our wrapped handlers. This way, the
original stack frame is preserved in a "read-only" fashion. The comments
in the code describe this phenomenon.
This may very well be a breaking change for some configurations, however
I do not expect it to impact many people. I will make it clear in the
release notes that this change has occurred.
