Browse code

Merge pull request #21641 from yongtang/21595-discrepancy-on-hostname-validation

API/CLI discrepancy on hostname validation (#21595).

Vincent Demeester authored on 2016/04/28 16:25:13
Showing 4 changed files
... ...
@@ -13,6 +13,7 @@ import (
13 13
 	"os"
14 14
 	"path"
15 15
 	"path/filepath"
16
+	"regexp"
16 17
 	"runtime"
17 18
 	"strings"
18 19
 	"sync"
... ...
@@ -1332,6 +1333,18 @@ func (daemon *Daemon) verifyContainerSettings(hostConfig *containertypes.HostCon
1332 1332
 				return nil, err
1333 1333
 			}
1334 1334
 		}
1335
+
1336
+		// Validate if the given hostname is RFC 1123 (https://tools.ietf.org/html/rfc1123) compliant.
1337
+		if len(config.Hostname) > 0 {
1338
+			// RFC1123 specifies that 63 bytes is the maximium length
1339
+			// Windows has the limitation of 63 bytes in length
1340
+			// Linux hostname is limited to HOST_NAME_MAX=64, not not including the terminating null byte.
1341
+			// We limit the length to 63 bytes here to match RFC1035 and RFC1123.
1342
+			matched, _ := regexp.MatchString("^(([[:alnum:]]|[[:alnum:]][[:alnum:]\\-]*[[:alnum:]])\\.)*([[:alnum:]]|[[:alnum:]][[:alnum:]\\-]*[[:alnum:]])$", config.Hostname)
1343
+			if len(config.Hostname) > 63 || !matched {
1344
+				return nil, fmt.Errorf("invalid hostname format: %s", config.Hostname)
1345
+			}
1346
+		}
1335 1347
 	}
1336 1348
 
