Merge pull request #721 from stevvooe/disambiguate-url-routes

Disambiguate routing for multi-level repository names

app_test.go

@@ -87,21 +87,21 @@ func TestAppDispatcher(t *testing.T) {
 			endpoint: routeNameLayer,
 			vars: []string{
 				"name", "foo/bar",
-				"tarsum", "thetarsum",
+				"tarsum", "tarsum.v1+bogus:abcdef0123456789",
 			},
 		},
 		{
 			endpoint: routeNameLayerUpload,
 			vars: []string{
 				"name", "foo/bar",
-				"tarsum", "thetarsum",
+				"tarsum", "tarsum.v1+bogus:abcdef0123456789",
 			},
 		},
 		{
 			endpoint: routeNameLayerUploadResume,
 			vars: []string{
 				"name", "foo/bar",
-				"tarsum", "thetarsum",
+				"tarsum", "tarsum.v1+bogus:abcdef0123456789",
 				"uuid", "theuuid",
 			},
 		},

common/names.go

@@ -0,0 +1,19 @@
+package common
+
+import (
+	"regexp"
+)
+
+// RepositoryNameComponentRegexp restricts registtry path components names to
+// start with at least two letters or numbers, with following parts able to
+// separated by one period, dash or underscore.
+var RepositoryNameComponentRegexp = regexp.MustCompile(`[a-z0-9]{2,}(?:[._-][a-z0-9]+)*`)
+
+// RepositoryNameRegexp builds on RepositoryNameComponentRegexp to allow 2 to
+// 5 path components, separated by a forward slash.
+var RepositoryNameRegexp = regexp.MustCompile(`(?:` + RepositoryNameComponentRegexp.String() + `/){1,4}` + RepositoryNameComponentRegexp.String())
+
+// TagNameRegexp matches valid tag names. From docker/docker:graph/tags.go.
+var TagNameRegexp = regexp.MustCompile(`[\w][\w.-]{0,127}`)
+
+// TODO(stevvooe): Contribute these exports back to core, so they are shared.

common/names_test.go

@@ -0,0 +1,62 @@
+package common
+
+import (
+	"testing"
+)
+
+func TestRepositoryNameRegexp(t *testing.T) {
+	for _, testcase := range []struct {
+		input string
+		valid bool
+	}{
+		{
+			input: "simple/name",
+			valid: true,
+		},
+		{
+			input: "library/ubuntu",
+			valid: true,
+		},
+		{
+			input: "docker/stevvooe/app",
+			valid: true,
+		},
+		{
+			input: "aa/aa/aa/aa/aa/aa/aa/aa/aa/bb/bb/bb/bb/bb/bb",
+			valid: true,
+		},
+		{
+			input: "a/a/a/a/a/a/b/b/b/b",
+			valid: false,
+		},
+		{
+			input: "a/a/a/a/",
+			valid: false,
+		},
+		{
+			input: "foo.com/bar/baz",
+			valid: true,
+		},
+		{
+			input: "blog.foo.com/bar/baz",
+			valid: true,
+		},
+		{
+			input: "asdf",
+			valid: false,
+		},
+		{
+			input: "asdf$$^/",
+			valid: false,
+		},
+	} {
+		if RepositoryNameRegexp.MatchString(testcase.input) != testcase.valid {
+			status := "invalid"
+			if testcase.valid {
+				status = "valid"
+			}
+
+			t.Fatalf("expected %q to be %s repository name", testcase.input, status)
+		}
+	}
+}

common/tarsum.go

