From d10d5466f2db2758bb616f627775692d2f69fe8a Mon Sep 17 00:00:00 2001 From: Philip Schlump Date: Thu, 14 Nov 2013 19:27:38 -0700 Subject: [PATCH 1/4] Fixed problem with droping query string. --- mux.go | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/mux.go b/mux.go index ddc1acc..ca51a01 100644 --- a/mux.go +++ b/mux.go @@ -67,6 +67,14 @@ func (r *Router) Match(req *http.Request, match *RouteMatch) bool { func (r *Router) ServeHTTP(w http.ResponseWriter, req *http.Request) { // Clean path to canonical form and redirect. if p := cleanPath(req.URL.Path); p != req.URL.Path { + + // Added 3 lines (Philip Schlump) - It was droping the query string and #whatever from query. + // This matches with fix in go 1.2 r.c. 4 for same problem. Go Issue: + // http://code.google.com/p/go/issues/detail?id=5252 + url := *req.URL + url.Path = p + p = url.String() + w.Header().Set("Location", p) w.WriteHeader(http.StatusMovedPermanently) return From 1a2411f44a858576b750ffb9d5c0975570ef1f2d Mon Sep 17 00:00:00 2001 From: Philip Schlump Date: Mon, 18 Nov 2013 10:14:28 -0700 Subject: [PATCH 2/4] Added tests to verify that the 301 redirect returns query string --- mux_test.go | 79 ++++++++++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 78 insertions(+), 1 deletion(-) diff --git a/mux_test.go b/mux_test.go index 8789697..4cbd55d 100644 --- a/mux_test.go +++ b/mux_test.go @@ -8,6 +8,7 @@ import ( "fmt" "net/http" "testing" +// "encoding/json" // Philip Schlump "github.com/gorilla/context" ) @@ -22,6 +23,18 @@ type routeTest struct { shouldMatch bool // whether the request is expected to match the route at all } +// Philip Schlump - added to understand the results from tests. +//func dumpVar ( v interface{} ) { +// // s, err := json.Marshal ( v ) +// s, err := json.MarshalIndent ( v, "", "\t" ) +// if ( err != nil ) { +// fmt.Printf ( "Error: %s\n", err ) +// } else { +// fmt.Printf ( "%s\n", s ) +// } +//} + + func TestHost(t *testing.T) { // newRequestHost a new request with a method, url, and host header newRequestHost := func(method, url, host string) *http.Request { @@ -416,6 +429,15 @@ func TestQueries(t *testing.T) { path: "", shouldMatch: true, }, + { + title: "Queries route, match (ToDo - with redirect 301) (Philip Schlump added)", + route: new(Route).Host("www.2cwhy.com").Path("/api").Queries("foo", "bar", "baz", "ding"), + request: newRequest("GET", "http://www.2cwhy.com/api?foo=bar&baz=ding"), + vars: map[string]string{}, + host: "", + path: "", + shouldMatch: true, + }, { title: "Queries route, bad query", route: new(Route).Queries("foo", "bar", "baz", "ding"), @@ -655,6 +677,8 @@ func testRoute(t *testing.T, test routeTest) { return } } + // Philip Schlump - Added to understand what match is returning. + // dumpVar ( match ) } } @@ -663,7 +687,7 @@ func testRoute(t *testing.T, test routeTest) { func TestKeepContext(t *testing.T) { func1 := func(w http.ResponseWriter, r *http.Request) {} - r := NewRouter() + r:= NewRouter() r.HandleFunc("/", func1).Name("func1") req, _ := http.NewRequest("GET", "http://localhost/", nil) @@ -688,6 +712,59 @@ func TestKeepContext(t *testing.T) { } + +type TestA301ResponseWriter struct { + hh http.Header + status int +} + +func (ho TestA301ResponseWriter) Header() http.Header { + // fmt.Printf ( "Header() called\n" ); + return http.Header(ho.hh) +} + +func (ho TestA301ResponseWriter) Write( b []byte) (int, error) { + // fmt.Printf ( "Write called %v\n", b ); + return 0, nil +} + +func (ho TestA301ResponseWriter) WriteHeader( code int ) { + // fmt.Printf ( "WriteHeader called code=%d\n", code ); + ho.status = code +} + +func Test301Redirect(t *testing.T) { + m := make(http.Header) + + func1 := func(w http.ResponseWriter, r *http.Request) {} + func2 := func(w http.ResponseWriter, r *http.Request) {} + + r:= NewRouter() + r.HandleFunc("/api/", func2).Name("func2") + r.HandleFunc("/", func1).Name("func1") + + req, _ := http.NewRequest("GET", "http://localhost//api/?abc=def", nil) + + res := TestA301ResponseWriter{ + hh: m, + status : 0, + } + r.ServeHTTP(&res, req) + + //fmt.Printf ( "This one %v\n", res ); + //fmt.Printf ( "res[\"Location\"] = ///%v///\n", res.hh["Location"] ); + //fmt.Printf ( "res[\"Location\"][0] = ///%v///\n", res.hh["Location"][0] ); + if "http://localhost/api/?abc=def" != res.hh["Location"][0] { + t.Errorf("Should have complete URL with query string") + } + // OK - I don't understand why this check on "status is not working. + // (p.s. the real answer is I am still learning go) + //if 301 != res.status { + // t.Errorf("Should have status of 301, got %d", res.status ) + //} + //fmt.Printf ( "Done\n" ); + +} // https://plus.google.com/101022900381697718949/posts/eWy6DjFJ6uW func TestSubrouterHeader(t *testing.T) { expected := "func1 response" From 6689ee8243e28381f38fff95d65c84a575fa86f8 Mon Sep 17 00:00:00 2001 From: Philip Schlump Date: Mon, 18 Nov 2013 10:54:45 -0700 Subject: [PATCH 3/4] Cleaned up testing. --- mux_test.go | 30 ++---------------------------- 1 file changed, 2 insertions(+), 28 deletions(-) diff --git a/mux_test.go b/mux_test.go index 4cbd55d..35f0037 100644 --- a/mux_test.go +++ b/mux_test.go @@ -8,7 +8,6 @@ import ( "fmt" "net/http" "testing" -// "encoding/json" // Philip Schlump "github.com/gorilla/context" ) @@ -23,17 +22,6 @@ type routeTest struct { shouldMatch bool // whether the request is expected to match the route at all } -// Philip Schlump - added to understand the results from tests. -//func dumpVar ( v interface{} ) { -// // s, err := json.Marshal ( v ) -// s, err := json.MarshalIndent ( v, "", "\t" ) -// if ( err != nil ) { -// fmt.Printf ( "Error: %s\n", err ) -// } else { -// fmt.Printf ( "%s\n", s ) -// } -//} - func TestHost(t *testing.T) { // newRequestHost a new request with a method, url, and host header @@ -430,7 +418,7 @@ func TestQueries(t *testing.T) { shouldMatch: true, }, { - title: "Queries route, match (ToDo - with redirect 301) (Philip Schlump added)", + title: "Queries route, match with a query string", route: new(Route).Host("www.2cwhy.com").Path("/api").Queries("foo", "bar", "baz", "ding"), request: newRequest("GET", "http://www.2cwhy.com/api?foo=bar&baz=ding"), vars: map[string]string{}, @@ -677,8 +665,6 @@ func testRoute(t *testing.T, test routeTest) { return } } - // Philip Schlump - Added to understand what match is returning. - // dumpVar ( match ) } } @@ -719,17 +705,14 @@ type TestA301ResponseWriter struct { } func (ho TestA301ResponseWriter) Header() http.Header { - // fmt.Printf ( "Header() called\n" ); return http.Header(ho.hh) } func (ho TestA301ResponseWriter) Write( b []byte) (int, error) { - // fmt.Printf ( "Write called %v\n", b ); return 0, nil } func (ho TestA301ResponseWriter) WriteHeader( code int ) { - // fmt.Printf ( "WriteHeader called code=%d\n", code ); ho.status = code } @@ -751,20 +734,11 @@ func Test301Redirect(t *testing.T) { } r.ServeHTTP(&res, req) - //fmt.Printf ( "This one %v\n", res ); - //fmt.Printf ( "res[\"Location\"] = ///%v///\n", res.hh["Location"] ); - //fmt.Printf ( "res[\"Location\"][0] = ///%v///\n", res.hh["Location"][0] ); if "http://localhost/api/?abc=def" != res.hh["Location"][0] { t.Errorf("Should have complete URL with query string") } - // OK - I don't understand why this check on "status is not working. - // (p.s. the real answer is I am still learning go) - //if 301 != res.status { - // t.Errorf("Should have status of 301, got %d", res.status ) - //} - //fmt.Printf ( "Done\n" ); - } + // https://plus.google.com/101022900381697718949/posts/eWy6DjFJ6uW func TestSubrouterHeader(t *testing.T) { expected := "func1 response" From ab8ae247f17a9f1bbe9bda321aa6719600eb4775 Mon Sep 17 00:00:00 2001 From: Philip Schlump Date: Mon, 18 Nov 2013 11:53:02 -0700 Subject: [PATCH 4/4] Fixed the domain name copied from my other code to be example.com --- mux_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/mux_test.go b/mux_test.go index 35f0037..1a2a092 100644 --- a/mux_test.go +++ b/mux_test.go @@ -419,8 +419,8 @@ func TestQueries(t *testing.T) { }, { title: "Queries route, match with a query string", - route: new(Route).Host("www.2cwhy.com").Path("/api").Queries("foo", "bar", "baz", "ding"), - request: newRequest("GET", "http://www.2cwhy.com/api?foo=bar&baz=ding"), + route: new(Route).Host("www.example.com").Path("/api").Queries("foo", "bar", "baz", "ding"), + request: newRequest("GET", "http://www.example.com/api?foo=bar&baz=ding"), vars: map[string]string{}, host: "", path: "",