1337 1349
 	if hostConfig == nil {
... ...
@@ -4295,15 +4295,33 @@ func (s *DockerSuite) TestRunTooLongHostname(c *check.C) {
4295 4295
 	hostname1 := "this-is-a-way-too-long-hostname-but-it-should-give-a-nice-error.local"
4296 4296
 	out, _, err := dockerCmdWithError("run", "--hostname", hostname1, "busybox", "echo", "test")
4297 4297
 	c.Assert(err, checker.NotNil, check.Commentf("Expected docker run to fail!"))
4298
-	c.Assert(out, checker.Contains, "invalid hostname format for --hostname:", check.Commentf("Expected to have 'invalid hostname format for --hostname:' in the output, get: %s!", out))
4298
+	c.Assert(out, checker.Contains, "invalid hostname format:", check.Commentf("Expected to have 'invalid hostname format:' in the output, get: %s!", out))
4299 4299
 
4300
-	// HOST_NAME_MAX=64 so 65 bytes will fail
4301
-	hostname2 := "this-is-a-hostname-with-65-bytes-so-it-should-give-an-error.local"
4302
-	out, _, err = dockerCmdWithError("run", "--hostname", hostname2, "busybox", "echo", "test")
4303
-	c.Assert(err, checker.NotNil, check.Commentf("Expected docker run to fail!"))
4304
-	c.Assert(out, checker.Contains, "invalid hostname format for --hostname:", check.Commentf("Expected to have 'invalid hostname format for --hostname:' in the output, get: %s!", out))
4300
+	// Addtional test cases
4301
+	validHostnames := map[string]string{
4302
+		"hostname":    "hostname",
4303
+		"host-name":   "host-name",
4304
+		"hostname123": "hostname123",
4305
+		"123hostname": "123hostname",
4306
+		"hostname-of-63-bytes-long-should-be-valid-and-without-any-error": "hostname-of-63-bytes-long-should-be-valid-and-without-any-error",
4307
+	}
4308
+	for hostname := range validHostnames {
4309
+		dockerCmd(c, "run", "--hostname", hostname, "busybox", "echo", "test")
4310
+	}
4305 4311
 
4306
-	// 64 bytes will be OK
4307
-	hostname3 := "this-is-a-hostname-with-64-bytes-so-will-not-give-an-error.local"
4308
-	dockerCmd(c, "run", "--hostname", hostname3, "busybox", "echo", "test")
4312
+	invalidHostnames := map[string]string{
4313
+		"^hostname": "invalid hostname format: ^hostname",
4314
+		"hostname%": "invalid hostname format: hostname%",
4315
+		"host&name": "invalid hostname format: host&name",
4316
+		"-hostname": "invalid hostname format: -hostname",
4317
+		"host_name": "invalid hostname format: host_name",
4318
+		"hostname-of-64-bytes-long-should-be-invalid-and-be-with-an-error": "invalid hostname format: hostname-of-64-bytes-long-should-be-invalid-and-be-with-an-error",
4319
+	}
4320
+
4321
+	for hostname, expectedError := range invalidHostnames {
4322
+		out, _, err = dockerCmdWithError("run", "--hostname", hostname, "busybox", "echo", "test")
4323
+		c.Assert(err, checker.NotNil, check.Commentf("Expected docker run to fail!"))
4324
+		c.Assert(out, checker.Contains, expectedError, check.Commentf("Expected to have '%s' in the output, get: %s!", expectedError, out))
4325
+
4326
+	}
4309 4327
 }
... ...
@@ -6,7 +6,6 @@ import (
6 6
 	"fmt"
7 7
 	"io/ioutil"
8 8
 	"path"
9
-	"regexp"
10 9
 	"strconv"
11 10
 	"strings"
12 11
 
... ...
@@ -260,15 +259,6 @@ func Parse(cmd *flag.FlagSet, args []string) (*container.Config, *container.Host
260 260
 	if *flEntrypoint != "" {
261 261
 		entrypoint = strslice.StrSlice{*flEntrypoint}
262 262
 	}
263
-	// Validate if the given hostname is RFC 1123 (https://tools.ietf.org/html/rfc1123) compliant.
264
-	hostname := *flHostname
265
-	if hostname != "" {
266
-		// Linux hostname is limited to HOST_NAME_MAX=64, not including the terminating null byte.
267
-		matched, _ := regexp.MatchString("^(([[:alnum:]]|[[:alnum:]][[:alnum:]\\-]*[[:alnum:]])\\.)*([[:alnum:]]|[[:alnum:]][[:alnum:]\\-]*[[:alnum:]])$", hostname)
268
-		if len(hostname) > 64 || !matched {
269
-			return nil, nil, nil, cmd, fmt.Errorf("invalid hostname format for --hostname: %s", hostname)
270
-		}
271
-	}
272 263
 
273 264
 	ports, portBindings, err := nat.ParsePortSpecs(flPublish.GetAll())
274 265
 	if err != nil {
... ...
@@ -390,15 +390,7 @@ func TestParseHostname(t *testing.T) {
390 390
 		"host-name":   "host-name",
391 391
 		"hostname123": "hostname123",
392 392
 		"123hostname": "123hostname",
393
-		"hostname-of-64-bytes-long-should-be-valid-and-without-any-errors": "hostname-of-64-bytes-long-should-be-valid-and-without-any-errors",
394
-	}
395
-	invalidHostnames := map[string]string{
396
-		"^hostname": "invalid hostname format for --hostname: ^hostname",
397
-		"hostname%": "invalid hostname format for --hostname: hostname%",
398
-		"host&name": "invalid hostname format for --hostname: host&name",
399
-		"-hostname": "invalid hostname format for --hostname: -hostname",
400
-		"host_name": "invalid hostname format for --hostname: host_name",
401
-		"hostname-of-65-bytes-long-should-be-invalid-and-be-given-an-error": "invalid hostname format for --hostname: hostname-of-65-bytes-long-should-be-invalid-and-be-given-an-error",
393
+		"hostname-of-63-bytes-long-should-be-valid-and-without-any-error": "hostname-of-63-bytes-long-should-be-valid-and-without-any-error",
402 394
 	}
403 395
 	hostnameWithDomain := "--hostname=hostname.domainname"
404 396
 	hostnameWithDomainTld := "--hostname=hostname.domainname.tld"
... ...
@@ -407,11 +399,6 @@ func TestParseHostname(t *testing.T) {
407 407
 			t.Fatalf("Expected the config to have 'hostname' as hostname, got '%v'", config.Hostname)
408 408
 		}
409 409
 	}
410
-	for hostname, expectedError := range invalidHostnames {
411
-		if _, _, err := parse(t, fmt.Sprintf("--hostname=%s", hostname)); err == nil || err.Error() != expectedError {
412
-			t.Fatalf("Expected error '%v' with '--hostname=%s', got '%s'", expectedError, hostname, err)
413
-		}
414
-	}
415 410
 	if config, _ := mustParse(t, hostnameWithDomain); config.Hostname != "hostname.domainname" && config.Domainname != "" {
416 411
 		t.Fatalf("Expected the config to have 'hostname' as hostname.domainname, got '%v'", config.Hostname)
417 412
 	}