Browse code

Pass proper regex to mux for query fields

- So that it will not discard empty query fields

Signed-off-by: Alessandro Boch <aboch@docker.com>

Alessandro Boch authored on 2015/06/16 16:13:54
Showing 2 changed files
... ...
@@ -22,20 +22,24 @@ var (
22 22
 
23 23
 const (
24 24
 	// Resource name regex
25
+	// Gorilla mux encloses the passed pattern with '^' and '$'. So we need to do some tricks
26
+	// to have mux eventually build a query regex which matches empty or word string (`^$|[\w]+`)
25 27
 	regex = "[a-zA-Z_0-9-]+"
28
+	qregx = "$|" + regex
26 29
 	// Router URL variable definition
27
-	nwName = "{" + urlNwName + ":" + regex + "}"
28
-	nwID   = "{" + urlNwID + ":" + regex + "}"
29
-	nwPID  = "{" + urlNwPID + ":" + regex + "}"
30
-	epName = "{" + urlEpName + ":" + regex + "}"
31
-	epID   = "{" + urlEpID + ":" + regex + "}"
32
-	epPID  = "{" + urlEpPID + ":" + regex + "}"
33
-	cnID   = "{" + urlCnID + ":" + regex + "}"
34
-
35
-	// Though this name can be anything, in order to support default network,
36
-	// we will keep it as name
37
-	urlNwName = "name"
38
-	// Internal URL variable name, they can be anything
30
+	nwName   = "{" + urlNwName + ":" + regex + "}"
31
+	nwNameQr = "{" + urlNwName + ":" + qregx + "}"
32
+	nwID     = "{" + urlNwID + ":" + regex + "}"
33
+	nwPIDQr  = "{" + urlNwPID + ":" + qregx + "}"
34
+	epName   = "{" + urlEpName + ":" + regex + "}"
35
+	epNameQr = "{" + urlEpName + ":" + qregx + "}"
36
+	epID     = "{" + urlEpID + ":" + regex + "}"
37
+	epPIDQr  = "{" + urlEpPID + ":" + qregx + "}"
38
+	cnID     = "{" + urlCnID + ":" + regex + "}"
39
+
40
+	// Internal URL variable name.They can be anything as
41
+	// long as they do not collide with query fields.
42
+	urlNwName = "network-name"
39 43
 	urlNwID   = "network-id"
40 44
 	urlNwPID  = "network-partial-id"
41 45
 	urlEpName = "endpoint-name"
... ...
@@ -89,17 +93,17 @@ func (h *httpHandler) initRouter() {
89 89
 	}{
90 90
 		"GET": {
91 91
 			// Order matters
92
-			{"/networks", []string{"name", nwName}, procGetNetworks},
93
-			{"/networks", []string{"partial-id", nwPID}, procGetNetworks},
92
+			{"/networks", []string{"name", nwNameQr}, procGetNetworks},
93
+			{"/networks", []string{"partial-id", nwPIDQr}, procGetNetworks},
94 94
 			{"/networks", nil, procGetNetworks},
95 95
 			{"/networks/" + nwID, nil, procGetNetwork},
96
-			{"/networks/" + nwID + "/endpoints", []string{"name", epName}, procGetEndpoints},
97
-			{"/networks/" + nwID + "/endpoints", []string{"partial-id", epPID}, procGetEndpoints},
96
+			{"/networks/" + nwID + "/endpoints", []string{"name", epNameQr}, procGetEndpoints},
97
+			{"/networks/" + nwID + "/endpoints", []string{"partial-id", epPIDQr}, procGetEndpoints},
98 98
 			{"/networks/" + nwID + "/endpoints", nil, procGetEndpoints},
99 99
 			{"/networks/" + nwID + "/endpoints/" + epID, nil, procGetEndpoint},
100
-			{"/services", []string{"network", nwName}, procGetServices},
101
-			{"/services", []string{"name", epName}, procGetServices},
102
-			{"/services", []string{"partial-id", epPID}, procGetServices},
100
+			{"/services", []string{"network", nwNameQr}, procGetServices},
101
+			{"/services", []string{"name", epNameQr}, procGetServices},
102
+			{"/services", []string{"partial-id", epPIDQr}, procGetServices},
103 103
 			{"/services", nil, procGetServices},
104 104
 			{"/services/" + epID, nil, procGetService},
105 105
 			{"/services/" + epID + "/backend", nil, procGetContainers},
... ...
@@ -150,22 +154,7 @@ func makeHandler(ctrl libnetwork.NetworkController, fct processor) http.HandlerF
150 150
 			}
151 151
 		}
152 152
 
153
-		mvars := mux.Vars(req)
154
-		rvars := req.URL.Query()
155
-		// workaround a mux issue which filters out valid queries with empty value
156
-		for k := range rvars {
157
-			if _, ok := mvars[k]; !ok {
158
-				if rvars.Get(k) == "" {
159
-					mvars[k] = ""
160
-				}
161
-			}
162
-		}
163
-
164
-		res, rsp := fct(ctrl, mvars, body)
165
-		if !rsp.isOK() {
166
-			http.Error(w, rsp.Status, rsp.StatusCode)
167
-			return
168
-		}
153
+		res, rsp := fct(ctrl, mux.Vars(req), body)
169 154
 		if res != nil {
170 155
 			writeJSON(w, rsp.StatusCode, res)
171 156
 		}
... ...
@@ -8,6 +8,7 @@ import (
8 8
 	"io"
9 9
 	"net/http"
10 10
 	"os"
11
+	"regexp"
11 12
 	"runtime"
12 13
 	"testing"
13 14
 
... ...
@@ -1818,6 +1819,23 @@ func TestEndToEnd(t *testing.T) {
1818 1818
 	}
1819 1819
 
1820 1820
 	// Query networks collection
1821
+	req, err = http.NewRequest("GET", "/v1.19/networks?name=", nil)
1822
+	if err != nil {
1823
+		t.Fatal(err)
1824
+	}
1825
+	handleRequest(rsp, req)
1826
+	if rsp.statusCode != http.StatusOK {
1827
+		t.Fatalf("Expected StatusOK. Got (%d): %s", rsp.statusCode, rsp.body)
1828
+	}
1829
+	var list []*networkResource
1830
+	err = json.Unmarshal(rsp.body, &list)
1831
+	if err != nil {
1832
+		t.Fatal(err)
1833
+	}
1834
+	if len(list) != 0 {
1835
+		t.Fatalf("Expected empty list. Got %v", list)
1836
+	}
1837
+
1821 1838
 	req, err = http.NewRequest("GET", "/v1.19/networks", nil)
1822 1839
 	if err != nil {
1823 1840
 		t.Fatal(err)
... ...
@@ -1853,7 +1871,6 @@ func TestEndToEnd(t *testing.T) {
1853 1853
 		t.Fatalf("Expected StatusOK. Got (%d): %s", rsp.statusCode, rsp.body)
1854 1854
 	}
1855 1855
 
1856
-	var list []*networkResource
1857 1856
 	err = json.Unmarshal(rsp.body, &list)
1858 1857
 	if err != nil {
1859 1858
 		t.Fatal(err)
... ...
@@ -2128,3 +2145,29 @@ func TestErrorConversion(t *testing.T) {
2128 2128
 		t.Fatalf("Failed to recognize not classified error as Internal error")
2129 2129
 	}
2130 2130
 }
2131
+
2132
+func TestFieldRegex(t *testing.T) {
2133
+	pr := regexp.MustCompile(regex)
2134
+	qr := regexp.MustCompile(`^` + qregx + `$`) // mux compiles it like this
2135
+
2136
+	if pr.MatchString("") {
2137
+		t.Fatalf("Unexpected match")
2138
+	}
2139
+	if !qr.MatchString("") {
2140
+		t.Fatalf("Unexpected match failure")
2141
+	}
2142
+
2143
+	if pr.MatchString(":") {
2144
+		t.Fatalf("Unexpected match")
2145
+	}
2146
+	if qr.MatchString(":") {
2147
+		t.Fatalf("Unexpected match")
2148
+	}
2149
+
2150
+	if pr.MatchString(".") {
2151
+		t.Fatalf("Unexpected match")
2152
+	}
2153
+	if qr.MatchString(".") {
2154
+		t.Fatalf("Unexpected match")
2155
+	}
2156
+}