diff --git a/client/keys.go b/client/keys.go index 411cf44f7..064a78356 100644 --- a/client/keys.go +++ b/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 } diff --git a/client/keys_test.go b/client/keys_test.go index 1d00e6ba4..0dc05211c 100644 --- a/client/keys_test.go +++ b/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 diff --git a/pkg/pathutil/path.go b/pkg/pathutil/path.go new file mode 100644 index 000000000..35d753d1f --- /dev/null +++ b/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 +} diff --git a/pkg/pathutil/path_test.go b/pkg/pathutil/path_test.go new file mode 100644 index 000000000..6d3d803cf --- /dev/null +++ b/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) + } + } +}