From 050337b25798d23b2a8296531d5d492bdaeb1774 Mon Sep 17 00:00:00 2001 From: Josh Hawn Date: Fri, 30 Jan 2015 17:26:00 -0800 Subject: [PATCH] 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 (github: jlhawn) --- docs/v2/urls.go | 25 +++++++++--- docs/v2/urls_test.go | 93 +++++++++++++++++++++++++------------------- 2 files changed, 73 insertions(+), 45 deletions(-) diff --git a/docs/v2/urls.go b/docs/v2/urls.go index 19ef06fa..d1380b47 100644 --- a/docs/v2/urls.go +++ b/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. - Schemes(ub.root.Scheme). - Host(ub.root.Host) + *route = *ub.router.GetRoute(name) // clone the route + *root = *ub.root + + 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. diff --git a/docs/v2/urls_test.go b/docs/v2/urls_test.go index a9590dba..f30c96c0 100644 --- a/docs/v2/urls_test.go +++ b/docs/v2/urls_test.go @@ -6,62 +6,58 @@ import ( ) type urlBuilderTestCase struct { - description string - expected string - build func() (string, error) + description string + expectedPath string + 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/" - 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) - }{ + testCases := []urlBuilderTestCase{ { - description: "test base url", - expected: "http://localhost:5000/v2/", - build: urlBuilder.BuildBaseURL, + description: "test base url", + expectedPath: "/v2/", + build: func() (string, error) { + return urlBuilder.BuildBaseURL() + }, }, { - description: "test tags url", - expected: "http://localhost:5000/v2/foo/bar/tags/list", + description: "test tags url", + expectedPath: "/v2/foo/bar/tags/list", build: func() (string, error) { return urlBuilder.BuildTagsURL("foo/bar") }, }, { - description: "test manifest url", - expected: "http://localhost:5000/v2/foo/bar/manifests/tag", + description: "test manifest url", + expectedPath: "/v2/foo/bar/manifests/tag", build: func() (string, error) { return urlBuilder.BuildManifestURL("foo/bar", "tag") }, }, { - description: "build blob url", - expected: "http://localhost:5000/v2/foo/bar/blobs/tarsum.v1+sha256:abcdef0123456789", + description: "build blob url", + 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", - expected: "http://localhost:5000/v2/foo/bar/blobs/uploads/", + description: "build blob upload url", + expectedPath: "/v2/foo/bar/blobs/uploads/", build: func() (string, error) { return urlBuilder.BuildBlobUploadURL("foo/bar") }, }, { - 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", + description: "build blob upload url with digest and size", + 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", - expected: "http://localhost:5000/v2/foo/bar/blobs/uploads/uuid-part", + description: "build blob upload chunk url", + 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", - expected: "http://localhost:5000/v2/foo/bar/blobs/uploads/uuid-part?digest=tarsum.v1%2Bsha256%3Aabcdef0123456789&size=10000", + description: "build blob upload chunk url with digest and size", + 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) + } + } + } }