aboutsummaryrefslogtreecommitdiffhomepage
diff options
context:
space:
mode:
authorAziz Rmadi <[email protected]>2024-02-18 18:22:48 -0600
committerGitHub <[email protected]>2024-02-19 00:22:48 +0000
commitb893c8c5f856ba7399ed05c8de82fa5c8158ff00 (patch)
tree9a68972de32ea0bd728e79937c476ba79e80decd
parent127788807fdc32feb48a7f24a7e89f954ad3a049 (diff)
downloadcaddy-b893c8c5f856ba7399ed05c8de82fa5c8158ff00.tar.gz
caddy-b893c8c5f856ba7399ed05c8de82fa5c8158ff00.zip
caddyfile: Reject directives in the place of site addresses (#6104)
Co-authored-by: Francis Lavoie <[email protected]>
-rw-r--r--caddyconfig/caddyfile/parse.go52
-rw-r--r--caddyconfig/caddyfile/parse_test.go18
-rw-r--r--caddyconfig/httpcaddyfile/addresses.go10
-rw-r--r--caddyconfig/httpcaddyfile/httptype.go13
-rw-r--r--caddytest/integration/acme_test.go2
-rw-r--r--caddytest/integration/caddyfile_adapt/http_valid_directive_like_site_address.txt46
-rw-r--r--caddytest/integration/caddyfile_test.go28
-rw-r--r--caddytest/integration/pki_test.go6
8 files changed, 135 insertions, 40 deletions
diff --git a/caddyconfig/caddyfile/parse.go b/caddyconfig/caddyfile/parse.go
index 65d6ee927..9f79d913a 100644
--- a/caddyconfig/caddyfile/parse.go
+++ b/caddyconfig/caddyfile/parse.go
@@ -160,14 +160,14 @@ func (p *parser) begin() error {
}
if ok, name := p.isNamedRoute(); ok {
- // named routes only have one key, the route name
- p.block.Keys = []string{name}
- p.block.IsNamedRoute = true
-
// we just need a dummy leading token to ease parsing later
nameToken := p.Token()
nameToken.Text = name
+ // named routes only have one key, the route name
+ p.block.Keys = []Token{nameToken}
+ p.block.IsNamedRoute = true
+
// get all the tokens from the block, including the braces
tokens, err := p.blockTokens(true)
if err != nil {
@@ -211,10 +211,11 @@ func (p *parser) addresses() error {
var expectingAnother bool
for {
- tkn := p.Val()
+ value := p.Val()
+ token := p.Token()
// special case: import directive replaces tokens during parse-time
- if tkn == "import" && p.isNewLine() {
+ if value == "import" && p.isNewLine() {
err := p.doImport(0)
if err != nil {
return err
@@ -223,9 +224,9 @@ func (p *parser) addresses() error {
}
// Open brace definitely indicates end of addresses
- if tkn == "{" {
+ if value == "{" {
if expectingAnother {
- return p.Errf("Expected another address but had '%s' - check for extra comma", tkn)
+ return p.Errf("Expected another address but had '%s' - check for extra comma", value)
}
// Mark this server block as being defined with braces.
// This is used to provide a better error message when
@@ -237,15 +238,15 @@ func (p *parser) addresses() error {
}
// Users commonly forget to place a space between the address and the '{'
- if strings.HasSuffix(tkn, "{") {
- return p.Errf("Site addresses cannot end with a curly brace: '%s' - put a space between the token and the brace", tkn)
+ if strings.HasSuffix(value, "{") {
+ return p.Errf("Site addresses cannot end with a curly brace: '%s' - put a space between the token and the brace", value)
}
- if tkn != "" { // empty token possible if user typed ""
+ if value != "" { // empty token possible if user typed ""
// Trailing comma indicates another address will follow, which
// may possibly be on the next line
- if tkn[len(tkn)-1] == ',' {
- tkn = tkn[:len(tkn)-1]
+ if value[len(value)-1] == ',' {
+ value = value[:len(value)-1]
expectingAnother = true
} else {
expectingAnother = false // but we may still see another one on this line
@@ -254,11 +255,12 @@ func (p *parser) addresses() error {
// If there's a comma here, it's probably because they didn't use a space
// between their two domains, e.g. "foo.com,bar.com", which would not be
// parsed as two separate site addresses.
- if strings.Contains(tkn, ",") {
- return p.Errf("Site addresses cannot contain a comma ',': '%s' - put a space after the comma to separate site addresses", tkn)
+ if strings.Contains(value, ",") {
+ return p.Errf("Site addresses cannot contain a comma ',': '%s' - put a space after the comma to separate site addresses", value)
}
- p.block.Keys = append(p.block.Keys, tkn)
+ token.Text = value
+ p.block.Keys = append(p.block.Keys, token)
}
// Advance token and possibly break out of loop or return error
@@ -637,8 +639,8 @@ func (p *parser) closeCurlyBrace() error {
func (p *parser) isNamedRoute() (bool, string) {
keys := p.block.Keys
// A named route block is a single key with parens, prefixed with &.
- if len(keys) == 1 && strings.HasPrefix(keys[0], "&(") && strings.HasSuffix(keys[0], ")") {
- return true, strings.TrimSuffix(keys[0][2:], ")")
+ if len(keys) == 1 && strings.HasPrefix(keys[0].Text, "&(") && strings.HasSuffix(keys[0].Text, ")") {
+ return true, strings.TrimSuffix(keys[0].Text[2:], ")")
}
return false, ""
}
@@ -646,8 +648,8 @@ func (p *parser) isNamedRoute() (bool, string) {
func (p *parser) isSnippet() (bool, string) {
keys := p.block.Keys
// A snippet block is a single key with parens. Nothing else qualifies.
- if len(keys) == 1 && strings.HasPrefix(keys[0], "(") && strings.HasSuffix(keys[0], ")") {
- return true, strings.TrimSuffix(keys[0][1:], ")")
+ if len(keys) == 1 && strings.HasPrefix(keys[0].Text, "(") && strings.HasSuffix(keys[0].Text, ")") {
+ return true, strings.TrimSuffix(keys[0].Text[1:], ")")
}
return false, ""
}
@@ -691,11 +693,19 @@ func (p *parser) blockTokens(retainCurlies bool) ([]Token, error) {
// grouped by segments.
type ServerBlock struct {
HasBraces bool
- Keys []string
+ Keys []Token
Segments []Segment
IsNamedRoute bool
}
+func (sb ServerBlock) GetKeysText() []string {
+ res := []string{}
+ for _, k := range sb.Keys {
+ res = append(res, k.Text)
+ }
+ return res
+}
+
// DispenseDirective returns a dispenser that contains
// all the tokens in the server block.
func (sb ServerBlock) DispenseDirective(dir string) *Dispenser {
diff --git a/caddyconfig/caddyfile/parse_test.go b/caddyconfig/caddyfile/parse_test.go
index eec94e3af..6daded1cf 100644
--- a/caddyconfig/caddyfile/parse_test.go
+++ b/caddyconfig/caddyfile/parse_test.go
@@ -347,7 +347,7 @@ func TestParseOneAndImport(t *testing.T) {
i, len(test.keys), len(result.Keys))
continue
}
- for j, addr := range result.Keys {
+ for j, addr := range result.GetKeysText() {
if addr != test.keys[j] {
t.Errorf("Test %d, key %d: Expected '%s', but was '%s'",
i, j, test.keys[j], addr)
@@ -379,8 +379,9 @@ func TestRecursiveImport(t *testing.T) {
}
isExpected := func(got ServerBlock) bool {
- if len(got.Keys) != 1 || got.Keys[0] != "localhost" {
- t.Errorf("got keys unexpected: expect localhost, got %v", got.Keys)
+ textKeys := got.GetKeysText()
+ if len(textKeys) != 1 || textKeys[0] != "localhost" {
+ t.Errorf("got keys unexpected: expect localhost, got %v", textKeys)
return false
}
if len(got.Segments) != 2 {
@@ -474,8 +475,9 @@ func TestDirectiveImport(t *testing.T) {
}
isExpected := func(got ServerBlock) bool {
- if len(got.Keys) != 1 || got.Keys[0] != "localhost" {
- t.Errorf("got keys unexpected: expect localhost, got %v", got.Keys)
+ textKeys := got.GetKeysText()
+ if len(textKeys) != 1 || textKeys[0] != "localhost" {
+ t.Errorf("got keys unexpected: expect localhost, got %v", textKeys)
return false
}
if len(got.Segments) != 2 {
@@ -616,7 +618,7 @@ func TestParseAll(t *testing.T) {
i, len(test.keys[j]), j, len(block.Keys))
continue
}
- for k, addr := range block.Keys {
+ for k, addr := range block.GetKeysText() {
if addr != test.keys[j][k] {
t.Errorf("Test %d, block %d, key %d: Expected '%s', but got '%s'",
i, j, k, test.keys[j][k], addr)
@@ -769,7 +771,7 @@ func TestSnippets(t *testing.T) {
if len(blocks) != 1 {
t.Fatalf("Expect exactly one server block. Got %d.", len(blocks))
}
- if actual, expected := blocks[0].Keys[0], "http://example.com"; expected != actual {
+ if actual, expected := blocks[0].GetKeysText()[0], "http://example.com"; expected != actual {
t.Errorf("Expected server name to be '%s' but was '%s'", expected, actual)
}
if len(blocks[0].Segments) != 2 {
@@ -844,7 +846,7 @@ func TestSnippetAcrossMultipleFiles(t *testing.T) {
if len(blocks) != 1 {
t.Fatalf("Expect exactly one server block. Got %d.", len(blocks))
}
- if actual, expected := blocks[0].Keys[0], "http://example.com"; expected != actual {
+ if actual, expected := blocks[0].GetKeysText()[0], "http://example.com"; expected != actual {
t.Errorf("Expected server name to be '%s' but was '%s'", expected, actual)
}
if len(blocks[0].Segments) != 1 {
diff --git a/caddyconfig/httpcaddyfile/addresses.go b/caddyconfig/httpcaddyfile/addresses.go
index 658da48ed..da51fe9b0 100644
--- a/caddyconfig/httpcaddyfile/addresses.go
+++ b/caddyconfig/httpcaddyfile/addresses.go
@@ -88,15 +88,15 @@ func (st *ServerType) mapAddressToServerBlocks(originalServerBlocks []serverBloc
// will be served by them; this has the effect of treating each
// key of a server block as its own, but without having to repeat its
// contents in cases where multiple keys really can be served together
- addrToKeys := make(map[string][]string)
+ addrToKeys := make(map[string][]caddyfile.Token)
for j, key := range sblock.block.Keys {
// a key can have multiple listener addresses if there are multiple
// arguments to the 'bind' directive (although they will all have
// the same port, since the port is defined by the key or is implicit
// through automatic HTTPS)
- addrs, err := st.listenerAddrsForServerBlockKey(sblock, key, options)
+ addrs, err := st.listenerAddrsForServerBlockKey(sblock, key.Text, options)
if err != nil {
- return nil, fmt.Errorf("server block %d, key %d (%s): determining listener address: %v", i, j, key, err)
+ return nil, fmt.Errorf("server block %d, key %d (%s): determining listener address: %v", i, j, key.Text, err)
}
// associate this key with each listener address it is served on
@@ -122,9 +122,9 @@ func (st *ServerType) mapAddressToServerBlocks(originalServerBlocks []serverBloc
// parse keys so that we only have to do it once
parsedKeys := make([]Address, 0, len(keys))
for _, key := range keys {
- addr, err := ParseAddress(key)
+ addr, err := ParseAddress(key.Text)
if err != nil {
- return nil, fmt.Errorf("parsing key '%s': %v", key, err)
+ return nil, fmt.Errorf("parsing key '%s': %v", key.Text, err)
}
parsedKeys = append(parsedKeys, addr.Normalize())
}
diff --git a/caddyconfig/httpcaddyfile/httptype.go b/caddyconfig/httpcaddyfile/httptype.go
index 54e781119..da5557aa8 100644
--- a/caddyconfig/httpcaddyfile/httptype.go
+++ b/caddyconfig/httpcaddyfile/httptype.go
@@ -65,8 +65,11 @@ func (st ServerType) Setup(
originalServerBlocks := make([]serverBlock, 0, len(inputServerBlocks))
for _, sblock := range inputServerBlocks {
for j, k := range sblock.Keys {
- if j == 0 && strings.HasPrefix(k, "@") {
- return nil, warnings, fmt.Errorf("cannot define a matcher outside of a site block: '%s'", k)
+ if j == 0 && strings.HasPrefix(k.Text, "@") {
+ return nil, warnings, fmt.Errorf("%s:%d: cannot define a matcher outside of a site block: '%s'", k.File, k.Line, k.Text)
+ }
+ if _, ok := registeredDirectives[k.Text]; ok {
+ return nil, warnings, fmt.Errorf("%s:%d: parsed '%s' as a site address, but it is a known directive; directives must appear in a site block", k.File, k.Line, k.Text)
}
}
originalServerBlocks = append(originalServerBlocks, serverBlock{
@@ -490,7 +493,7 @@ func (ServerType) extractNamedRoutes(
route.HandlersRaw = []json.RawMessage{caddyconfig.JSONModuleObject(handler, "handler", subroute.CaddyModule().ID.Name(), h.warnings)}
}
- namedRoutes[sb.block.Keys[0]] = &route
+ namedRoutes[sb.block.GetKeysText()[0]] = &route
}
options["named_routes"] = namedRoutes
@@ -528,12 +531,12 @@ func (st *ServerType) serversFromPairings(
// address), otherwise their routes will improperly be added
// to the same server (see issue #4635)
for j, sblock1 := range p.serverBlocks {
- for _, key := range sblock1.block.Keys {
+ for _, key := range sblock1.block.GetKeysText() {
for k, sblock2 := range p.serverBlocks {
if k == j {
continue
}
- if sliceContains(sblock2.block.Keys, key) {
+ if sliceContains(sblock2.block.GetKeysText(), key) {
return nil, fmt.Errorf("ambiguous site definition: %s", key)
}
}
diff --git a/caddytest/integration/acme_test.go b/caddytest/integration/acme_test.go
index 00a3a6c4e..45db8f017 100644
--- a/caddytest/integration/acme_test.go
+++ b/caddytest/integration/acme_test.go
@@ -23,7 +23,7 @@ import (
"go.uber.org/zap"
)
-const acmeChallengePort = 8080
+const acmeChallengePort = 9081
// Test the basic functionality of Caddy's ACME server
func TestACMEServerWithDefaults(t *testing.T) {
diff --git a/caddytest/integration/caddyfile_adapt/http_valid_directive_like_site_address.txt b/caddytest/integration/caddyfile_adapt/http_valid_directive_like_site_address.txt
new file mode 100644
index 000000000..675523a57
--- /dev/null
+++ b/caddytest/integration/caddyfile_adapt/http_valid_directive_like_site_address.txt
@@ -0,0 +1,46 @@
+http://handle {
+ file_server
+}
+----------
+{
+ "apps": {
+ "http": {
+ "servers": {
+ "srv0": {
+ "listen": [
+ ":80"
+ ],
+ "routes": [
+ {
+ "match": [
+ {
+ "host": [
+ "handle"
+ ]
+ }
+ ],
+ "handle": [
+ {
+ "handler": "subroute",
+ "routes": [
+ {
+ "handle": [
+ {
+ "handler": "file_server",
+ "hide": [
+ "./Caddyfile"
+ ]
+ }
+ ]
+ }
+ ]
+ }
+ ],
+ "terminal": true
+ }
+ ]
+ }
+ }
+ }
+ }
+} \ No newline at end of file
diff --git a/caddytest/integration/caddyfile_test.go b/caddytest/integration/caddyfile_test.go
index f9a98fb38..959783be7 100644
--- a/caddytest/integration/caddyfile_test.go
+++ b/caddytest/integration/caddyfile_test.go
@@ -496,6 +496,7 @@ func TestUriReplace(t *testing.T) {
tester.AssertGetResponse("http://localhost:9080/endpoint?test={%20content%20}", 200, "test=%7B%20content%20%7D")
}
+
func TestHandleErrorSimpleCodes(t *testing.T) {
tester := caddytest.NewTester(t)
tester.InitServer(`{
@@ -584,3 +585,30 @@ func TestHandleErrorRangeAndCodes(t *testing.T) {
tester.AssertGetResponse("http://localhost:9080/threehundred", 301, "Error code is equal to 500 or in the [300..399] range")
tester.AssertGetResponse("http://localhost:9080/private", 410, "Error in the [400 .. 499] range")
}
+
+func TestInvalidSiteAddressesAsDirectives(t *testing.T) {
+ type testCase struct {
+ config, expectedError string
+ }
+
+ failureCases := []testCase{
+ {
+ config: `
+ handle {
+ file_server
+ }`,
+ expectedError: `Caddyfile:2: parsed 'handle' as a site address, but it is a known directive; directives must appear in a site block`,
+ },
+ {
+ config: `
+ reverse_proxy localhost:9000 localhost:9001 {
+ file_server
+ }`,
+ expectedError: `Caddyfile:2: parsed 'reverse_proxy' as a site address, but it is a known directive; directives must appear in a site block`,
+ },
+ }
+
+ for _, failureCase := range failureCases {
+ caddytest.AssertLoadError(t, failureCase.config, "caddyfile", failureCase.expectedError)
+ }
+}
diff --git a/caddytest/integration/pki_test.go b/caddytest/integration/pki_test.go
index 5e9928c0c..846798209 100644
--- a/caddytest/integration/pki_test.go
+++ b/caddytest/integration/pki_test.go
@@ -9,6 +9,9 @@ import (
func TestLeafCertLifetimeLessThanIntermediate(t *testing.T) {
caddytest.AssertLoadError(t, `
{
+ "admin": {
+ "disabled": true
+ },
"apps": {
"http": {
"servers": {
@@ -56,6 +59,9 @@ func TestLeafCertLifetimeLessThanIntermediate(t *testing.T) {
func TestIntermediateLifetimeLessThanRoot(t *testing.T) {
caddytest.AssertLoadError(t, `
{
+ "admin": {
+ "disabled": true
+ },
"apps": {
"http": {
"servers": {