Allow hostname components in component names.

Fixes https://github.com/docker/distribution/issues/1062

This relaxes the naming restrictions places on Docker images to permit
valid hostnames according to [RFC-2396](https://www.ietf.org/rfc/rfc2396.txt).

It deviates from the RFC in the following ways:
 1) Allow underscores where we allow hyphens (hostnames don't allow
 underscores, which we must for backwards compatibility).
 2) Leave "top-level" name segments unrestricted (domains require an
 alpha character to begin a top-level domain, e.g. "com").
 3) DO NOT allow a trailing dot, as permitted by FQDNs.

Signed-off-by: Matt Moore <mattmoor@google.com>

registry/api/v2/names.go

@@ -15,10 +15,23 @@ const (
 	RepositoryNameTotalLengthMax = 255
 )
 
+// domainLabelRegexp represents the following RFC-2396 BNF construct:
+//   domainlabel   = alphanum | alphanum *( alphanum | "-" ) alphanum
+var domainLabelRegexp = regexp.MustCompile(`[a-z0-9](?:-*[a-z0-9])*`)
+
 // RepositoryNameComponentRegexp restricts registry path component names to
-// start with at least one letter or number, with following parts able to
+// the allow valid hostnames according to: https://www.ietf.org/rfc/rfc2396.txt
-// be separated by one period, dash or underscore.
+// with the following differences:
-var RepositoryNameComponentRegexp = regexp.MustCompile(`[a-z0-9]+(?:[._-][a-z0-9]+)*`)
+//  1) It DOES NOT allow for fully-qualified domain names, which include a
+//    trailing '.', e.g. "google.com."
+//  2) It DOES NOT restrict 'top-level' domain labels to start with just alpha
+//    characters.
+//  3) It DOES allow for underscores to appear in the same situations as dots.
+//
+// RFC-2396 uses the BNF construct:
+//   hostname      = *( domainlabel "." ) toplabel [ "." ]
+var RepositoryNameComponentRegexp = regexp.MustCompile(
+	domainLabelRegexp.String() + `(?:[._]` + domainLabelRegexp.String() + `)*`)
 
 // RepositoryNameComponentAnchoredRegexp is the version of
 // RepositoryNameComponentRegexp which must completely match the content

registry/api/v2/names_test.go

@@ -164,22 +164,47 @@ var (
 			err:     ErrRepositoryNameComponentInvalid,
 			invalid: true,
 		},
+		{
+			input:   "do__cker/docker",
+			err:     ErrRepositoryNameComponentInvalid,
+			invalid: true,
+		},
+		{
+			input:   "docker./docker",
+			err:     ErrRepositoryNameComponentInvalid,
+			invalid: true,
+		},
+		{
+			input:   ".docker/docker",
+			err:     ErrRepositoryNameComponentInvalid,
+			invalid: true,
+		},
+		{
+			input:   "do..cker/docker",
+			err:     ErrRepositoryNameComponentInvalid,
+			invalid: true,
+		},
+		{
+			input:   "docker-/docker",
+			err:     ErrRepositoryNameComponentInvalid,
+			invalid: true,
+		},
+		{
+			input:   "-docker/docker",
+			err:     ErrRepositoryNameComponentInvalid,
+			invalid: true,
+		},
 		{
 			input: "b.gcr.io/test.example.com/my-app", // embedded domain component
 		},
-		// TODO(stevvooe): The following is a punycode domain name that we may
-		// want to allow in the future. Currently, this is not allowed but we
-		// may want to change this in the future. Adding this here as invalid
-		// for the time being.
 		{
-			input:   "xn--n3h.com/myimage", // http://☃.com in punycode
+			input: "xn--n3h.com/myimage", // http://☃.com in punycode
-			err:     ErrRepositoryNameComponentInvalid,
-			invalid: true,
 		},
 		{
-			input:   "xn--7o8h.com/myimage", // http://🐳.com in punycode
+			input: "xn--7o8h.com/myimage", // http://🐳.com in punycode
-			err:     ErrRepositoryNameComponentInvalid,
+		},
-			invalid: true,
+		{
+			input: "registry.io/foo/project--id.module--name.ver---sion--name", // image with hostname
 		},
 	}
 )

registry/client/repository.go

@@ -359,25 +359,18 @@ type blobs struct {
 	distribution.BlobDeleter
 }
 
-func sanitizeLocation(location, source string) (string, error) {
+func sanitizeLocation(location, base string) (string, error) {
+	baseURL, err := url.Parse(base)
+	if err != nil {
+		return "", err
+	}
+
 	locationURL, err := url.Parse(location)
 	if err != nil {
 		return "", err
 	}
 
-	if locationURL.Scheme == "" {
+	return baseURL.ResolveReference(locationURL).String(), nil
-		sourceURL, err := url.Parse(source)
-		if err != nil {
-			return "", err
-		}
-		locationURL = &url.URL{
-			Scheme: sourceURL.Scheme,
-			Host:   sourceURL.Host,
-			Path:   location,
-		}
-		location = locationURL.String()
-	}
-	return location, nil
 }
 
 func (bs *blobs) Stat(ctx context.Context, dgst digest.Digest) (distribution.Descriptor, error) {

registry/client/repository_test.go

@@ -857,3 +857,49 @@ func TestCatalogInParts(t *testing.T) {
 		t.Fatalf("Got wrong number of repos")
 	}
 }
+
+func TestSanitizeLocation(t *testing.T) {
+	for _, testcase := range []struct {
+		description string
+		location    string
+		source      string
+		expected    string
+		err         error
+	}{
+		{
+			description: "ensure relative location correctly resolved",
+			location:    "/v2/foo/baasdf",
+			source:      "http://blahalaja.com/v1",
+			expected:    "http://blahalaja.com/v2/foo/baasdf",
+		},
+		{
+			description: "ensure parameters are preserved",
+			location:    "/v2/foo/baasdf?_state=asdfasfdasdfasdf&digest=foo",
+			source:      "http://blahalaja.com/v1",
+			expected:    "http://blahalaja.com/v2/foo/baasdf?_state=asdfasfdasdfasdf&digest=foo",
+		},
+		{
+			description: "ensure new hostname overidden",
+			location:    "https://mwhahaha.com/v2/foo/baasdf?_state=asdfasfdasdfasdf",
+			source:      "http://blahalaja.com/v1",
+			expected:    "https://mwhahaha.com/v2/foo/baasdf?_state=asdfasfdasdfasdf",
+		},
+	} {
+		fatalf := func(format string, args ...interface{}) {
+			t.Fatalf(testcase.description+": "+format, args...)
+		}
+
+		s, err := sanitizeLocation(testcase.location, testcase.source)
+		if err != testcase.err {
+			if testcase.err != nil {
+				fatalf("expected error: %v != %v", err, testcase)
+			} else {
+				fatalf("unexpected error sanitizing: %v", err)
+			}
+		}
+
+		if s != testcase.expected {
+			fatalf("bad sanitize: %q != %q", s, testcase.expected)
+		}
+	}
+}