From e117f36e483b34d176af9c364726a76975436fa8 Mon Sep 17 00:00:00 2001 From: Yicheng Qin Date: Sat, 3 Oct 2015 15:08:58 -0700 Subject: [PATCH] pkg/types: fix unwanted unescape in NewURLsMap We use url.ParseQuery to parse names-to-urls string, but it has side effect that unescape the string. If the initial-cluster string has ipv6 which contains `%25`, it will unescape it to `%` and make further url parse failed. Fix it by modifiying the parse process. Go1.4 doesn't support literal IPv6 address w/ zone in URI(https://github.com/golang/go/issues/6530), so we only enable tests in Go1.5+. --- pkg/types/urlsmap.go | 34 +++++++++++++++++++++-------- pkg/types/urlsmap_go15_test.go | 40 ++++++++++++++++++++++++++++++++++ pkg/types/urlsmap_test.go | 30 +++++++++++++++++++++++++ 3 files changed, 95 insertions(+), 9 deletions(-) create mode 100644 pkg/types/urlsmap_go15_test.go diff --git a/pkg/types/urlsmap.go b/pkg/types/urlsmap.go index a2aa7fe66..f7d5c1c90 100644 --- a/pkg/types/urlsmap.go +++ b/pkg/types/urlsmap.go @@ -16,7 +16,6 @@ package types import ( "fmt" - "net/url" "sort" "strings" ) @@ -27,15 +26,10 @@ type URLsMap map[string]URLs // which consists of discovery-formatted names-to-URLs, like: // mach0=http://1.1.1.1:2380,mach0=http://2.2.2.2::2380,mach1=http://3.3.3.3:2380,mach2=http://4.4.4.4:2380 func NewURLsMap(s string) (URLsMap, error) { + m := parse(s) + cl := URLsMap{} - v, err := url.ParseQuery(strings.Replace(s, ",", "&", -1)) - if err != nil { - return nil, err - } - for name, urls := range v { - if len(urls) == 0 || urls[0] == "" { - return nil, fmt.Errorf("empty URL given for %q", name) - } + for name, urls := range m { us, err := NewURLs(urls) if err != nil { return nil, err @@ -73,3 +67,25 @@ func (c URLsMap) URLs() []string { func (c URLsMap) Len() int { return len(c) } + +// parse parses the given string and returns a map listing the values specified for each key. +func parse(s string) map[string][]string { + m := make(map[string][]string) + for s != "" { + key := s + if i := strings.IndexAny(key, ","); i >= 0 { + key, s = key[:i], key[i+1:] + } else { + s = "" + } + if key == "" { + continue + } + value := "" + if i := strings.Index(key, "="); i >= 0 { + key, value = key[:i], key[i+1:] + } + m[key] = append(m[key], value) + } + return m +} diff --git a/pkg/types/urlsmap_go15_test.go b/pkg/types/urlsmap_go15_test.go new file mode 100644 index 000000000..1955bb9cb --- /dev/null +++ b/pkg/types/urlsmap_go15_test.go @@ -0,0 +1,40 @@ +// 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. + +// +build go1.5 + +package types + +import ( + "reflect" + "testing" + + "github.com/coreos/etcd/pkg/testutil" +) + +// This is only tested in Go1.5+ because Go1.4 doesn't support literal IPv6 address with zone in +// URI (https://github.com/golang/go/issues/6530). +func TestNewURLsMapIPV6(t *testing.T) { + c, err := NewURLsMap("mem1=http://[2001:db8::1]:2380,mem1=http://[fe80::6e40:8ff:feb1:58e4%25en0]:2380,mem2=http://[fe80::92e2:baff:fe7c:3224%25ext0]:2380") + if err != nil { + t.Fatalf("unexpected parse error: %v", err) + } + wc := URLsMap(map[string]URLs{ + "mem1": testutil.MustNewURLs(t, []string{"http://[2001:db8::1]:2380", "http://[fe80::6e40:8ff:feb1:58e4%25en0]:2380"}), + "mem2": testutil.MustNewURLs(t, []string{"http://[fe80::92e2:baff:fe7c:3224%25ext0]:2380"}), + }) + if !reflect.DeepEqual(c, wc) { + t.Errorf("cluster = %#v, want %#v", c, wc) + } +} diff --git a/pkg/types/urlsmap_test.go b/pkg/types/urlsmap_test.go index 8b52dc17b..a01b5efa9 100644 --- a/pkg/types/urlsmap_test.go +++ b/pkg/types/urlsmap_test.go @@ -67,3 +67,33 @@ func TestNameURLPairsString(t *testing.T) { t.Fatalf("NameURLPairs.String():\ngot %#v\nwant %#v", g, w) } } + +func TestParse(t *testing.T) { + tests := []struct { + s string + wm map[string][]string + }{ + { + "", + map[string][]string{}, + }, + { + "a=b", + map[string][]string{"a": {"b"}}, + }, + { + "a=b,a=c", + map[string][]string{"a": {"b", "c"}}, + }, + { + "a=b,a1=c", + map[string][]string{"a": {"b"}, "a1": {"c"}}, + }, + } + for i, tt := range tests { + m := parse(tt.s) + if !reflect.DeepEqual(m, tt.wm) { + t.Errorf("#%d: m = %+v, want %+v", i, m, tt.wm) + } + } +}