From 307c18598d7193e3b3a93a638b2ae3e92115a7d1 Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Thu, 19 Jun 2025 13:40:06 +0200 Subject: [PATCH 1/2] registry: ValidateMirror: improve validation for missing schemes Before this patch, a missing scheme would sometimes produce a confusing error message. If no scheme was specified at all, an empty "" would be included in the message; echo '{"registry-mirrors":["example.com"]}' > my-config.json dockerd --config-file ./my-config.json # ... failed to start daemon: invalid mirror: unsupported scheme "" in "example.com" If a scheme was missing, but a port was included, the hostname would be printed as the scheme; echo '{"registry-mirrors":["example.com:8080"]}' > my-config.json dockerd --config-file ./my-config.json # ... failed to start daemon: invalid mirror: unsupported scheme "example.com" in "example.com:8080" With this patch applied, the error messages are slightly more user-friendly; echo '{"registry-mirrors":["example.com"]}' > my-config.json dockerd --config-file ./my-config.json # ... failed to start daemon: invalid mirror: no scheme specified for "example.com": must use either 'https://' or 'http://' echo '{"registry-mirrors":["example.com:8080"]}' > my-config.json dockerd --config-file ./my-config.json # ... failed to start daemon: invalid mirror: no scheme specified for "example.com:8080": must use either 'https://' or 'http://' Signed-off-by: Sebastiaan van Stijn --- registry/config.go | 7 ++++++- registry/config_test.go | 12 ++++++++++-- 2 files changed, 16 insertions(+), 3 deletions(-) diff --git a/registry/config.go b/registry/config.go index 14d000c745..218a12683a 100644 --- a/registry/config.go +++ b/registry/config.go @@ -298,12 +298,17 @@ func isCIDRMatch(cidrs []*registry.NetIPNet, URLHost string) bool { // // It is used by the daemon to validate the daemon configuration. func ValidateMirror(mirrorURL string) (string, error) { + // Fast path for missing scheme, as url.Parse splits by ":", which can + // cause the hostname to be considered the "scheme" when using "hostname:port". + if scheme, _, ok := strings.Cut(mirrorURL, "://"); !ok || scheme == "" { + return "", invalidParamf("invalid mirror: no scheme specified for %q: must use either 'https://' or 'http://'", mirrorURL) + } uri, err := url.Parse(mirrorURL) if err != nil { return "", invalidParamWrapf(err, "invalid mirror: %q is not a valid URI", mirrorURL) } if uri.Scheme != "http" && uri.Scheme != "https" { - return "", invalidParamf("invalid mirror: unsupported scheme %q in %q", uri.Scheme, uri) + return "", invalidParamf("invalid mirror: unsupported scheme %q in %q: must use either 'https://' or 'http://'", uri.Scheme, uri) } if uri.RawQuery != "" || uri.Fragment != "" { return "", invalidParamf("invalid mirror: query or fragment at end of the URI %q", uri) diff --git a/registry/config_test.go b/registry/config_test.go index 17a9a52d73..699e763be6 100644 --- a/registry/config_test.go +++ b/registry/config_test.go @@ -77,9 +77,17 @@ func TestValidateMirror(t *testing.T) { input: "!invalid!://%as%", expectedErr: `invalid mirror: "!invalid!://%as%" is not a valid URI: parse "!invalid!://%as%": first path segment in URL cannot contain colon`, }, + { + input: "mirror-1.example.com", + expectedErr: `invalid mirror: no scheme specified for "mirror-1.example.com": must use either 'https://' or 'http://'`, + }, + { + input: "mirror-1.example.com:5000", + expectedErr: `invalid mirror: no scheme specified for "mirror-1.example.com:5000": must use either 'https://' or 'http://'`, + }, { input: "ftp://mirror-1.example.com", - expectedErr: `invalid mirror: unsupported scheme "ftp" in "ftp://mirror-1.example.com"`, + expectedErr: `invalid mirror: unsupported scheme "ftp" in "ftp://mirror-1.example.com": must use either 'https://' or 'http://'`, }, { input: "http://mirror-1.example.com/?q=foo", @@ -235,7 +243,7 @@ func TestNewServiceConfig(t *testing.T) { opts: ServiceOptions{ Mirrors: []string{"example.com:5000"}, }, - errStr: `invalid mirror: unsupported scheme "example.com" in "example.com:5000"`, + errStr: `invalid mirror: no scheme specified for "example.com:5000": must use either 'https://' or 'http://'`, }, { doc: "valid mirror", From 1d8545d60c3e5500ad1ebc9fce15725bef07a92a Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Thu, 19 Jun 2025 13:57:10 +0200 Subject: [PATCH 2/2] daemon/config: Validate: add missing validation for registry mirrors Validation of registry mirrors was performed during daemon startup, but after the config-file was validated. As a result, the `--validate` option would incorrectly print that the configuration was valid, but the daemon would fail to start; echo '{"registry-mirrors":["example.com"]}' > my-config.json dockerd --config-file ./my-config.json --validate configuration OK dockerd --config-file ./my-config.json # ... failed to start daemon: invalid mirror: no scheme specified for "example.com": must use either 'https://' or 'http://' With this patch applied, validation is also performed as part of the daemon config validation; echo '{"registry-mirrors":["example.com"]}' > my-config.json dockerd --config-file ./my-config.json --validate unable to configure the Docker daemon with file ./my-config.json: merged configuration validation from file and command line flags failed: invalid mirror: no scheme specified for "example.com": must use either 'https://' or 'http://' # fix the invalid config echo '{"registry-mirrors":["https://example.com"]}' > my-config.json dockerd --config-file ./my-config.json --validate configuration OK Signed-off-by: Sebastiaan van Stijn --- daemon/config/config.go | 6 ++++++ daemon/config/config_test.go | 12 ++++++++++++ 2 files changed, 18 insertions(+) diff --git a/daemon/config/config.go b/daemon/config/config.go index 66a0c76db8..c2d464f8c3 100644 --- a/daemon/config/config.go +++ b/daemon/config/config.go @@ -748,6 +748,12 @@ func Validate(config *Config) error { } } + for _, mirror := range config.ServiceOptions.Mirrors { + if _, err := registry.ValidateMirror(mirror); err != nil { + return err + } + } + if config.CorsHeaders != "" { // TODO(thaJeztah): option is used to produce error when used; remove in next release return errors.New(`DEPRECATED: The "api-cors-header" config parameter and the dockerd "--api-cors-header" option have been removed; use a reverse proxy if you need CORS headers`) diff --git a/daemon/config/config_test.go b/daemon/config/config_test.go index bf89f672c3..20514404a0 100644 --- a/daemon/config/config_test.go +++ b/daemon/config/config_test.go @@ -14,6 +14,7 @@ import ( "github.com/docker/docker/api" "github.com/docker/docker/libnetwork/ipamutils" "github.com/docker/docker/opts" + "github.com/docker/docker/registry" "github.com/google/go-cmp/cmp" "github.com/google/go-cmp/cmp/cmpopts" "github.com/spf13/pflag" @@ -428,6 +429,17 @@ func TestValidateConfigurationErrors(t *testing.T) { platform: "windows", expectedErr: "invalid exec-opt (native.cgroupdriver=systemd): option 'native.cgroupdriver' is only supported on linux", }, + { + name: "invalid mirror", + config: &Config{ + CommonConfig: CommonConfig{ + ServiceOptions: registry.ServiceOptions{ + Mirrors: []string{"ftp://example.com"}, + }, + }, + }, + expectedErr: `invalid mirror: unsupported scheme "ftp" in "ftp://example.com": must use either 'https://' or 'http://'`, + }, } for _, tc := range testCases { t.Run(tc.name, func(t *testing.T) {