1. docker
  2. Commit 908be50c441eea157a32333c17a17b817cf4733a
Browse code

Handle gorilla/mux route url bug

When getting the URL from a v2 registry url builder, it does not
honor the scheme from the endpoint object and will cause an https
endpoint to return urls starting with http.

Docker-DCO-1.1-Signed-off-by: Josh Hawn <josh.hawn@docker.com> (github: jlhawn)

Josh Hawn authored on 2015/01/31 10:26:00
Showing 2 changed files
registry/v2/urls.go
History View file @ 908be50
... ... 
@@ -128,13 +128,28 @@ func (ub *URLBuilder) BuildBlobUploadChunkURL(name, uuid string, values ...url.V
128 128
129 129 
 // clondedRoute returns a clone of the named route from the router. Routes
130 130 
 // must be cloned to avoid modifying them during url generation.
131 
-func (ub *URLBuilder) cloneRoute(name string) *mux.Route {
131 
+func (ub *URLBuilder) cloneRoute(name string) clonedRoute {
132 132 
 	route := new(mux.Route)
133 
+	root := new(url.URL)
134 
+
133 135 
 	*route = *ub.router.GetRoute(name) // clone the route
136 
+	*root = *ub.root
137 
+
138 
+	return clonedRoute{Route: route, root: root}
139 
+}
140 
+
141 
+type clonedRoute struct {
142 
+	*mux.Route
143 
+	root *url.URL
144 
+}
145 
+
146 
+func (cr clonedRoute) URL(pairs ...string) (*url.URL, error) {
147 
+	routeURL, err := cr.Route.URL(pairs...)
148 
+	if err != nil {
149 
+		return nil, err
150 
+	}
134 151
135 
-	return route.
136 
-		Schemes(ub.root.Scheme).
137 
-		Host(ub.root.Host)
152 
+	return cr.root.ResolveReference(routeURL), nil
138 153 
 }
139 154
140 155 
 // appendValuesURL appends the parameters to the url.
registry/v2/urls_test.go
History View file @ 908be50
... ... 
@@ -6,62 +6,58 @@ import (
6 6 
 )
7 7
8 8 
 type urlBuilderTestCase struct {
9 
-	description string
10 
-	expected    string
11 
-	build       func() (string, error)
9 
+	description  string
10 
+	expectedPath string
11 
+	build        func() (string, error)
12 12 
 }
13 13
14 14 
 // TestURLBuilder tests the various url building functions, ensuring they are
15 15 
 // returning the expected values.
16 16 
 func TestURLBuilder(t *testing.T) {
17 
+	var (
18 
+		urlBuilder *URLBuilder
19 
+		err        error
20 
+	)
17 21
18 
-	root := "http://localhost:5000/"
19 
-	urlBuilder, err := NewURLBuilderFromString(root)
20 
-	if err != nil {
21 
-		t.Fatalf("unexpected error creating urlbuilder: %v", err)
22 
-	}
23 
-
24 
-	for _, testcase := range []struct {
25 
-		description string
26 
-		expected    string
27 
-		build       func() (string, error)
28 
-	}{
22 
+	testCases := []urlBuilderTestCase{
29 23 
 		{
30 
-			description: "test base url",
31 
-			expected:    "http://localhost:5000/v2/",
32 
-			build:       urlBuilder.BuildBaseURL,
24 
+			description:  "test base url",
25 
+			expectedPath: "/v2/",
26 
+			build: func() (string, error) {
27 
+				return urlBuilder.BuildBaseURL()
28 
+			},
33 29 
 		},
34 30 
 		{
35 
-			description: "test tags url",
36 
-			expected:    "http://localhost:5000/v2/foo/bar/tags/list",
31 
+			description:  "test tags url",
32 
+			expectedPath: "/v2/foo/bar/tags/list",
37 33 
 			build: func() (string, error) {
38 34 
 				return urlBuilder.BuildTagsURL("foo/bar")
39 35 
 			},
40 36 
 		},
41 37 
 		{
42 
-			description: "test manifest url",
43 
-			expected:    "http://localhost:5000/v2/foo/bar/manifests/tag",
38 
+			description:  "test manifest url",
39 
+			expectedPath: "/v2/foo/bar/manifests/tag",
44 40 
 			build: func() (string, error) {
45 41 
 				return urlBuilder.BuildManifestURL("foo/bar", "tag")
46 42 
 			},
47 43 
 		},
48 44 
 		{
49 
-			description: "build blob url",
50 
-			expected:    "http://localhost:5000/v2/foo/bar/blobs/tarsum.v1+sha256:abcdef0123456789",
45 
+			description:  "build blob url",
46 
+			expectedPath: "/v2/foo/bar/blobs/tarsum.v1+sha256:abcdef0123456789",
51 47 
 			build: func() (string, error) {
52 48 
 				return urlBuilder.BuildBlobURL("foo/bar", "tarsum.v1+sha256:abcdef0123456789")
53 49 
 			},
54 50 
 		},
55 51 
 		{
56 
-			description: "build blob upload url",
57 
-			expected:    "http://localhost:5000/v2/foo/bar/blobs/uploads/",
52 
+			description:  "build blob upload url",
53 
+			expectedPath: "/v2/foo/bar/blobs/uploads/",
58 54 
 			build: func() (string, error) {
59 55 
 				return urlBuilder.BuildBlobUploadURL("foo/bar")
60 56 
 			},
61 57 
 		},
62 58 
 		{
63 
-			description: "build blob upload url with digest and size",
64 
-			expected:    "http://localhost:5000/v2/foo/bar/blobs/uploads/?digest=tarsum.v1%2Bsha256%3Aabcdef0123456789&size=10000",
59 
+			description:  "build blob upload url with digest and size",
60 
+			expectedPath: "/v2/foo/bar/blobs/uploads/?digest=tarsum.v1%2Bsha256%3Aabcdef0123456789&size=10000",
65 61 
 			build: func() (string, error) {
66 62 
 				return urlBuilder.BuildBlobUploadURL("foo/bar", url.Values{
67 63 
 					"size":   []string{"10000"},
... ... 
@@ -70,15 +66,15 @@ func TestURLBuilder(t *testing.T) {
70 70 
 			},
71 71 
 		},
72 72 
 		{
73 
-			description: "build blob upload chunk url",
74 
-			expected:    "http://localhost:5000/v2/foo/bar/blobs/uploads/uuid-part",
73 
+			description:  "build blob upload chunk url",
74 
+			expectedPath: "/v2/foo/bar/blobs/uploads/uuid-part",
75 75 
 			build: func() (string, error) {
76 76 
 				return urlBuilder.BuildBlobUploadChunkURL("foo/bar", "uuid-part")
77 77 
 			},
78 78 
 		},
79 79 
 		{
80 
-			description: "build blob upload chunk url with digest and size",
81 
-			expected:    "http://localhost:5000/v2/foo/bar/blobs/uploads/uuid-part?digest=tarsum.v1%2Bsha256%3Aabcdef0123456789&size=10000",
80 
+			description:  "build blob upload chunk url with digest and size",
81 
+			expectedPath: "/v2/foo/bar/blobs/uploads/uuid-part?digest=tarsum.v1%2Bsha256%3Aabcdef0123456789&size=10000",
82 82 
 			build: func() (string, error) {
83 83 
 				return urlBuilder.BuildBlobUploadChunkURL("foo/bar", "uuid-part", url.Values{
84 84 
 					"size":   []string{"10000"},
... ... 
@@ -86,15 +82,32 @@ func TestURLBuilder(t *testing.T) {
86 86 
 				})
87 87 
 			},
88 88 
 		},
89 
-	} {
90 
-		u, err := testcase.build()
89 
+	}
90 
+
91 
+	roots := []string{
92 
+		"http://example.com",
93 
+		"https://example.com",
94 
+		"http://localhost:5000",
95 
+		"https://localhost:5443",
96 
+	}
97 
+
98 
+	for _, root := range roots {
99 
+		urlBuilder, err = NewURLBuilderFromString(root)
91 100 
 		if err != nil {
92 
-			t.Fatalf("%s: error building url: %v", testcase.description, err)
101 
+			t.Fatalf("unexpected error creating urlbuilder: %v", err)
93 102 
 		}
94 103
95 
-		if u != testcase.expected {
96 
-			t.Fatalf("%s: %q != %q", testcase.description, u, testcase.expected)
104 
+		for _, testCase := range testCases {
105 
+			url, err := testCase.build()
106 
+			if err != nil {
107 
+				t.Fatalf("%s: error building url: %v", testCase.description, err)
108 
+			}
109 
+
110 
+			expectedURL := root + testCase.expectedPath
111 
+
112 
+			if url != expectedURL {
113 
+				t.Fatalf("%s: %q != %q", testCase.description, url, expectedURL)
114 
+			}
97 115 
 		}
98 116 
 	}
99 
-
100 117 
 }