Fast and extensible multi-platform HTTP/1-2-3 web server with automatic HTTPS
Mirrored from https://github.com/Slazee/caddy.git
Synced Nov 13, 16:45 UTC
httpcaddyfile: Detect ambiguous site definitions (fix #4635)
Previously, our "duplicate key in server block" logic was flawed because it did not account for the site's bind address. We defer this check to when the listener addresses have been assigned, but before we commit a server block to its listener. Also refined how network address parsing and joining works, which was necessary for a less convoluted fix.
Matthew Holt committed
Jul 25, 2022 at 17:28 UTC
1e18afb5c862d62be130d563785de5c58f08ae8e
5 files changed
+99
-45
caddyconfig/httpcaddyfile/addresses.go
+9
-5
@@ -183,6 +183,8 @@ func (st *ServerType) consolidateAddrMappings(addrToServerBlocks map[string][]se
183
return sbaddrs
184
}
185
186
+// listenerAddrsForServerBlockKey essentially converts the Caddyfile
187
+// site addresses to Caddy listener addresses for each server block.
188
func (st *ServerType) listenerAddrsForServerBlockKey(sblock serverBlock, key string,
189
options map[string]interface{}) ([]string, error) {
190
addr, err := ParseAddress(key)
@@ -232,12 +234,14 @@ func (st *ServerType) listenerAddrsForServerBlockKey(sblock serverBlock, key str
234
// use a map to prevent duplication
235
listeners := make(map[string]struct{})
236
for _, host := range lnHosts {
235
- addr, err := caddy.ParseNetworkAddress(host)
236
- if err == nil && addr.IsUnixNetwork() {
237
- listeners[host] = struct{}{}
238
- } else {
239
- listeners[host+":"+lnPort] = struct{}{}
237
+ // host can have network + host (e.g. "tcp6/localhost") but
238
+ // will/should not have port information because this usually
239
+ // comes from the bind directive, so we append the port
240
+ addr, err := caddy.ParseNetworkAddress(host + ":" + lnPort)
241
+ if err != nil {
242
+ return nil, fmt.Errorf("parsing network address: %v", err)
243
}
244
+ listeners[addr.String()] = struct{}{}
245
}
246
247
// now turn map into list
caddyconfig/httpcaddyfile/httptype.go
+19
-11
@@ -58,22 +58,13 @@ func (st ServerType) Setup(inputServerBlocks []caddyfile.ServerBlock,
58
gc := counter{new(int)}
59
state := make(map[string]interface{})
60
61
- // load all the server blocks and associate them with a "pile"
62
- // of config values; also prohibit duplicate keys because they
63
- // can make a config confusing if more than one server block is
64
- // chosen to handle a request - we actually will make each
65
- // server block's route terminal so that only one will run
66
- sbKeys := make(map[string]struct{})
61
+ // load all the server blocks and associate them with a "pile" of config values
62
originalServerBlocks := make([]serverBlock, 0, len(inputServerBlocks))
68
- for i, sblock := range inputServerBlocks {
63
+ for _, sblock := range inputServerBlocks {
64
for j, k := range sblock.Keys {
65
if j == 0 && strings.HasPrefix(k, "@") {
66
return nil, warnings, fmt.Errorf("cannot define a matcher outside of a site block: '%s'", k)
67
}
73
- if _, ok := sbKeys[k]; ok {
74
- return nil, warnings, fmt.Errorf("duplicate site address not allowed: '%s' in %v (site block %d, key %d)", k, sblock.Keys, i, j)
75
- }
76
- sbKeys[k] = struct{}{}
68
}
69
originalServerBlocks = append(originalServerBlocks, serverBlock{
70
block: sblock,
@@ -420,6 +411,23 @@ func (st *ServerType) serversFromPairings(
411
}
412
413
for i, p := range pairings {
414
+ // detect ambiguous site definitions: server blocks which
415
+ // have the same host bound to the same interface (listener
416
+ // address), otherwise their routes will improperly be added
417
+ // to the same server (see issue #4635)
418
+ for j, sblock1 := range p.serverBlocks {
419
+ for _, key := range sblock1.block.Keys {
420
+ for k, sblock2 := range p.serverBlocks {
421
+ if k == j {
422
+ continue
423
+ }
424
+ if sliceContains(sblock2.block.Keys, key) {
425
+ return nil, fmt.Errorf("ambiguous site definition: %s", key)
426
+ }
427
+ }
428
+ }
429
+ }
430
+
431
srv := &caddyhttp.Server{
432
Listen: p.addresses,
433
}
caddytest/integration/caddyfile_test.go
+1
-1
@@ -68,7 +68,7 @@ func TestDuplicateHosts(t *testing.T) {
68
}
69
`,
70
"caddyfile",
71
- "duplicate site address not allowed")
71
+ "ambiguous site definition")
72
}
73
74
func TestReadCookie(t *testing.T) {
listeners.go
+41
-23
@@ -391,10 +391,13 @@ func (na NetworkAddress) port() string {
391
return fmt.Sprintf("%d-%d", na.StartPort, na.EndPort)
392
}
393
394
-// String reconstructs the address string to the form expected
395
-// by ParseNetworkAddress(). If the address is a unix socket,
396
-// any non-zero port will be dropped.
394
+// String reconstructs the address string for human display.
395
+// The output can be parsed by ParseNetworkAddress(). If the
396
+// address is a unix socket, any non-zero port will be dropped.
397
func (na NetworkAddress) String() string {
398
+ if na.Network == "tcp" && (na.Host != "" || na.port() != "") {
399
+ na.Network = "" // omit default network value for brevity
400
+ }
401
return JoinNetworkAddress(na.Network, na.Host, na.port())
402
}
403
@@ -427,36 +430,38 @@ func isListenBindAddressAlreadyInUseError(err error) bool {
430
func ParseNetworkAddress(addr string) (NetworkAddress, error) {
431
var host, port string
432
network, host, port, err := SplitNetworkAddress(addr)
430
- if network == "" {
431
- network = "tcp"
432
- }
433
if err != nil {
434
return NetworkAddress{}, err
435
}
436
+ if network == "" {
437
+ network = "tcp"
438
+ }
439
if isUnixNetwork(network) {
440
return NetworkAddress{
441
Network: network,
442
Host: host,
443
}, nil
444
}
442
- ports := strings.SplitN(port, "-", 2)
443
- if len(ports) == 1 {
444
- ports = append(ports, ports[0])
445
- }
445
var start, end uint64
447
- start, err = strconv.ParseUint(ports[0], 10, 16)
448
- if err != nil {
449
- return NetworkAddress{}, fmt.Errorf("invalid start port: %v", err)
450
- }
451
- end, err = strconv.ParseUint(ports[1], 10, 16)
452
- if err != nil {
453
- return NetworkAddress{}, fmt.Errorf("invalid end port: %v", err)
454
- }
455
- if end < start {
456
- return NetworkAddress{}, fmt.Errorf("end port must not be less than start port")
457
- }
458
- if (end - start) > maxPortSpan {
459
- return NetworkAddress{}, fmt.Errorf("port range exceeds %d ports", maxPortSpan)
446
+ if port != "" {
447
+ ports := strings.SplitN(port, "-", 2)
448
+ if len(ports) == 1 {
449
+ ports = append(ports, ports[0])
450
+ }
451
+ start, err = strconv.ParseUint(ports[0], 10, 16)
452
+ if err != nil {
453
+ return NetworkAddress{}, fmt.Errorf("invalid start port: %v", err)
454
+ }
455
+ end, err = strconv.ParseUint(ports[1], 10, 16)
456
+ if err != nil {
457
+ return NetworkAddress{}, fmt.Errorf("invalid end port: %v", err)
458
+ }
459
+ if end < start {
460
+ return NetworkAddress{}, fmt.Errorf("end port must not be less than start port")
461
+ }
462
+ if (end - start) > maxPortSpan {
463
+ return NetworkAddress{}, fmt.Errorf("port range exceeds %d ports", maxPortSpan)
464
+ }
465
}
466
return NetworkAddress{
467
Network: network,
@@ -478,6 +483,19 @@ func SplitNetworkAddress(a string) (network, host, port string, err error) {
483
return
484
}
485
host, port, err = net.SplitHostPort(a)
486
+ if err == nil || a == "" {
487
+ return
488
+ }
489
+ // in general, if there was an error, it was likely "missing port",
490
+ // so try adding a bogus port to take advantage of standard library's
491
+ // robust parser, then strip the artificial port before returning
492
+ // (don't overwrite original error though; might still be relevant)
493
+ var err2 error
494
+ host, port, err2 = net.SplitHostPort(a + ":0")
495
+ if err2 == nil {
496
+ err = nil
497
+ port = ""
498
+ }
499
return
500
}
501
listeners_test.go
+29
-5
@@ -32,9 +32,24 @@ func TestSplitNetworkAddress(t *testing.T) {
32
expectErr: true,
33
},
34
{
35
- input: "foo",
35
+ input: "foo",
36
+ expectHost: "foo",
37
+ },
38
+ {
39
+ input: ":", // empty host & empty port
40
+ },
41
+ {
42
+ input: "::",
43
expectErr: true,
44
},
45
+ {
46
+ input: "[::]",
47
+ expectHost: "::",
48
+ },
49
+ {
50
+ input: ":1234",
51
+ expectPort: "1234",
52
+ },
53
{
54
input: "foo:1234",
55
expectHost: "foo",
@@ -80,10 +95,10 @@ func TestSplitNetworkAddress(t *testing.T) {
95
} {
96
actualNetwork, actualHost, actualPort, err := SplitNetworkAddress(tc.input)
97
if tc.expectErr && err == nil {
83
- t.Errorf("Test %d: Expected error but got: %v", i, err)
98
+ t.Errorf("Test %d: Expected error but got %v", i, err)
99
}
100
if !tc.expectErr && err != nil {
86
- t.Errorf("Test %d: Expected no error but got: %v", i, err)
101
+ t.Errorf("Test %d: Expected no error but got %v", i, err)
102
}
103
if actualNetwork != tc.expectNetwork {
104
t.Errorf("Test %d: Expected network '%s' but got '%s'", i, tc.expectNetwork, actualNetwork)
@@ -169,8 +184,17 @@ func TestParseNetworkAddress(t *testing.T) {
184
expectErr: true,
185
},
186
{
172
- input: ":",
173
- expectErr: true,
187
+ input: ":",
188
+ expectAddr: NetworkAddress{
189
+ Network: "tcp",
190
+ },
191
+ },
192
+ {
193
+ input: "[::]",
194
+ expectAddr: NetworkAddress{
195
+ Network: "tcp",
196
+ Host: "::",
197
+ },
198
},
199
{
200
input: ":1234",