From e51e56a4944622c0a2c7d19da4bb6b9bb07c1973 Mon Sep 17 00:00:00 2001 From: Matthew Holt Date: Thu, 16 Jan 2020 17:08:52 -0700 Subject: 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. --- modules/caddyhttp/autohttps.go | 3 +++ modules/caddyhttp/matchers_test.go | 2 +- .../caddyhttp/reverseproxy/fastcgi/caddyfile.go | 31 +++++++++------------- modules/caddyhttp/rewrite/rewrite.go | 2 +- modules/caddyhttp/routes.go | 1 - 5 files changed, 18 insertions(+), 21 deletions(-) (limited to 'modules') 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) }) -- cgit v1.2.3