diff options
author | Matthew Holt <mholt@users.noreply.github.com> | 2020-01-16 17:08:52 -0700 |
---|---|---|
committer | Matthew Holt <mholt@users.noreply.github.com> | 2020-01-16 17:08:52 -0700 |
commit | e51e56a4944622c0a2c7d19da4bb6b9bb07c1973 (patch) | |
tree | b232cf029b993ec8563d10ee6c0a520603816c6d /modules/caddyhttp | |
parent | 21643a007a2d2d90e1636ecd6b49f82560f4c939 (diff) |
httpcaddyfile: Fix nested blocks; add handle directive; refactor
The fix that was initially put forth in #2971 was good, but only for
up to one layer of nesting. The real problem was that we forgot to
increment nesting when already inside a block if we saw another open
curly brace that opens another block (dispenser.go L157-158).
The new 'handle' directive allows HTTP Caddyfiles to be designed more
like nginx location blocks if the user prefers. Inside a handle block,
directives are still ordered just like they are outside of them, but
handler blocks at a given level of nesting are mutually exclusive.
This work benefitted from some refactoring and cleanup.
Diffstat (limited to 'modules/caddyhttp')
-rw-r--r-- | modules/caddyhttp/autohttps.go | 3 | ||||
-rw-r--r-- | modules/caddyhttp/matchers_test.go | 2 | ||||
-rw-r--r-- | modules/caddyhttp/reverseproxy/fastcgi/caddyfile.go | 31 | ||||
-rw-r--r-- | modules/caddyhttp/rewrite/rewrite.go | 2 | ||||
-rw-r--r-- | modules/caddyhttp/routes.go | 1 |
5 files changed, 18 insertions, 21 deletions
diff --git a/modules/caddyhttp/autohttps.go b/modules/caddyhttp/autohttps.go index 6cb0492..69e3318 100644 --- a/modules/caddyhttp/autohttps.go +++ b/modules/caddyhttp/autohttps.go @@ -338,6 +338,9 @@ func (app *App) automaticHTTPSPhase2() error { if err != nil { return fmt.Errorf("%s: managing certificate for %s: %s", srvName, domains, err) } + + // no longer needed; allow GC to deallocate + srv.AutoHTTPS.domainSet = nil } return nil diff --git a/modules/caddyhttp/matchers_test.go b/modules/caddyhttp/matchers_test.go index 8e06546..06f137e 100644 --- a/modules/caddyhttp/matchers_test.go +++ b/modules/caddyhttp/matchers_test.go @@ -199,7 +199,7 @@ func TestPathMatcher(t *testing.T) { }, { match: MatchPath{"*.ext"}, - input: "/foo.ext", + input: "/foo/bar.ext", expect: true, }, { diff --git a/modules/caddyhttp/reverseproxy/fastcgi/caddyfile.go b/modules/caddyhttp/reverseproxy/fastcgi/caddyfile.go index dee6eb5..8c9fd38 100644 --- a/modules/caddyhttp/reverseproxy/fastcgi/caddyfile.go +++ b/modules/caddyhttp/reverseproxy/fastcgi/caddyfile.go @@ -81,7 +81,7 @@ func (t *Transport) UnmarshalCaddyfile(d *caddyfile.Dispenser) error { // // php_fastcgi localhost:7777 // -// is equivalent to: +// is equivalent to a route consisting of: // // @canonicalPath { // file { @@ -104,8 +104,8 @@ func (t *Transport) UnmarshalCaddyfile(d *caddyfile.Dispenser) error { // } // } // -// Thus, this directive produces multiple routes, each with a different -// matcher because multiple consecutive routes are necessary to support +// Thus, this directive produces multiple handlers, each with a different +// matcher because multiple consecutive hgandlers are necessary to support // the common PHP use case. If this "common" config is not compatible // with a user's PHP requirements, they can use a manual approach based // on the example above to configure it precisely as they need. @@ -114,7 +114,7 @@ func (t *Transport) UnmarshalCaddyfile(d *caddyfile.Dispenser) error { // // php_fastcgi /subpath localhost:7777 // -// then the resulting routes are wrapped in a subroute that uses the +// then the resulting handlers are wrapped in a subroute that uses the // user's matcher as a prerequisite to enter the subroute. In other // words, the directive's matcher is necessary, but not sufficient. func parsePHPFastCGI(h httpcaddyfile.Helper) ([]httpcaddyfile.ConfigValue, error) { @@ -198,12 +198,13 @@ func parsePHPFastCGI(h httpcaddyfile.Helper) ([]httpcaddyfile.ConfigValue, error HandlersRaw: []json.RawMessage{caddyconfig.JSONModuleObject(rpHandler, "handler", "reverse_proxy", nil)}, } + subroute := caddyhttp.Subroute{ + Routes: caddyhttp.RouteList{redirRoute, rewriteRoute, rpRoute}, + } + // the user's matcher is a prerequisite for ours, so // wrap ours in a subroute and return that if hasUserMatcher { - subroute := caddyhttp.Subroute{ - Routes: caddyhttp.RouteList{redirRoute, rewriteRoute, rpRoute}, - } return []httpcaddyfile.ConfigValue{ { Class: "route", @@ -215,20 +216,14 @@ func parsePHPFastCGI(h httpcaddyfile.Helper) ([]httpcaddyfile.ConfigValue, error }, nil } - // if the user did not specify a matcher, then - // we can just use our own matchers + // otherwise, return the literal subroute instead of + // individual routes, to ensure they stay together and + // are treated as a single unit, without necessarily + // creating an actual subroute in the output return []httpcaddyfile.ConfigValue{ { Class: "route", - Value: redirRoute, - }, - { - Class: "route", - Value: rewriteRoute, - }, - { - Class: "route", - Value: rpRoute, + Value: subroute, }, }, nil } diff --git a/modules/caddyhttp/rewrite/rewrite.go b/modules/caddyhttp/rewrite/rewrite.go index e2d57c4..ad05486 100644 --- a/modules/caddyhttp/rewrite/rewrite.go +++ b/modules/caddyhttp/rewrite/rewrite.go @@ -50,7 +50,7 @@ type Rewrite struct { // You can also use placeholders. For example, to preserve the existing // query string, you might use: "?{http.request.uri.query}&a=b". Any // key-value pairs you add to the query string will not overwrite - // existing values. + // existing values (individual pairs are append-only). // // To clear the query string, explicitly set an empty one: "?" URI string `json:"uri,omitempty"` diff --git a/modules/caddyhttp/routes.go b/modules/caddyhttp/routes.go index d4ff02a..1224e32 100644 --- a/modules/caddyhttp/routes.go +++ b/modules/caddyhttp/routes.go @@ -252,7 +252,6 @@ func wrapMiddleware(mh MiddlewareHandler) Middleware { return HandlerFunc(func(w http.ResponseWriter, r *http.Request) error { // TODO: This is where request tracing could be implemented - // TODO: Trace a diff of the request, would be cool too... see what changed since the last middleware (host, headers, URI...) // TODO: see what the std lib gives us in terms of stack tracing too return mh.ServeHTTP(w, r, nextCopy) }) |