opts: ParseTCPAddr(): extract parsing logic, consistent errors

Make sure we validate the default address given before using it, and
combine the parsing/validation logic so that it can be reused.

This patch also makes the errors more consistent, and uses pkg/errors
for generating them.

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
This commit is contained in:
Sebastiaan van Stijn 2022-04-03 22:00:08 +02:00
parent 5486146943
commit a3ae9a5956
No known key found for this signature in database
GPG Key ID: 76698F39D527CE8C
5 changed files with 96 additions and 70 deletions

View File

@ -68,7 +68,7 @@ const (
swarmConnectTimeout = 20 * time.Second
swarmRequestTimeout = 20 * time.Second
stateFile = "docker-state.json"
defaultAddr = "0.0.0.0:2377"
defaultAddr = "tcp://0.0.0.0:2377"
isWindows = runtime.GOOS == "windows"
initialReconnectDelay = 100 * time.Millisecond
maxReconnectDelay = 30 * time.Second

View File

@ -20,7 +20,7 @@ const (
func resolveListenAddr(specifiedAddr string) (string, string, error) {
specifiedHost, specifiedPort, err := net.SplitHostPort(specifiedAddr)
if err != nil {
return "", "", fmt.Errorf("could not parse listen address %s", specifiedAddr)
return "", "", configError("could not parse listen address " + specifiedAddr)
}
// Does the host component match any of the interface names on the
// system? If so, use the address from that interface.

View File

@ -52,7 +52,7 @@ func (c *Cluster) Init(req types.InitRequest) (string, error) {
listenHost, listenPort, err := resolveListenAddr(req.ListenAddr)
if err != nil {
return "", err
return "", errdefs.InvalidParameter(err)
}
advertiseHost, advertisePort, err := c.resolveAdvertiseAddr(req.AdvertiseAddr, listenPort)
@ -549,6 +549,7 @@ func validateAddr(addr string) (string, error) {
}
newaddr, err := opts.ParseTCPAddr(addr, defaultAddr)
if err != nil {
// TODO(thaJeztah) why are we ignoring the error here? Is this to allow "non-tcp" addresses?
return addr, nil
}
return strings.TrimPrefix(newaddr, "tcp://"), nil

View File

@ -1,7 +1,6 @@
package opts // import "github.com/docker/docker/opts"
import (
"fmt"
"net"
"net/url"
"path/filepath"
@ -77,7 +76,7 @@ func ParseHost(defaultToTLS, defaultToUnixXDG bool, val string) (string, error)
}
// parseDaemonHost parses the specified address and returns an address that will be used as the host.
// Depending of the address specified, this may return one of the global Default* strings defined in hosts.go.
// Depending on the address specified, this may return one of the global Default* strings defined in hosts.go.
func parseDaemonHost(addr string) (string, error) {
addrParts := strings.SplitN(addr, "://", 2)
if len(addrParts) == 1 && addrParts[0] != "" {
@ -86,7 +85,7 @@ func parseDaemonHost(addr string) (string, error) {
switch addrParts[0] {
case "tcp":
return ParseTCPAddr(addrParts[1], DefaultTCPHost)
return ParseTCPAddr(addr, DefaultTCPHost)
case "unix":
return parseSimpleProtoAddr("unix", addrParts[1], DefaultUnixSocket)
case "npipe":
@ -94,7 +93,7 @@ func parseDaemonHost(addr string) (string, error) {
case "fd":
return addr, nil
default:
return "", fmt.Errorf("Invalid bind address format: %s", addr)
return "", errors.Errorf("invalid bind address (%s): unsupported proto '%s'", addr, addrParts[0])
}
}
@ -105,12 +104,12 @@ func parseDaemonHost(addr string) (string, error) {
func parseSimpleProtoAddr(proto, addr, defaultAddr string) (string, error) {
addr = strings.TrimPrefix(addr, proto+"://")
if strings.Contains(addr, "://") {
return "", fmt.Errorf("Invalid proto, expected %s: %s", proto, addr)
return "", errors.Errorf("invalid proto, expected %s: %s", proto, addr)
}
if addr == "" {
addr = defaultAddr
}
return fmt.Sprintf("%s://%s", proto, addr), nil
return proto + "://" + addr, nil
}
// ParseTCPAddr parses and validates that the specified address is a valid TCP
@ -119,62 +118,68 @@ func parseSimpleProtoAddr(proto, addr, defaultAddr string) (string, error) {
// tryAddr is expected to have already been Trim()'d
// defaultAddr must be in the full `tcp://host:port` form
func ParseTCPAddr(tryAddr string, defaultAddr string) (string, error) {
if tryAddr == "" || tryAddr == "tcp://" {
return defaultAddr, nil
}
addr := strings.TrimPrefix(tryAddr, "tcp://")
if strings.Contains(addr, "://") || addr == "" {
return "", fmt.Errorf("Invalid proto, expected tcp: %s", tryAddr)
def, err := parseTCPAddr(defaultAddr, true)
if err != nil {
return "", errors.Wrapf(err, "invalid default address (%s)", defaultAddr)
}
defaultAddr = strings.TrimPrefix(defaultAddr, "tcp://")
defaultHost, defaultPort, err := net.SplitHostPort(defaultAddr)
addr, err := parseTCPAddr(tryAddr, false)
if err != nil {
return "", err
}
u, err := url.Parse("tcp://" + addr)
if err != nil {
return "", err
}
host, port, err := net.SplitHostPort(u.Host)
if err != nil {
// try port addition once
host, port, err = net.SplitHostPort(net.JoinHostPort(u.Host, defaultPort))
}
if err != nil {
return "", fmt.Errorf("Invalid bind address format: %s", tryAddr)
return "", errors.Wrapf(err, "invalid bind address (%s)", tryAddr)
}
host := addr.Hostname()
if host == "" {
host = defaultHost
host = def.Hostname()
}
port := addr.Port()
if port == "" {
port = defaultPort
}
p, err := strconv.Atoi(port)
if err != nil && p == 0 {
return "", fmt.Errorf("Invalid bind address format: %s", tryAddr)
}
if u.Path != "" {
return "", errors.Errorf("invalid bind address (%s): should not contain a path element", tryAddr)
port = def.Port()
}
return "tcp://" + net.JoinHostPort(host, port), nil
}
// parseTCPAddr parses the given addr and validates if it is in the expected
// format. If strict is enabled, the address must contain a scheme (tcp://),
// a host (or IP-address) and a port number.
func parseTCPAddr(address string, strict bool) (*url.URL, error) {
if !strict && !strings.Contains(address, "://") {
address = "tcp://" + address
}
parsedURL, err := url.Parse(address)
if err != nil {
return nil, err
}
if parsedURL.Scheme != "tcp" {
return nil, errors.Errorf("unsupported proto '%s'", parsedURL.Scheme)
}
if parsedURL.Path != "" {
return nil, errors.New("should not contain a path element")
}
if strict && parsedURL.Host == "" {
return nil, errors.New("no host or IP address")
}
if parsedURL.Port() != "" || strict {
if p, err := strconv.Atoi(parsedURL.Port()); err != nil || p == 0 {
return nil, errors.Errorf("invalid port: %s", parsedURL.Port())
}
}
return parsedURL, nil
}
// ValidateExtraHost validates that the specified string is a valid extrahost and returns it.
// ExtraHost is in the form of name:ip where the ip has to be a valid ip (IPv4 or IPv6).
func ValidateExtraHost(val string) (string, error) {
// allow for IPv6 addresses in extra hosts by only splitting on first ":"
arr := strings.SplitN(val, ":", 2)
if len(arr) != 2 || len(arr[0]) == 0 {
return "", fmt.Errorf("bad format for add-host: %q", val)
return "", errors.Errorf("bad format for add-host: %q", val)
}
// Skip IPaddr validation for special "host-gateway" string
if arr[1] != HostGatewayName {
if _, err := ValidateIPAddress(arr[1]); err != nil {
return "", fmt.Errorf("invalid IP address in add-host: %q", arr[1])
return "", errors.Errorf("invalid IP address in add-host: %q", arr[1])
}
}
return val, nil

View File

@ -8,19 +8,19 @@ import (
func TestParseHost(t *testing.T) {
invalid := map[string]string{
"something with spaces": `parse "tcp://something with spaces": invalid character " " in host name`,
"://": `Invalid bind address format: ://`,
"unknown://": `Invalid bind address format: unknown://`,
"tcp://:port": `parse "tcp://:port": invalid port ":port" after host`,
"tcp://invalid:port": `parse "tcp://invalid:port": invalid port ":port" after host`,
"tcp://:5555/": `invalid bind address (:5555/): should not contain a path element`,
"tcp://:5555/p": `invalid bind address (:5555/p): should not contain a path element`,
"tcp://0.0.0.0:5555/": `invalid bind address (0.0.0.0:5555/): should not contain a path element`,
"tcp://0.0.0.0:5555/p": `invalid bind address (0.0.0.0:5555/p): should not contain a path element`,
"tcp://[::1]:/": `invalid bind address ([::1]:/): should not contain a path element`,
"tcp://[::1]:5555/": `invalid bind address ([::1]:5555/): should not contain a path element`,
"tcp://[::1]:5555/p": `invalid bind address ([::1]:5555/p): should not contain a path element`,
" tcp://:5555/path ": `invalid bind address (:5555/path): should not contain a path element`,
"something with spaces": `invalid bind address (something with spaces): parse "tcp://something with spaces": invalid character " " in host name`,
"://": `invalid bind address (://): unsupported proto ''`,
"unknown://": `invalid bind address (unknown://): unsupported proto 'unknown'`,
"tcp://:port": `invalid bind address (tcp://:port): parse "tcp://:port": invalid port ":port" after host`,
"tcp://invalid:port": `invalid bind address (tcp://invalid:port): parse "tcp://invalid:port": invalid port ":port" after host`,
"tcp://:5555/": `invalid bind address (tcp://:5555/): should not contain a path element`,
"tcp://:5555/p": `invalid bind address (tcp://:5555/p): should not contain a path element`,
"tcp://0.0.0.0:5555/": `invalid bind address (tcp://0.0.0.0:5555/): should not contain a path element`,
"tcp://0.0.0.0:5555/p": `invalid bind address (tcp://0.0.0.0:5555/p): should not contain a path element`,
"tcp://[::1]:/": `invalid bind address (tcp://[::1]:/): should not contain a path element`,
"tcp://[::1]:5555/": `invalid bind address (tcp://[::1]:5555/): should not contain a path element`,
"tcp://[::1]:5555/p": `invalid bind address (tcp://[::1]:5555/p): should not contain a path element`,
" tcp://:5555/path ": `invalid bind address (tcp://:5555/path): should not contain a path element`,
}
valid := map[string]string{
@ -33,6 +33,7 @@ func TestParseHost(t *testing.T) {
"tcp://": DefaultTCPHost,
"tcp://:": DefaultTCPHost,
"tcp://:5555": fmt.Sprintf("tcp://%s:5555", DefaultHTTPHost),
"tcp://[::1]": fmt.Sprintf(`tcp://[::1]:%d`, DefaultHTTPPort),
"tcp://[::1]:": fmt.Sprintf(`tcp://[::1]:%d`, DefaultHTTPPort),
"tcp://[::1]:5555": `tcp://[::1]:5555`,
"tcp://0.0.0.0:5555": "tcp://0.0.0.0:5555",
@ -70,18 +71,19 @@ func TestParseHost(t *testing.T) {
func TestParseDockerDaemonHost(t *testing.T) {
invalids := map[string]string{
"tcp:a.b.c.d": `parse "tcp://tcp:a.b.c.d": invalid port ":a.b.c.d" after host`,
"tcp:a.b.c.d/path": `parse "tcp://tcp:a.b.c.d/path": invalid port ":a.b.c.d" after host`,
"udp://127.0.0.1": "Invalid bind address format: udp://127.0.0.1",
"udp://127.0.0.1:5555": "Invalid bind address format: udp://127.0.0.1:5555",
"tcp://unix:///run/docker.sock": "Invalid proto, expected tcp: unix:///run/docker.sock",
" tcp://:5555/path ": "Invalid bind address format: tcp://:5555/path ",
"": "Invalid bind address format: ",
"tcp:a.b.c.d": `invalid bind address (tcp:a.b.c.d): parse "tcp://tcp:a.b.c.d": invalid port ":a.b.c.d" after host`,
"tcp:a.b.c.d/path": `invalid bind address (tcp:a.b.c.d/path): parse "tcp://tcp:a.b.c.d/path": invalid port ":a.b.c.d" after host`,
"tcp://127.0.0.1/": "invalid bind address (tcp://127.0.0.1/): should not contain a path element",
"udp://127.0.0.1": "invalid bind address (udp://127.0.0.1): unsupported proto 'udp'",
"udp://127.0.0.1:5555": "invalid bind address (udp://127.0.0.1:5555): unsupported proto 'udp'",
"tcp://unix:///run/docker.sock": "invalid bind address (tcp://unix:///run/docker.sock): should not contain a path element",
" tcp://:5555/path ": "invalid bind address ( tcp://:5555/path ): unsupported proto ' tcp'",
"": "invalid bind address (): unsupported proto ''",
":5555/path": "invalid bind address (:5555/path): should not contain a path element",
"0.0.0.1:5555/path": "invalid bind address (0.0.0.1:5555/path): should not contain a path element",
"[::1]:5555/path": "invalid bind address ([::1]:5555/path): should not contain a path element",
"[0:0:0:0:0:0:0:1]:5555/path": "invalid bind address ([0:0:0:0:0:0:0:1]:5555/path): should not contain a path element",
"tcp://:5555/path": "invalid bind address (:5555/path): should not contain a path element",
"tcp://:5555/path": "invalid bind address (tcp://:5555/path): should not contain a path element",
"localhost:5555/path": "invalid bind address (localhost:5555/path): should not contain a path element",
}
valids := map[string]string{
@ -89,8 +91,10 @@ func TestParseDockerDaemonHost(t *testing.T) {
":5555": fmt.Sprintf("tcp://%s:5555", DefaultHTTPHost),
"0.0.0.1:": fmt.Sprintf("tcp://0.0.0.1:%d", DefaultHTTPPort),
"0.0.0.1:5555": "tcp://0.0.0.1:5555",
"[::1]": fmt.Sprintf("tcp://[::1]:%d", DefaultHTTPPort),
"[::1]:": fmt.Sprintf("tcp://[::1]:%d", DefaultHTTPPort),
"[::1]:5555": "tcp://[::1]:5555",
"[0:0:0:0:0:0:0:1]": fmt.Sprintf("tcp://[0:0:0:0:0:0:0:1]:%d", DefaultHTTPPort),
"[0:0:0:0:0:0:0:1]:": fmt.Sprintf("tcp://[0:0:0:0:0:0:0:1]:%d", DefaultHTTPPort),
"[0:0:0:0:0:0:0:1]:5555": "tcp://[0:0:0:0:0:0:0:1]:5555",
"localhost": fmt.Sprintf("tcp://localhost:%d", DefaultHTTPPort),
@ -102,6 +106,7 @@ func TestParseDockerDaemonHost(t *testing.T) {
"npipe:////./pipe/foo": "npipe:////./pipe/foo",
"tcp://": DefaultTCPHost,
"tcp://:5555": fmt.Sprintf("tcp://%s:5555", DefaultHTTPHost),
"tcp://[::1]": fmt.Sprintf("tcp://[::1]:%d", DefaultHTTPPort),
"tcp://[::1]:": fmt.Sprintf("tcp://[::1]:%d", DefaultHTTPPort),
"tcp://[::1]:5555": "tcp://[::1]:5555",
"unix://": "unix://" + DefaultUnixSocket,
@ -136,10 +141,10 @@ func TestParseTCP(t *testing.T) {
defaultHTTPHost = "tcp://127.0.0.1:8888"
)
invalids := map[string]string{
"tcp:a.b.c.d": `parse "tcp://tcp:a.b.c.d": invalid port ":a.b.c.d" after host`,
"tcp:a.b.c.d/path": `parse "tcp://tcp:a.b.c.d/path": invalid port ":a.b.c.d" after host`,
"udp://127.0.0.1": "Invalid proto, expected tcp: udp://127.0.0.1",
"udp://127.0.0.1:5555": "Invalid proto, expected tcp: udp://127.0.0.1:5555",
"tcp:a.b.c.d": `invalid bind address (tcp:a.b.c.d): parse "tcp://tcp:a.b.c.d": invalid port ":a.b.c.d" after host`,
"tcp:a.b.c.d/path": `invalid bind address (tcp:a.b.c.d/path): parse "tcp://tcp:a.b.c.d/path": invalid port ":a.b.c.d" after host`,
"udp://127.0.0.1": "invalid bind address (udp://127.0.0.1): unsupported proto 'udp'",
"udp://127.0.0.1:5555": "invalid bind address (udp://127.0.0.1:5555): unsupported proto 'udp'",
":5555/path": "invalid bind address (:5555/path): should not contain a path element",
"0.0.0.1:5555/path": "invalid bind address (0.0.0.1:5555/path): should not contain a path element",
"[::1]:5555/path": "invalid bind address ([::1]:5555/path): should not contain a path element",
@ -147,6 +152,12 @@ func TestParseTCP(t *testing.T) {
"tcp://:5555/path": "invalid bind address (tcp://:5555/path): should not contain a path element",
"localhost:5555/path": "invalid bind address (localhost:5555/path): should not contain a path element",
}
invalidDefaults := map[string]string{
"localhost": `invalid default address (localhost): unsupported proto ''`,
"udp://localhost": `invalid default address (udp://localhost): unsupported proto 'udp'`,
"tcp://localhost:noport": `invalid default address (tcp://localhost:noport): parse "tcp://localhost:noport": invalid port ":noport" after host`,
"tcp://localhost:5555/path": `invalid default address (tcp://localhost:5555/path): should not contain a path element`,
}
valids := map[string]string{
"": defaultHTTPHost,
"0.0.0.1": "tcp://0.0.0.1:8888",
@ -154,9 +165,10 @@ func TestParseTCP(t *testing.T) {
"0.0.0.1:5555": "tcp://0.0.0.1:5555",
":": "tcp://127.0.0.1:8888",
":5555": "tcp://127.0.0.1:5555",
"::1": "tcp://[::1]:8888",
"[::1]": "tcp://[::1]:8888",
"[::1]:": "tcp://[::1]:8888",
"[::1]:5555": "tcp://[::1]:5555",
"[0:0:0:0:0:0:0:1]": "tcp://[0:0:0:0:0:0:0:1]:8888",
"[0:0:0:0:0:0:0:1]:": "tcp://[0:0:0:0:0:0:0:1]:8888",
"[0:0:0:0:0:0:0:1]:5555": "tcp://[0:0:0:0:0:0:0:1]:5555",
"localhost": "tcp://localhost:8888",
@ -177,6 +189,14 @@ func TestParseTCP(t *testing.T) {
}
})
}
for invalidAddr, expectedError := range invalidDefaults {
t.Run("default "+invalidAddr, func(t *testing.T) {
_, err := ParseTCPAddr("tcp://0.0.0.0:2375", invalidAddr)
if err == nil || err.Error() != expectedError {
t.Errorf(`expected error "%s", got "%v"`, expectedError, err)
}
})
}
for validAddr, expectedAddr := range valids {
t.Run(validAddr, func(t *testing.T) {
addr, err := ParseTCPAddr(validAddr, defaultHTTPHost)
@ -191,10 +211,10 @@ func TestParseTCP(t *testing.T) {
}
func TestParseInvalidUnixAddrInvalid(t *testing.T) {
if _, err := parseSimpleProtoAddr("unix", "tcp://127.0.0.1", "unix:///var/run/docker.sock"); err == nil || err.Error() != "Invalid proto, expected unix: tcp://127.0.0.1" {
if _, err := parseSimpleProtoAddr("unix", "tcp://127.0.0.1", "unix:///var/run/docker.sock"); err == nil || err.Error() != "invalid proto, expected unix: tcp://127.0.0.1" {
t.Fatalf("Expected an error, got %v", err)
}
if _, err := parseSimpleProtoAddr("unix", "unix://tcp://127.0.0.1", "/var/run/docker.sock"); err == nil || err.Error() != "Invalid proto, expected unix: tcp://127.0.0.1" {
if _, err := parseSimpleProtoAddr("unix", "unix://tcp://127.0.0.1", "/var/run/docker.sock"); err == nil || err.Error() != "invalid proto, expected unix: tcp://127.0.0.1" {
t.Fatalf("Expected an error, got %v", err)
}
if v, err := parseSimpleProtoAddr("unix", "", "/var/run/docker.sock"); err != nil || v != "unix:///var/run/docker.sock" {