@@ -0,0 +1,70 @@
+package common
+
+import (
+	"fmt"
+
+	"regexp"
+)
+
+// TarSumRegexp defines a reguler expression to match tarsum identifiers.
+var TarsumRegexp = regexp.MustCompile("tarsum(?:.[a-z0-9]+)?\\+[a-zA-Z0-9]+:[A-Fa-f0-9]+")
+
+// TarsumRegexpCapturing defines a reguler expression to match tarsum identifiers with
+// capture groups corresponding to each component.
+var TarsumRegexpCapturing = regexp.MustCompile("(tarsum)(.([a-z0-9]+))?\\+([a-zA-Z0-9]+):([A-Fa-f0-9]+)")
+
+// TarSumInfo contains information about a parsed tarsum.
+type TarSumInfo struct {
+	// Version contains the version of the tarsum.
+	Version string
+
+	// Algorithm contains the algorithm for the final digest
+	Algorithm string
+
+	// Digest contains the hex-encoded digest.
+	Digest string
+}
+
+type InvalidTarSumError struct {
+	TarSum string
+}
+
+func (e InvalidTarSumError) Error() string {
+	return fmt.Sprintf("invalid tarsum: %q", e.TarSum)
+}
+
+// ParseTarSum parses a tarsum string into its components of interest. For
+// example, this method may receive the tarsum in the following format:
+//
+//		tarsum.v1+sha256:220a60ecd4a3c32c282622a625a54db9ba0ff55b5ba9c29c7064a2bc358b6a3e
+//
+// The function will return the following:
+//
+//		TarSumInfo{
+//			Version: "v1",
+//			Algorithm: "sha256",
+//			Digest: "220a60ecd4a3c32c282622a625a54db9ba0ff55b5ba9c29c7064a2bc358b6a3e",
+//		}
+//
+func ParseTarSum(tarSum string) (tsi TarSumInfo, err error) {
+	components := TarsumRegexpCapturing.FindStringSubmatch(tarSum)
+
+	if len(components) != 1+TarsumRegexpCapturing.NumSubexp() {
+		return TarSumInfo{}, InvalidTarSumError{TarSum: tarSum}
+	}
+
+	return TarSumInfo{
+		Version:   components[3],
+		Algorithm: components[4],
+		Digest:    components[5],
+	}, nil
+}
+
+// String returns the valid, string representation of the tarsum info.
+func (tsi TarSumInfo) String() string {
+	if tsi.Version == "" {
+		return fmt.Sprintf("tarsum+%s:%s", tsi.Algorithm, tsi.Digest)
+	}
+
+	return fmt.Sprintf("tarsum.%s+%s:%s", tsi.Version, tsi.Algorithm, tsi.Digest)
+}

common/tarsum_test.go

@@ -0,0 +1,79 @@
+package common
+
+import (
+	"reflect"
+	"testing"
+)
+
+func TestParseTarSumComponents(t *testing.T) {
+	for _, testcase := range []struct {
+		input    string
+		expected TarSumInfo
+		err      error
+	}{
+		{
+			input: "tarsum.v1+sha256:220a60ecd4a3c32c282622a625a54db9ba0ff55b5ba9c29c7064a2bc358b6a3e",
+			expected: TarSumInfo{
+				Version:   "v1",
+				Algorithm: "sha256",
+				Digest:    "220a60ecd4a3c32c282622a625a54db9ba0ff55b5ba9c29c7064a2bc358b6a3e",
+			},
+		},
+		{
+			input: "",
+			err:   InvalidTarSumError{},
+		},
+		{
+			input: "purejunk",
+			err:   InvalidTarSumError{TarSum: "purejunk"},
+		},
+		{
+			input: "tarsum.v23+test:12341234123412341effefefe",
+			expected: TarSumInfo{
+				Version:   "v23",
+				Algorithm: "test",
+				Digest:    "12341234123412341effefefe",
+			},
+		},
+
+		// The following test cases are ported from docker core
+		{
+			// Version 0 tarsum
+			input: "tarsum+sha256:e58fcf7418d4390dec8e8fb69d88c06ec07039d651fedd3aa72af9972e7d046b",
+			expected: TarSumInfo{
+				Algorithm: "sha256",
+				Digest:    "e58fcf7418d4390dec8e8fb69d88c06ec07039d651fedd3aa72af9972e7d046b",
+			},
+		},
+		{
+			// Dev version tarsum
+			input: "tarsum.dev+sha256:e58fcf7418d4390dec8e8fb69d88c06ec07039d651fedd3aa72af9972e7d046b",
+			expected: TarSumInfo{
+				Version:   "dev",
+				Algorithm: "sha256",
+				Digest:    "e58fcf7418d4390dec8e8fb69d88c06ec07039d651fedd3aa72af9972e7d046b",
+			},
+		},
+	} {
+		tsi, err := ParseTarSum(testcase.input)
+		if err != nil {
+			if testcase.err != nil && err == testcase.err {
+				continue // passes
+			}
+
+			t.Fatalf("unexpected error parsing tarsum: %v", err)
+		}
+
+		if testcase.err != nil {
+			t.Fatalf("expected error not encountered on %q: %v", testcase.input, testcase.err)
+		}
+
+		if !reflect.DeepEqual(tsi, testcase.expected) {
+			t.Fatalf("expected tarsum info: %v != %v", tsi, testcase.expected)
+		}
+
+		if testcase.input != tsi.String() {
+			t.Fatalf("input should equal output: %q != %q", tsi.String(), testcase.input)
+		}
+	}
+}

