client: use canonical url path in request

The main change is that it keeps the trailing slash. This helps
auth feature to judge path permission accurately.

client/keys.go

@@ -20,12 +20,12 @@ import (
 	"fmt"
 	"net/http"
 	"net/url"
-	"path"
 	"strconv"
 	"strings"
 	"time"
 
 	"github.com/coreos/etcd/Godeps/_workspace/src/golang.org/x/net/context"
+	"github.com/coreos/etcd/pkg/pathutil"
 )
 
 const (
@@ -445,7 +445,16 @@ func (hw *httpWatcher) Next(ctx context.Context) (*Response, error) {
 // provided endpoint's path to the root of the keys API
 // (typically "/v2/keys").
 func v2KeysURL(ep url.URL, prefix, key string) *url.URL {
-	ep.Path = path.Join(ep.Path, prefix, key)
+	// We concatenate all parts together manually. We cannot use
+	// path.Join because it does not reserve trailing slash.
+	// We call CanonicalURLPath to further cleanup the path.
+	if prefix != "" && prefix[0] != '/' {
+		prefix = "/" + prefix
+	}
+	if key != "" && key[0] != '/' {
+		key = "/" + key
+	}
+	ep.Path = pathutil.CanonicalURLPath(ep.Path + prefix + key)
 	return &ep
 }
 

client/keys_test.go

@@ -80,6 +80,20 @@ func TestV2KeysURLHelper(t *testing.T) {
 			key:      "/baz",
 			want:     url.URL{Scheme: "https", Host: "example.com", Path: "/foo/bar/baz"},
 		},
+		// Prefix is joined to path
+		{
+			endpoint: url.URL{Scheme: "https", Host: "example.com", Path: "/foo"},
+			prefix:   "/bar",
+			key:      "",
+			want:     url.URL{Scheme: "https", Host: "example.com", Path: "/foo/bar"},
+		},
+		// Keep trailing slash
+		{
+			endpoint: url.URL{Scheme: "https", Host: "example.com", Path: "/foo"},
+			prefix:   "/bar",
+			key:      "/baz/",
+			want:     url.URL{Scheme: "https", Host: "example.com", Path: "/foo/bar/baz/"},
+		},
 	}
 
 	for i, tt := range tests {
@@ -269,7 +283,7 @@ func TestSetAction(t *testing.T) {
 			act: setAction{
 				Key: "/foo/",
 			},
-			wantURL:  "http://example.com/foo",
+			wantURL:  "http://example.com/foo/",
 			wantBody: "value=",
 		},
 
@@ -460,7 +474,7 @@ func TestCreateInOrderAction(t *testing.T) {
 			act: createInOrderAction{
 				Dir: "/foo/",
 			},
-			wantURL:  "http://example.com/foo",
+			wantURL:  "http://example.com/foo/",
 			wantBody: "value=",
 		},
 
@@ -555,7 +569,7 @@ func TestDeleteAction(t *testing.T) {
 			act: deleteAction{
 				Key: "/foo/",
 			},
-			wantURL: "http://example.com/foo",
+			wantURL: "http://example.com/foo/",
 		},
 
 		// Recursive set to true

pkg/pathutil/path.go

@@ -0,0 +1,38 @@
+// Copyright 2015 CoreOS, Inc.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package pathutil
+
+import "path"
+
+// CanonicalURLPath returns the canonical url path for p, which follows the rules:
+// 1. the path always starts with "/"
+// 2. replace multiple slashes with a single slash
+// 3. replace each '.' '..' path name element with equivalent one
+// 4. keep the trailing slash
+func CanonicalURLPath(p string) string {
+	if p == "" {
+		return "/"
+	}
+	if p[0] != '/' {
+		p = "/" + p
+	}
+	np := path.Clean(p)
+	// path.Clean removes trailing slash except for root,
+	// put the trailing slash back if necessary.
+	if p[len(p)-1] == '/' && np != "/" {
+		np += "/"
+	}
+	return np
+}

pkg/pathutil/path_test.go

@@ -0,0 +1,38 @@
+// Copyright 2015 CoreOS, Inc.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package pathutil
+
+import "testing"
+
+func TestCanonicalURLPath(t *testing.T) {
+	tests := []struct {
+		p  string
+		wp string
+	}{
+		{"/a", "/a"},
+		{"", "/"},
+		{"a", "/a"},
+		{"//a", "/a"},
+		{"/a/.", "/a"},
+		{"/a/..", "/"},
+		{"/a/", "/a/"},
+		{"/a//", "/a/"},
+	}
+	for i, tt := range tests {
+		if g := CanonicalURLPath(tt.p); g != tt.wp {
+			t.Errorf("#%d: canonical path = %s, want %s", i, g, tt.wp)
+		}
+	}
+}