From dac1bfc618fe979a53d90520eb2fd607edcaf8f3 Mon Sep 17 00:00:00 2001 From: Chris Hines Date: Thu, 18 May 2017 15:39:54 -0400 Subject: [PATCH 1/4] Move misplaced tests and fix comments. --- mux_test.go | 91 ++++++++++++++++++++++++++--------------------------- 1 file changed, 45 insertions(+), 46 deletions(-) diff --git a/mux_test.go b/mux_test.go index d0584dfc..1ada3cbe 100644 --- a/mux_test.go +++ b/mux_test.go @@ -1,4 +1,4 @@ -// Copyright 2012 The Gorilla Authors. All rights reserved. +// Copyright 2012 The Gorilla Authors. All rights reserved. // Use of this source code is governed by a BSD-style // license that can be found in the LICENSE file. @@ -31,10 +31,10 @@ type routeTest struct { route *Route // the route being tested request *http.Request // a request to test the route vars map[string]string // the expected vars of the match - host string // the expected host of the match - path string // the expected path of the match - pathTemplate string // the expected path template to match - hostTemplate string // the expected host template to match + host string // the expected host of the built URL + path string // the expected path of the built URL + pathTemplate string // the expected path template of the route + hostTemplate string // the expected host template of the route methods []string // the expected route methods pathRegexp string // the expected path regexp shouldMatch bool // whether the request is expected to match the route at all @@ -197,46 +197,6 @@ func TestHost(t *testing.T) { hostTemplate: `{v-1:[a-z]{3}}.{v-2:[a-z]{3}}.{v-3:[a-z]{3}}`, shouldMatch: true, }, - { - title: "Path route with single pattern with pipe, match", - route: new(Route).Path("/{category:a|b/c}"), - request: newRequest("GET", "http://localhost/a"), - vars: map[string]string{"category": "a"}, - host: "", - path: "/a", - pathTemplate: `/{category:a|b/c}`, - shouldMatch: true, - }, - { - title: "Path route with single pattern with pipe, match", - route: new(Route).Path("/{category:a|b/c}"), - request: newRequest("GET", "http://localhost/b/c"), - vars: map[string]string{"category": "b/c"}, - host: "", - path: "/b/c", - pathTemplate: `/{category:a|b/c}`, - shouldMatch: true, - }, - { - title: "Path route with multiple patterns with pipe, match", - route: new(Route).Path("/{category:a|b/c}/{product}/{id:[0-9]+}"), - request: newRequest("GET", "http://localhost/a/product_name/1"), - vars: map[string]string{"category": "a", "product": "product_name", "id": "1"}, - host: "", - path: "/a/product_name/1", - pathTemplate: `/{category:a|b/c}/{product}/{id:[0-9]+}`, - shouldMatch: true, - }, - { - title: "Path route with multiple patterns with pipe, match", - route: new(Route).Path("/{category:a|b/c}/{product}/{id:[0-9]+}"), - request: newRequest("GET", "http://localhost/b/c/product_name/1"), - vars: map[string]string{"category": "b/c", "product": "product_name", "id": "1"}, - host: "", - path: "/b/c/product_name/1", - pathTemplate: `/{category:a|b/c}/{product}/{id:[0-9]+}`, - shouldMatch: true, - }, } for _, test := range tests { testRoute(t, test) @@ -428,6 +388,46 @@ func TestPath(t *testing.T) { pathRegexp: `^/(?P[0-9]*)(?P[a-z]*)/(?P[0-9]*)$`, shouldMatch: true, }, + { + title: "Path route with single pattern with pipe, match", + route: new(Route).Path("/{category:a|b/c}"), + request: newRequest("GET", "http://localhost/a"), + vars: map[string]string{"category": "a"}, + host: "", + path: "/a", + pathTemplate: `/{category:a|b/c}`, + shouldMatch: true, + }, + { + title: "Path route with single pattern with pipe, match", + route: new(Route).Path("/{category:a|b/c}"), + request: newRequest("GET", "http://localhost/b/c"), + vars: map[string]string{"category": "b/c"}, + host: "", + path: "/b/c", + pathTemplate: `/{category:a|b/c}`, + shouldMatch: true, + }, + { + title: "Path route with multiple patterns with pipe, match", + route: new(Route).Path("/{category:a|b/c}/{product}/{id:[0-9]+}"), + request: newRequest("GET", "http://localhost/a/product_name/1"), + vars: map[string]string{"category": "a", "product": "product_name", "id": "1"}, + host: "", + path: "/a/product_name/1", + pathTemplate: `/{category:a|b/c}/{product}/{id:[0-9]+}`, + shouldMatch: true, + }, + { + title: "Path route with multiple patterns with pipe, match", + route: new(Route).Path("/{category:a|b/c}/{product}/{id:[0-9]+}"), + request: newRequest("GET", "http://localhost/b/c/product_name/1"), + vars: map[string]string{"category": "b/c", "product": "product_name", "id": "1"}, + host: "", + path: "/b/c/product_name/1", + pathTemplate: `/{category:a|b/c}/{product}/{id:[0-9]+}`, + shouldMatch: true, + }, } for _, test := range tests { @@ -649,7 +649,6 @@ func TestHeaders(t *testing.T) { testRoute(t, test) testTemplate(t, test) } - } func TestMethods(t *testing.T) { From d62a8cec03fd2aefb9bc63beeb5336cd6197c942 Mon Sep 17 00:00:00 2001 From: Chris Hines Date: Thu, 18 May 2017 16:17:09 -0400 Subject: [PATCH 2/4] Support building URLs with non-http schemes. - Capture first scheme configured for a route for use when building URLs. - Add new Route.URLScheme method similar to URLHost and URLPath. - Update Route.URLHost and Route.URL to use the captured scheme if present. --- mux_test.go | 114 +++++++++++++++++++++++++++++++++++++++------------- route.go | 36 +++++++++++++++-- 2 files changed, 117 insertions(+), 33 deletions(-) diff --git a/mux_test.go b/mux_test.go index 1ada3cbe..beb850c2 100644 --- a/mux_test.go +++ b/mux_test.go @@ -10,6 +10,7 @@ import ( "errors" "fmt" "net/http" + "net/url" "strings" "testing" ) @@ -31,6 +32,7 @@ type routeTest struct { route *Route // the route being tested request *http.Request // a request to test the route vars map[string]string // the expected vars of the match + scheme string // the expected scheme of the built URL host string // the expected host of the built URL path string // the expected path of the built URL pathTemplate string // the expected path template of the route @@ -516,15 +518,28 @@ func TestPathPrefix(t *testing.T) { } } -func TestHostPath(t *testing.T) { +func TestSchemeHostPath(t *testing.T) { tests := []routeTest{ { title: "Host and Path route, match", route: new(Route).Host("aaa.bbb.ccc").Path("/111/222/333"), request: newRequest("GET", "http://aaa.bbb.ccc/111/222/333"), vars: map[string]string{}, - host: "", - path: "", + scheme: "http", + host: "aaa.bbb.ccc", + path: "/111/222/333", + pathTemplate: `/111/222/333`, + hostTemplate: `aaa.bbb.ccc`, + shouldMatch: true, + }, + { + title: "Scheme, Host, and Path route, match", + route: new(Route).Schemes("https").Host("aaa.bbb.ccc").Path("/111/222/333"), + request: newRequest("GET", "https://aaa.bbb.ccc/111/222/333"), + vars: map[string]string{}, + scheme: "https", + host: "aaa.bbb.ccc", + path: "/111/222/333", pathTemplate: `/111/222/333`, hostTemplate: `aaa.bbb.ccc`, shouldMatch: true, @@ -534,8 +549,9 @@ func TestHostPath(t *testing.T) { route: new(Route).Host("aaa.bbb.ccc").Path("/111/222/333"), request: newRequest("GET", "http://aaa.222.ccc/111/222/333"), vars: map[string]string{}, - host: "", - path: "", + scheme: "http", + host: "aaa.bbb.ccc", + path: "/111/222/333", pathTemplate: `/111/222/333`, hostTemplate: `aaa.bbb.ccc`, shouldMatch: false, @@ -545,6 +561,19 @@ func TestHostPath(t *testing.T) { route: new(Route).Host("aaa.{v1:[a-z]{3}}.ccc").Path("/111/{v2:[0-9]{3}}/333"), request: newRequest("GET", "http://aaa.bbb.ccc/111/222/333"), vars: map[string]string{"v1": "bbb", "v2": "222"}, + scheme: "http", + host: "aaa.bbb.ccc", + path: "/111/222/333", + pathTemplate: `/111/{v2:[0-9]{3}}/333`, + hostTemplate: `aaa.{v1:[a-z]{3}}.ccc`, + shouldMatch: true, + }, + { + title: "Scheme, Host, and Path route with host and path patterns, match", + route: new(Route).Schemes("ftp", "ssss").Host("aaa.{v1:[a-z]{3}}.ccc").Path("/111/{v2:[0-9]{3}}/333"), + request: newRequest("GET", "ssss://aaa.bbb.ccc/111/222/333"), + vars: map[string]string{"v1": "bbb", "v2": "222"}, + scheme: "ftp", host: "aaa.bbb.ccc", path: "/111/222/333", pathTemplate: `/111/{v2:[0-9]{3}}/333`, @@ -556,6 +585,7 @@ func TestHostPath(t *testing.T) { route: new(Route).Host("aaa.{v1:[a-z]{3}}.ccc").Path("/111/{v2:[0-9]{3}}/333"), request: newRequest("GET", "http://aaa.222.ccc/111/222/333"), vars: map[string]string{"v1": "bbb", "v2": "222"}, + scheme: "http", host: "aaa.bbb.ccc", path: "/111/222/333", pathTemplate: `/111/{v2:[0-9]{3}}/333`, @@ -567,6 +597,7 @@ func TestHostPath(t *testing.T) { route: new(Route).Host("{v1:[a-z]{3}}.{v2:[a-z]{3}}.{v3:[a-z]{3}}").Path("/{v4:[0-9]{3}}/{v5:[0-9]{3}}/{v6:[0-9]{3}}"), request: newRequest("GET", "http://aaa.bbb.ccc/111/222/333"), vars: map[string]string{"v1": "aaa", "v2": "bbb", "v3": "ccc", "v4": "111", "v5": "222", "v6": "333"}, + scheme: "http", host: "aaa.bbb.ccc", path: "/111/222/333", pathTemplate: `/{v4:[0-9]{3}}/{v5:[0-9]{3}}/{v6:[0-9]{3}}`, @@ -578,6 +609,7 @@ func TestHostPath(t *testing.T) { route: new(Route).Host("{v1:[a-z]{3}}.{v2:[a-z]{3}}.{v3:[a-z]{3}}").Path("/{v4:[0-9]{3}}/{v5:[0-9]{3}}/{v6:[0-9]{3}}"), request: newRequest("GET", "http://aaa.222.ccc/111/222/333"), vars: map[string]string{"v1": "aaa", "v2": "bbb", "v3": "ccc", "v4": "111", "v5": "222", "v6": "333"}, + scheme: "http", host: "aaa.bbb.ccc", path: "/111/222/333", pathTemplate: `/{v4:[0-9]{3}}/{v5:[0-9]{3}}/{v6:[0-9]{3}}`, @@ -937,30 +969,38 @@ func TestSchemes(t *testing.T) { tests := []routeTest{ // Schemes { - title: "Schemes route, match https", + title: "Schemes route, default scheme, match http, build http", + route: new(Route), + request: newRequest("GET", "http://localhost"), + scheme: "http", + shouldMatch: true, + }, + { + title: "Schemes route, match https, build https", route: new(Route).Schemes("https", "ftp"), request: newRequest("GET", "https://localhost"), - vars: map[string]string{}, - host: "", - path: "", + scheme: "https", shouldMatch: true, }, { - title: "Schemes route, match ftp", + title: "Schemes route, match ftp, build https", route: new(Route).Schemes("https", "ftp"), request: newRequest("GET", "ftp://localhost"), - vars: map[string]string{}, - host: "", - path: "", + scheme: "https", + shouldMatch: true, + }, + { + title: "Schemes route, match ftp, build ftp", + route: new(Route).Schemes("ftp", "https"), + request: newRequest("GET", "ftp://localhost"), + scheme: "ftp", shouldMatch: true, }, { title: "Schemes route, bad scheme", route: new(Route).Schemes("https", "ftp"), request: newRequest("GET", "http://localhost"), - vars: map[string]string{}, - host: "", - path: "", + scheme: "https", shouldMatch: false, }, } @@ -1447,10 +1487,15 @@ func testRoute(t *testing.T, test routeTest) { route := test.route vars := test.vars shouldMatch := test.shouldMatch - host := test.host - path := test.path - url := test.host + test.path shouldRedirect := test.shouldRedirect + uri := url.URL{ + Scheme: test.scheme, + Host: test.host, + Path: test.path, + } + if uri.Scheme == "" { + uri.Scheme = "http" + } var match RouteMatch ok := route.Match(request, &match) @@ -1463,28 +1508,39 @@ func testRoute(t *testing.T, test routeTest) { return } if shouldMatch { - if test.vars != nil && !stringMapEqual(test.vars, match.Vars) { + if vars != nil && !stringMapEqual(vars, match.Vars) { t.Errorf("(%v) Vars not equal: expected %v, got %v", test.title, vars, match.Vars) return } - if host != "" { + if test.scheme != "" { + u, _ := route.URLScheme() + if uri.Scheme != u.Scheme { + t.Errorf("(%v) URLScheme not equal: expected %v, got %v", test.title, uri.Scheme, u.Scheme) + return + } + } + if test.host != "" { u, _ := test.route.URLHost(mapToPairs(match.Vars)...) - if host != u.Host { - t.Errorf("(%v) URLHost not equal: expected %v, got %v -- %v", test.title, host, u.Host, getRouteTemplate(route)) + if uri.Scheme != u.Scheme { + t.Errorf("(%v) URLHost scheme not equal: expected %v, got %v -- %v", test.title, uri.Scheme, u.Scheme, getRouteTemplate(route)) + return + } + if uri.Host != u.Host { + t.Errorf("(%v) URLHost host not equal: expected %v, got %v -- %v", test.title, uri.Host, u.Host, getRouteTemplate(route)) return } } - if path != "" { + if test.path != "" { u, _ := route.URLPath(mapToPairs(match.Vars)...) - if path != u.Path { - t.Errorf("(%v) URLPath not equal: expected %v, got %v -- %v", test.title, path, u.Path, getRouteTemplate(route)) + if uri.Path != u.Path { + t.Errorf("(%v) URLPath not equal: expected %v, got %v -- %v", test.title, uri.Path, u.Path, getRouteTemplate(route)) return } } - if url != "" { + if test.host != "" && test.path != "" { u, _ := route.URL(mapToPairs(match.Vars)...) - if url != u.Host+u.Path { - t.Errorf("(%v) URL not equal: expected %v, got %v -- %v", test.title, url, u.Host+u.Path, getRouteTemplate(route)) + if expected, got := uri.String(), u.String(); expected != got { + t.Errorf("(%v) URL not equal: expected %v, got %v -- %v", test.title, expected, got, getRouteTemplate(route)) return } } diff --git a/route.go b/route.go index a7c7d8c6..410d5de1 100644 --- a/route.go +++ b/route.go @@ -31,6 +31,8 @@ type Route struct { skipClean bool // If true, "/path/foo%2Fbar/to" will match the path "/path/{var}/to" useEncodedPath bool + // The scheme used when building URLs. + buildScheme string // If true, this route never matches: it is only used to build URLs. buildOnly bool // The name used to build URLs. @@ -394,6 +396,9 @@ func (r *Route) Schemes(schemes ...string) *Route { for k, v := range schemes { schemes[k] = strings.ToLower(v) } + if r.buildScheme == "" && len(schemes) > 0 { + r.buildScheme = schemes[0] + } return r.addMatcher(schemeMatcher(schemes)) } @@ -478,11 +483,13 @@ func (r *Route) URL(pairs ...string) (*url.URL, error) { } var scheme, host, path string if r.regexp.host != nil { - // Set a default scheme. - scheme = "http" if host, err = r.regexp.host.url(values); err != nil { return nil, err } + scheme = "http" + if r.buildScheme != "" { + scheme = r.buildScheme + } } if r.regexp.path != nil { if path, err = r.regexp.path.url(values); err != nil { @@ -496,6 +503,23 @@ func (r *Route) URL(pairs ...string) (*url.URL, error) { }, nil } +// URLScheme builds the scheme part of the URL for a route. See Route.URL(). +// +// A route with multiple schemes will return the first scheme of the route. A +// route with no schemes will return "http" as the scheme. +func (r *Route) URLScheme() (*url.URL, error) { + if r.err != nil { + return nil, r.err + } + u := &url.URL{ + Scheme: "http", + } + if r.buildScheme != "" { + u.Scheme = r.buildScheme + } + return u, nil +} + // URLHost builds the host part of the URL for a route. See Route.URL(). // // The route must have a host defined. @@ -514,10 +538,14 @@ func (r *Route) URLHost(pairs ...string) (*url.URL, error) { if err != nil { return nil, err } - return &url.URL{ + u := &url.URL{ Scheme: "http", Host: host, - }, nil + } + if r.buildScheme != "" { + u.Scheme = r.buildScheme + } + return u, nil } // URLPath builds the path part of the URL for a route. See Route.URL(). From a8bcb6a8f0ab891e7cf6ebbfc7de38d165eec9c0 Mon Sep 17 00:00:00 2001 From: Chris Hines Date: Sat, 20 May 2017 15:33:21 -0400 Subject: [PATCH 3/4] Remove Route.URLScheme method. --- mux_test.go | 35 ++++++++++++++++++++++++++--------- route.go | 17 ----------------- 2 files changed, 26 insertions(+), 26 deletions(-) diff --git a/mux_test.go b/mux_test.go index beb850c2..d186ae89 100644 --- a/mux_test.go +++ b/mux_test.go @@ -970,37 +970,42 @@ func TestSchemes(t *testing.T) { // Schemes { title: "Schemes route, default scheme, match http, build http", - route: new(Route), + route: new(Route).Host("localhost"), request: newRequest("GET", "http://localhost"), scheme: "http", + host: "localhost", shouldMatch: true, }, { title: "Schemes route, match https, build https", - route: new(Route).Schemes("https", "ftp"), + route: new(Route).Schemes("https", "ftp").Host("localhost"), request: newRequest("GET", "https://localhost"), scheme: "https", + host: "localhost", shouldMatch: true, }, { title: "Schemes route, match ftp, build https", - route: new(Route).Schemes("https", "ftp"), + route: new(Route).Schemes("https", "ftp").Host("localhost"), request: newRequest("GET", "ftp://localhost"), scheme: "https", + host: "localhost", shouldMatch: true, }, { title: "Schemes route, match ftp, build ftp", - route: new(Route).Schemes("ftp", "https"), + route: new(Route).Schemes("ftp", "https").Host("localhost"), request: newRequest("GET", "ftp://localhost"), scheme: "ftp", + host: "localhost", shouldMatch: true, }, { title: "Schemes route, bad scheme", - route: new(Route).Schemes("https", "ftp"), + route: new(Route).Schemes("https", "ftp").Host("localhost"), request: newRequest("GET", "http://localhost"), scheme: "https", + host: "localhost", shouldMatch: false, }, } @@ -1513,14 +1518,20 @@ func testRoute(t *testing.T, test routeTest) { return } if test.scheme != "" { - u, _ := route.URLScheme() + u, err := route.URL(mapToPairs(match.Vars)...) + if err != nil { + t.Fatalf("(%v) URL error: %v -- %v", test.title, err, getRouteTemplate(route)) + } if uri.Scheme != u.Scheme { t.Errorf("(%v) URLScheme not equal: expected %v, got %v", test.title, uri.Scheme, u.Scheme) return } } if test.host != "" { - u, _ := test.route.URLHost(mapToPairs(match.Vars)...) + u, err := test.route.URLHost(mapToPairs(match.Vars)...) + if err != nil { + t.Fatalf("(%v) URLHost error: %v -- %v", test.title, err, getRouteTemplate(route)) + } if uri.Scheme != u.Scheme { t.Errorf("(%v) URLHost scheme not equal: expected %v, got %v -- %v", test.title, uri.Scheme, u.Scheme, getRouteTemplate(route)) return @@ -1531,14 +1542,20 @@ func testRoute(t *testing.T, test routeTest) { } } if test.path != "" { - u, _ := route.URLPath(mapToPairs(match.Vars)...) + u, err := route.URLPath(mapToPairs(match.Vars)...) + if err != nil { + t.Fatalf("(%v) URLPath error: %v -- %v", test.title, err, getRouteTemplate(route)) + } if uri.Path != u.Path { t.Errorf("(%v) URLPath not equal: expected %v, got %v -- %v", test.title, uri.Path, u.Path, getRouteTemplate(route)) return } } if test.host != "" && test.path != "" { - u, _ := route.URL(mapToPairs(match.Vars)...) + u, err := route.URL(mapToPairs(match.Vars)...) + if err != nil { + t.Fatalf("(%v) URL error: %v -- %v", test.title, err, getRouteTemplate(route)) + } if expected, got := uri.String(), u.String(); expected != got { t.Errorf("(%v) URL not equal: expected %v, got %v -- %v", test.title, expected, got, getRouteTemplate(route)) return diff --git a/route.go b/route.go index 410d5de1..56dcbbdc 100644 --- a/route.go +++ b/route.go @@ -503,23 +503,6 @@ func (r *Route) URL(pairs ...string) (*url.URL, error) { }, nil } -// URLScheme builds the scheme part of the URL for a route. See Route.URL(). -// -// A route with multiple schemes will return the first scheme of the route. A -// route with no schemes will return "http" as the scheme. -func (r *Route) URLScheme() (*url.URL, error) { - if r.err != nil { - return nil, r.err - } - u := &url.URL{ - Scheme: "http", - } - if r.buildScheme != "" { - u.Scheme = r.buildScheme - } - return u, nil -} - // URLHost builds the host part of the URL for a route. See Route.URL(). // // The route must have a host defined. From 1c96fcc630f5ee8bfb85c75dd4217638728e341c Mon Sep 17 00:00:00 2001 From: Chris Hines Date: Sat, 20 May 2017 23:50:47 -0400 Subject: [PATCH 4/4] Remove UTF-8 BOM. --- mux_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/mux_test.go b/mux_test.go index d186ae89..19ef5a8c 100644 --- a/mux_test.go +++ b/mux_test.go @@ -1,4 +1,4 @@ -// Copyright 2012 The Gorilla Authors. All rights reserved. +// Copyright 2012 The Gorilla Authors. All rights reserved. // Use of this source code is governed by a BSD-style // license that can be found in the LICENSE file.