routes.go

@@ -1,12 +1,11 @@
 package registry
 
 import (
+	"github.com/docker/docker-registry/common"
 	"github.com/gorilla/mux"
 )
 
 const (
-	routeNameRoot              = "root"
-	routeNameName              = "name"
 	routeNameImageManifest     = "image-manifest"
 	routeNameTags              = "tags"
 	routeNameLayer             = "layer"
@@ -25,47 +24,36 @@ var allEndpoints = []string{
 // v2APIRouter builds a gorilla router with named routes for the various API
 // methods. We may export this for use by the client.
 func v2APIRouter() *mux.Router {
-	router := mux.NewRouter()
+	router := mux.NewRouter().
-
-	rootRouter := router.
-		PathPrefix("/v2").
-		Name(routeNameRoot).
-		Subrouter()
-
-	// All routes are subordinate to named routes
-	namedRouter := rootRouter.
-		PathPrefix("/{name:[A-Za-z0-9-_]+/[A-Za-z0-9-_]+}"). // TODO(stevvooe): Verify this format with core
-		Name(routeNameName).
-		Subrouter().
 		StrictSlash(true)
 
 	// GET      /v2/<name>/image/<tag>	Image Manifest	Fetch the image manifest identified by name and tag.
 	// PUT      /v2/<name>/image/<tag>	Image Manifest	Upload the image manifest identified by name and tag.
 	// DELETE   /v2/<name>/image/<tag>	Image Manifest	Delete the image identified by name and tag.
-	namedRouter.
+	router.
-		Path("/image/{tag:[A-Za-z0-9-_]+}").
+		Path("/v2/{name:" + common.RepositoryNameRegexp.String() + "}/image/{tag:" + common.TagNameRegexp.String() + "}").
 		Name(routeNameImageManifest)
 
-	// GET	/v2/<name>/tags	Tags	Fetch the tags under the repository identified by name.
+	// GET	/v2/<name>/tags/list	Tags	Fetch the tags under the repository identified by name.
-	namedRouter.
+	router.
-		Path("/tags").
+		Path("/v2/{name:" + common.RepositoryNameRegexp.String() + "}/tags/list").
 		Name(routeNameTags)
 
 	// GET	/v2/<name>/layer/<tarsum>	Layer	Fetch the layer identified by tarsum.
-	namedRouter.
+	router.
-		Path("/layer/{tarsum}").
+		Path("/v2/{name:" + common.RepositoryNameRegexp.String() + "}/layer/{tarsum:" + common.TarsumRegexp.String() + "}").
 		Name(routeNameLayer)
 
 	// POST	/v2/<name>/layer/<tarsum>/upload/	Layer Upload	Initiate an upload of the layer identified by tarsum. Requires length and a checksum parameter.
-	namedRouter.
+	router.
-		Path("/layer/{tarsum}/upload/").
+		Path("/v2/{name:" + common.RepositoryNameRegexp.String() + "}/layer/{tarsum:" + common.TarsumRegexp.String() + "}/upload/").
 		Name(routeNameLayerUpload)
 
 	// GET	/v2/<name>/layer/<tarsum>/upload/<uuid>	Layer Upload	Get the status of the upload identified by tarsum and uuid.
 	// PUT	/v2/<name>/layer/<tarsum>/upload/<uuid>	Layer Upload	Upload all or a chunk of the upload identified by tarsum and uuid.
 	// DELETE	/v2/<name>/layer/<tarsum>/upload/<uuid>	Layer Upload	Cancel the upload identified by layer and uuid
-	namedRouter.
+	router.
-		Path("/layer/{tarsum}/upload/{uuid}").
+		Path("/v2/{name:" + common.RepositoryNameRegexp.String() + "}/layer/{tarsum:" + common.TarsumRegexp.String() + "}/upload/{uuid}").
 		Name(routeNameLayerUploadResume)
 
 	return router

routes_test.go

@@ -10,9 +10,10 @@ import (
 	"github.com/gorilla/mux"
 )
 
-type routeInfo struct {
+type routeTestCase struct {
 	RequestURI string
 	Vars       map[string]string
+	RouteName  string
 }
 
 // TestRouter registers a test handler with all the routes and ensures that
@@ -26,14 +27,15 @@ func TestRouter(t *testing.T) {
 	router := v2APIRouter()
 
 	testHandler := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
-		routeInfo := routeInfo{
+		testCase := routeTestCase{
 			RequestURI: r.RequestURI,
 			Vars:       mux.Vars(r),
+			RouteName:  mux.CurrentRoute(r).GetName(),
 		}
 
 		enc := json.NewEncoder(w)
 
-		if err := enc.Encode(routeInfo); err != nil {
+		if err := enc.Encode(testCase); err != nil {
 			http.Error(w, err.Error(), http.StatusInternalServerError)
 			return
 		}
@@ -42,64 +44,81 @@ func TestRouter(t *testing.T) {
 	// Startup test server
 	server := httptest.NewServer(router)
 
-	for _, testcase := range []struct {
+	for _, testcase := range []routeTestCase{
-		routeName         string
-		expectedRouteInfo routeInfo
-	}{
 		{
-			routeName: routeNameImageManifest,
+			RouteName:  routeNameImageManifest,
-			expectedRouteInfo: routeInfo{
 			RequestURI: "/v2/foo/bar/image/tag",
 			Vars: map[string]string{
 				"name": "foo/bar",
 				"tag":  "tag",
 			},
 		},
-		},
 		{
-			routeName: routeNameTags,
+			RouteName:  routeNameTags,
-			expectedRouteInfo: routeInfo{
+			RequestURI: "/v2/foo/bar/tags/list",
-				RequestURI: "/v2/foo/bar/tags",
 			Vars: map[string]string{
 				"name": "foo/bar",
 			},
 		},
-		},
 		{
-			routeName: routeNameLayer,
+			RouteName:  routeNameLayer,
-			expectedRouteInfo: routeInfo{
+			RequestURI: "/v2/foo/bar/layer/tarsum.dev+foo:abcdef0919234",
-				RequestURI: "/v2/foo/bar/layer/tarsum",
 			Vars: map[string]string{
 				"name":   "foo/bar",
-					"tarsum": "tarsum",
+				"tarsum": "tarsum.dev+foo:abcdef0919234",
-				},
 			},
 		},
 		{
-			routeName: routeNameLayerUpload,
+			RouteName:  routeNameLayerUpload,
-			expectedRouteInfo: routeInfo{
+			RequestURI: "/v2/foo/bar/layer/tarsum.dev+foo:abcdef0919234/upload/",
-				RequestURI: "/v2/foo/bar/layer/tarsum/upload/",
 			Vars: map[string]string{
 				"name":   "foo/bar",
-					"tarsum": "tarsum",
+				"tarsum": "tarsum.dev+foo:abcdef0919234",
-				},
 			},
 		},
 		{
-			routeName: routeNameLayerUploadResume,
+			RouteName:  routeNameLayerUploadResume,
-			expectedRouteInfo: routeInfo{
+			RequestURI: "/v2/foo/bar/layer/tarsum.dev+foo:abcdef0919234/upload/uuid",
-				RequestURI: "/v2/foo/bar/layer/tarsum/upload/uuid",
 			Vars: map[string]string{
 				"name":   "foo/bar",
-					"tarsum": "tarsum",
+				"tarsum": "tarsum.dev+foo:abcdef0919234",
 				"uuid":   "uuid",
 			},
 		},
+		{
+			RouteName:  routeNameLayerUploadResume,
+			RequestURI: "/v2/foo/bar/layer/tarsum.dev+foo:abcdef0919234/upload/D95306FA-FAD3-4E36-8D41-CF1C93EF8286",
+			Vars: map[string]string{
+				"name":   "foo/bar",
+				"tarsum": "tarsum.dev+foo:abcdef0919234",
+				"uuid":   "D95306FA-FAD3-4E36-8D41-CF1C93EF8286",
+			},
+		},
+
+		{
+			// Check ambiguity: ensure we can distinguish between tags for
+			// "foo/bar/image/image" and image for "foo/bar/image" with tag
+			// "tags"
+			RouteName:  routeNameImageManifest,
+			RequestURI: "/v2/foo/bar/image/image/tags",
+			Vars: map[string]string{
+				"name": "foo/bar/image",
+				"tag":  "tags",
+			},
+		},
+		{
+			// This case presents an ambiguity between foo/bar with tag="tags"
+			// and list tags for "foo/bar/image"
+			RouteName:  routeNameTags,
+			RequestURI: "/v2/foo/bar/image/tags/list",
+			Vars: map[string]string{
+				"name": "foo/bar/image",
+			},
 		},
 	} {
 		// Register the endpoint
-		router.GetRoute(testcase.routeName).Handler(testHandler)
+		router.GetRoute(testcase.RouteName).Handler(testHandler)
-		u := server.URL + testcase.expectedRouteInfo.RequestURI
+		u := server.URL + testcase.RequestURI
 
 		resp, err := http.Get(u)
 
@@ -107,15 +126,23 @@ func TestRouter(t *testing.T) {
 			t.Fatalf("error issuing get request: %v", err)
 		}
 
+		if resp.StatusCode != http.StatusOK {
+			t.Fatalf("unexpected status for %s: %v %v", u, resp.Status, resp.StatusCode)
+		}
+
 		dec := json.NewDecoder(resp.Body)
 
-		var actualRouteInfo routeInfo
+		var actualRouteInfo routeTestCase
 		if err := dec.Decode(&actualRouteInfo); err != nil {
 			t.Fatalf("error reading json response: %v", err)
 		}
 
-		if !reflect.DeepEqual(actualRouteInfo, testcase.expectedRouteInfo) {
+		if actualRouteInfo.RouteName != testcase.RouteName {
-			t.Fatalf("actual does not equal expected: %v != %v", actualRouteInfo, testcase.expectedRouteInfo)
+			t.Fatalf("incorrect route %q matched, expected %q", actualRouteInfo.RouteName, testcase.RouteName)
+		}
+
+		if !reflect.DeepEqual(actualRouteInfo, testcase) {
+			t.Fatalf("actual does not equal expected: %#v != %#v", actualRouteInfo, testcase)
 		}
 	}