Merge pull request #10472 from jlhawn/handle_mux_route_url_bug

Handle gorilla/mux route url bug

docs/v2/urls.go

@@ -128,13 +128,28 @@ func (ub *URLBuilder) BuildBlobUploadChunkURL(name, uuid string, values ...url.V
 
 // clondedRoute returns a clone of the named route from the router. Routes
 // must be cloned to avoid modifying them during url generation.
-func (ub *URLBuilder) cloneRoute(name string) *mux.Route {
+func (ub *URLBuilder) cloneRoute(name string) clonedRoute {
 	route := new(mux.Route)
-	*route = *ub.router.GetRoute(name) // clone the route
+	root := new(url.URL)
 
-	return route.
+	*route = *ub.router.GetRoute(name) // clone the route
-		Schemes(ub.root.Scheme).
+	*root = *ub.root
-		Host(ub.root.Host)
+
+	return clonedRoute{Route: route, root: root}
+}
+
+type clonedRoute struct {
+	*mux.Route
+	root *url.URL
+}
+
+func (cr clonedRoute) URL(pairs ...string) (*url.URL, error) {
+	routeURL, err := cr.Route.URL(pairs...)
+	if err != nil {
+		return nil, err
+	}
+
+	return cr.root.ResolveReference(routeURL), nil
 }
 
 // appendValuesURL appends the parameters to the url.

docs/v2/urls_test.go

@@ -6,62 +6,58 @@ import (
 )
 
 type urlBuilderTestCase struct {
-	description string
+	description  string
-	expected    string
+	expectedPath string
-	build       func() (string, error)
+	build        func() (string, error)
 }
 
 // TestURLBuilder tests the various url building functions, ensuring they are
 // returning the expected values.
 func TestURLBuilder(t *testing.T) {
+	var (
+		urlBuilder *URLBuilder
+		err        error
+	)
 
-	root := "http://localhost:5000/"
+	testCases := []urlBuilderTestCase{
-	urlBuilder, err := NewURLBuilderFromString(root)
-	if err != nil {
-		t.Fatalf("unexpected error creating urlbuilder: %v", err)
-	}
-
-	for _, testcase := range []struct {
-		description string
-		expected    string
-		build       func() (string, error)
-	}{
 		{
-			description: "test base url",
+			description:  "test base url",
-			expected:    "http://localhost:5000/v2/",
+			expectedPath: "/v2/",
-			build:       urlBuilder.BuildBaseURL,
+			build: func() (string, error) {
+				return urlBuilder.BuildBaseURL()
+			},
 		},
 		{
-			description: "test tags url",
+			description:  "test tags url",
-			expected:    "http://localhost:5000/v2/foo/bar/tags/list",
+			expectedPath: "/v2/foo/bar/tags/list",
 			build: func() (string, error) {
 				return urlBuilder.BuildTagsURL("foo/bar")
 			},
 		},
 		{
-			description: "test manifest url",
+			description:  "test manifest url",
-			expected:    "http://localhost:5000/v2/foo/bar/manifests/tag",
+			expectedPath: "/v2/foo/bar/manifests/tag",
 			build: func() (string, error) {
 				return urlBuilder.BuildManifestURL("foo/bar", "tag")
 			},
 		},
 		{
-			description: "build blob url",
+			description:  "build blob url",
-			expected:    "http://localhost:5000/v2/foo/bar/blobs/tarsum.v1+sha256:abcdef0123456789",
+			expectedPath: "/v2/foo/bar/blobs/tarsum.v1+sha256:abcdef0123456789",
 			build: func() (string, error) {
 				return urlBuilder.BuildBlobURL("foo/bar", "tarsum.v1+sha256:abcdef0123456789")
 			},
 		},
 		{
-			description: "build blob upload url",
+			description:  "build blob upload url",
-			expected:    "http://localhost:5000/v2/foo/bar/blobs/uploads/",
+			expectedPath: "/v2/foo/bar/blobs/uploads/",
 			build: func() (string, error) {
 				return urlBuilder.BuildBlobUploadURL("foo/bar")
 			},
 		},
 		{
-			description: "build blob upload url with digest and size",
+			description:  "build blob upload url with digest and size",
-			expected:    "http://localhost:5000/v2/foo/bar/blobs/uploads/?digest=tarsum.v1%2Bsha256%3Aabcdef0123456789&size=10000",
+			expectedPath: "/v2/foo/bar/blobs/uploads/?digest=tarsum.v1%2Bsha256%3Aabcdef0123456789&size=10000",
 			build: func() (string, error) {
 				return urlBuilder.BuildBlobUploadURL("foo/bar", url.Values{
 					"size":   []string{"10000"},
@@ -70,15 +66,15 @@ func TestURLBuilder(t *testing.T) {
 			},
 		},
 		{
-			description: "build blob upload chunk url",
+			description:  "build blob upload chunk url",
-			expected:    "http://localhost:5000/v2/foo/bar/blobs/uploads/uuid-part",
+			expectedPath: "/v2/foo/bar/blobs/uploads/uuid-part",
 			build: func() (string, error) {
 				return urlBuilder.BuildBlobUploadChunkURL("foo/bar", "uuid-part")
 			},
 		},
 		{
-			description: "build blob upload chunk url with digest and size",
+			description:  "build blob upload chunk url with digest and size",
-			expected:    "http://localhost:5000/v2/foo/bar/blobs/uploads/uuid-part?digest=tarsum.v1%2Bsha256%3Aabcdef0123456789&size=10000",
+			expectedPath: "/v2/foo/bar/blobs/uploads/uuid-part?digest=tarsum.v1%2Bsha256%3Aabcdef0123456789&size=10000",
 			build: func() (string, error) {
 				return urlBuilder.BuildBlobUploadChunkURL("foo/bar", "uuid-part", url.Values{
 					"size":   []string{"10000"},
@@ -86,15 +82,32 @@ func TestURLBuilder(t *testing.T) {
 				})
 			},
 		},
-	} {
-		u, err := testcase.build()
-		if err != nil {
-			t.Fatalf("%s: error building url: %v", testcase.description, err)
-		}
-
-		if u != testcase.expected {
-			t.Fatalf("%s: %q != %q", testcase.description, u, testcase.expected)
-		}
 	}
 
+	roots := []string{
+		"http://example.com",
+		"https://example.com",
+		"http://localhost:5000",
+		"https://localhost:5443",
+	}
+
+	for _, root := range roots {
+		urlBuilder, err = NewURLBuilderFromString(root)
+		if err != nil {
+			t.Fatalf("unexpected error creating urlbuilder: %v", err)
+		}
+
+		for _, testCase := range testCases {
+			url, err := testCase.build()
+			if err != nil {
+				t.Fatalf("%s: error building url: %v", testCase.description, err)
+			}
+
+			expectedURL := root + testCase.expectedPath
+
+			if url != expectedURL {
+				t.Fatalf("%s: %q != %q", testCase.description, url, expectedURL)
+			}
+		}
+	}
 }