Merge pull request #56 from raphael/query-vars

Fix "Queries" matcher to support out-of-order query string parameters

mux_test.go

@@ -462,6 +462,15 @@ func TestQueries(t *testing.T) {
 			path:        "",
 			shouldMatch: true,
 		},
+		{
+			title:       "Queries route, match with a query string out of order",
+			route:       new(Route).Host("www.example.com").Path("/api").Queries("foo", "bar", "baz", "ding"),
+			request:     newRequest("GET", "http://www.example.com/api?baz=ding&foo=bar"),
+			vars:        map[string]string{},
+			host:        "",
+			path:        "",
+			shouldMatch: true,
+		},
 		{
 			title:       "Queries route, bad query",
 			route:       new(Route).Queries("foo", "bar", "baz", "ding"),

regexp.go

@@ -34,7 +34,7 @@ func newRouteRegexp(tpl string, matchHost, matchPrefix, matchQuery, strictSlash
 	// Now let's parse it.
 	defaultPattern := "[^/]+"
 	if matchQuery {
-		defaultPattern = "[^?]+"
+		defaultPattern = "[^?&]+"
 		matchPrefix, strictSlash = true, false
 	} else if matchHost {
 		defaultPattern = "[^.]+"
@@ -51,9 +51,9 @@ func newRouteRegexp(tpl string, matchHost, matchPrefix, matchQuery, strictSlash
 	}
 	varsN := make([]string, len(idxs)/2)
 	varsR := make([]*regexp.Regexp, len(idxs)/2)
-	pattern := bytes.NewBufferString("^")
+	pattern := bytes.NewBufferString("")
-	if matchQuery {
+	if !matchQuery {
-		pattern = bytes.NewBufferString("")
+		pattern.WriteByte('^')
 	}
 	reverse := bytes.NewBufferString("")
 	var end int
@@ -209,9 +209,9 @@ func braceIndices(s string) ([]int, error) {
 
 // routeRegexpGroup groups the route matchers that carry variables.
 type routeRegexpGroup struct {
-	host  *routeRegexp
+	host    *routeRegexp
-	path  *routeRegexp
+	path    *routeRegexp
-	query *routeRegexp
+	queries []*routeRegexp
 }
 
 // setMatch extracts the variables from the URL once a route matches.
@@ -249,10 +249,11 @@ func (v *routeRegexpGroup) setMatch(req *http.Request, m *RouteMatch, r *Route)
 		}
 	}
 	// Store query string variables.
-	if v.query != nil {
+	rawQuery := req.URL.RawQuery
-		queryVars := v.query.regexp.FindStringSubmatch(req.URL.RawQuery)
+	for _, q := range v.queries {
+		queryVars := q.regexp.FindStringSubmatch(rawQuery)
 		if queryVars != nil {
-			for k, v := range v.query.varsN {
+			for k, v := range q.varsN {
 				m.Vars[v] = queryVars[k+1]
 			}
 		}

route.go

@@ -5,7 +5,6 @@
 package mux
 
 import (
-	"bytes"
 	"errors"
 	"fmt"
 	"net/http"
@@ -153,17 +152,17 @@ func (r *Route) addRegexpMatcher(tpl string, matchHost, matchPrefix, matchQuery
 	if err != nil {
 		return err
 	}
+	for _, q := range r.regexp.queries {
+		if err = uniqueVars(rr.varsN, q.varsN); err != nil {
+			return err
+		}
+	}
 	if matchHost {
 		if r.regexp.path != nil {
 			if err = uniqueVars(rr.varsN, r.regexp.path.varsN); err != nil {
 				return err
 			}
 		}
-		if r.regexp.query != nil {
-			if err = uniqueVars(rr.varsN, r.regexp.query.varsN); err != nil {
-				return err
-			}
-		}
 		r.regexp.host = rr
 	} else {
 		if r.regexp.host != nil {
@@ -172,18 +171,8 @@ func (r *Route) addRegexpMatcher(tpl string, matchHost, matchPrefix, matchQuery
 			}
 		}
 		if matchQuery {
-			if r.regexp.path != nil {
+			r.regexp.queries = append(r.regexp.queries, rr)
-				if err = uniqueVars(rr.varsN, r.regexp.path.varsN); err != nil {
-					return err
-				}
-			}
-			r.regexp.query = rr
 		} else {
-			if r.regexp.query != nil {
-				if err = uniqueVars(rr.varsN, r.regexp.query.varsN); err != nil {
-					return err
-				}
-			}
 			r.regexp.path = rr
 		}
 	}
@@ -345,12 +334,11 @@ func (r *Route) Queries(pairs ...string) *Route {
 			"mux: number of parameters must be multiple of 2, got %v", pairs)
 		return nil
 	}
-	var buf bytes.Buffer
 	for i := 0; i < length; i += 2 {
-		buf.WriteString(fmt.Sprintf("%s=%s&", pairs[i], pairs[i+1]))
+		if r.err = r.addRegexpMatcher(pairs[i]+"="+pairs[i+1], false, true, true); r.err != nil {
+			return r
+		}
 	}
-	tpl := strings.TrimRight(buf.String(), "&")
-	r.err = r.addRegexpMatcher(tpl, false, true, true)
 
 	return r
 }
@@ -527,9 +515,9 @@ func (r *Route) getRegexpGroup() *routeRegexpGroup {
 		} else {
 			// Copy.
 			r.regexp = &routeRegexpGroup{
-				host:  regexp.host,
+				host:    regexp.host,
-				path:  regexp.path,
+				path:    regexp.path,
-				query: regexp.query,
+				queries: regexp.queries,
 			}
 		}
 	}