Merge pull request #300 from aboch/qr

Pass proper regex to mux for query fields

libnetwork/api/api.go

@@ -22,20 +22,24 @@ var (
 
 const (
 	// Resource name regex
+	// Gorilla mux encloses the passed pattern with '^' and '$'. So we need to do some tricks
+	// to have mux eventually build a query regex which matches empty or word string (`^$|[\w]+`)
 	regex = "[a-zA-Z_0-9-]+"
+	qregx = "$|" + regex
 	// Router URL variable definition
-	nwName = "{" + urlNwName + ":" + regex + "}"
+	nwName   = "{" + urlNwName + ":" + regex + "}"
-	nwID   = "{" + urlNwID + ":" + regex + "}"
+	nwNameQr = "{" + urlNwName + ":" + qregx + "}"
-	nwPID  = "{" + urlNwPID + ":" + regex + "}"
+	nwID     = "{" + urlNwID + ":" + regex + "}"
-	epName = "{" + urlEpName + ":" + regex + "}"
+	nwPIDQr  = "{" + urlNwPID + ":" + qregx + "}"
-	epID   = "{" + urlEpID + ":" + regex + "}"
+	epName   = "{" + urlEpName + ":" + regex + "}"
-	epPID  = "{" + urlEpPID + ":" + regex + "}"
+	epNameQr = "{" + urlEpName + ":" + qregx + "}"
-	cnID   = "{" + urlCnID + ":" + regex + "}"
+	epID     = "{" + urlEpID + ":" + regex + "}"
+	epPIDQr  = "{" + urlEpPID + ":" + qregx + "}"
+	cnID     = "{" + urlCnID + ":" + regex + "}"
 
-	// Though this name can be anything, in order to support default network,
+	// Internal URL variable name.They can be anything as
-	// we will keep it as name
+	// long as they do not collide with query fields.
-	urlNwName = "name"
+	urlNwName = "network-name"
-	// Internal URL variable name, they can be anything
 	urlNwID   = "network-id"
 	urlNwPID  = "network-partial-id"
 	urlEpName = "endpoint-name"
@@ -89,17 +93,17 @@ func (h *httpHandler) initRouter() {
 	}{
 		"GET": {
 			// Order matters
-			{"/networks", []string{"name", nwName}, procGetNetworks},
+			{"/networks", []string{"name", nwNameQr}, procGetNetworks},
-			{"/networks", []string{"partial-id", nwPID}, procGetNetworks},
+			{"/networks", []string{"partial-id", nwPIDQr}, procGetNetworks},
 			{"/networks", nil, procGetNetworks},
 			{"/networks/" + nwID, nil, procGetNetwork},
-			{"/networks/" + nwID + "/endpoints", []string{"name", epName}, procGetEndpoints},
+			{"/networks/" + nwID + "/endpoints", []string{"name", epNameQr}, procGetEndpoints},
-			{"/networks/" + nwID + "/endpoints", []string{"partial-id", epPID}, procGetEndpoints},
+			{"/networks/" + nwID + "/endpoints", []string{"partial-id", epPIDQr}, procGetEndpoints},
 			{"/networks/" + nwID + "/endpoints", nil, procGetEndpoints},
 			{"/networks/" + nwID + "/endpoints/" + epID, nil, procGetEndpoint},
-			{"/services", []string{"network", nwName}, procGetServices},
+			{"/services", []string{"network", nwNameQr}, procGetServices},
-			{"/services", []string{"name", epName}, procGetServices},
+			{"/services", []string{"name", epNameQr}, procGetServices},
-			{"/services", []string{"partial-id", epPID}, procGetServices},
+			{"/services", []string{"partial-id", epPIDQr}, procGetServices},
 			{"/services", nil, procGetServices},
 			{"/services/" + epID, nil, procGetService},
 			{"/services/" + epID + "/backend", nil, procGetContainers},
@@ -150,22 +154,7 @@ func makeHandler(ctrl libnetwork.NetworkController, fct processor) http.HandlerF
 			}
 		}
 
-		mvars := mux.Vars(req)
+		res, rsp := fct(ctrl, mux.Vars(req), body)
-		rvars := req.URL.Query()
-		// workaround a mux issue which filters out valid queries with empty value
-		for k := range rvars {
-			if _, ok := mvars[k]; !ok {
-				if rvars.Get(k) == "" {
-					mvars[k] = ""
-				}
-			}
-		}
-
-		res, rsp := fct(ctrl, mvars, body)
-		if !rsp.isOK() {
-			http.Error(w, rsp.Status, rsp.StatusCode)
-			return
-		}
 		if res != nil {
 			writeJSON(w, rsp.StatusCode, res)
 		}

libnetwork/api/api_test.go

@@ -8,6 +8,7 @@ import (
 	"io"
 	"net/http"
 	"os"
+	"regexp"
 	"runtime"
 	"testing"
 
@@ -1818,6 +1819,23 @@ func TestEndToEnd(t *testing.T) {
 	}
 
 	// Query networks collection
+	req, err = http.NewRequest("GET", "/v1.19/networks?name=", nil)
+	if err != nil {
+		t.Fatal(err)
+	}
+	handleRequest(rsp, req)
+	if rsp.statusCode != http.StatusOK {
+		t.Fatalf("Expected StatusOK. Got (%d): %s", rsp.statusCode, rsp.body)
+	}
+	var list []*networkResource
+	err = json.Unmarshal(rsp.body, &list)
+	if err != nil {
+		t.Fatal(err)
+	}
+	if len(list) != 0 {
+		t.Fatalf("Expected empty list. Got %v", list)
+	}
+
 	req, err = http.NewRequest("GET", "/v1.19/networks", nil)
 	if err != nil {
 		t.Fatal(err)
@@ -1853,7 +1871,6 @@ func TestEndToEnd(t *testing.T) {
 		t.Fatalf("Expected StatusOK. Got (%d): %s", rsp.statusCode, rsp.body)
 	}
 
-	var list []*networkResource
 	err = json.Unmarshal(rsp.body, &list)
 	if err != nil {
 		t.Fatal(err)
@@ -2128,3 +2145,29 @@ func TestErrorConversion(t *testing.T) {
 		t.Fatalf("Failed to recognize not classified error as Internal error")
 	}
 }
+
+func TestFieldRegex(t *testing.T) {
+	pr := regexp.MustCompile(regex)
+	qr := regexp.MustCompile(`^` + qregx + `$`) // mux compiles it like this
+
+	if pr.MatchString("") {
+		t.Fatalf("Unexpected match")
+	}
+	if !qr.MatchString("") {
+		t.Fatalf("Unexpected match failure")
+	}
+
+	if pr.MatchString(":") {
+		t.Fatalf("Unexpected match")
+	}
+	if qr.MatchString(":") {
+		t.Fatalf("Unexpected match")
+	}
+
+	if pr.MatchString(".") {
+		t.Fatalf("Unexpected match")
+	}
+	if qr.MatchString(".") {
+		t.Fatalf("Unexpected match")
+	}
+}