Skip to content

Commit

Permalink
Merge pull request #9174 from gyuho/url-error
Browse files Browse the repository at this point in the history
clientv3: prevent wrong URLs to cluster APIs
  • Loading branch information
gyuho committed Jan 22, 2018
2 parents 202cc9a + c837e01 commit b590d52
Show file tree
Hide file tree
Showing 3 changed files with 45 additions and 0 deletions.
1 change: 1 addition & 0 deletions .words
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ mutex
prefetching
protobuf
prometheus
rafthttp
repin
rpc
serializable
Expand Down
11 changes: 11 additions & 0 deletions clientv3/cluster.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import (
"context"

pb "github.com/coreos/etcd/etcdserver/etcdserverpb"
"github.com/coreos/etcd/pkg/types"

"google.golang.org/grpc"
)
Expand Down Expand Up @@ -66,6 +67,11 @@ func NewClusterFromClusterClient(remote pb.ClusterClient, c *Client) Cluster {
}

func (c *cluster) MemberAdd(ctx context.Context, peerAddrs []string) (*MemberAddResponse, error) {
// fail-fast before panic in rafthttp
if _, err := types.NewURLs(peerAddrs); err != nil {
return nil, err
}

r := &pb.MemberAddRequest{PeerURLs: peerAddrs}
resp, err := c.remote.MemberAdd(ctx, r, c.callOpts...)
if err != nil {
Expand All @@ -84,6 +90,11 @@ func (c *cluster) MemberRemove(ctx context.Context, id uint64) (*MemberRemoveRes
}

func (c *cluster) MemberUpdate(ctx context.Context, id uint64, peerAddrs []string) (*MemberUpdateResponse, error) {
// fail-fast before panic in rafthttp
if _, err := types.NewURLs(peerAddrs); err != nil {
return nil, err
}

// it is safe to retry on update.
r := &pb.MemberUpdateRequest{ID: id, PeerURLs: peerAddrs}
resp, err := c.remote.MemberUpdate(ctx, r, c.callOpts...)
Expand Down
33 changes: 33 additions & 0 deletions clientv3/integration/cluster_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -126,3 +126,36 @@ func TestMemberUpdate(t *testing.T) {
t.Errorf("urls = %v, want %v", urls, resp.Members[0].PeerURLs)
}
}

func TestMemberAddUpdateWrongURLs(t *testing.T) {
defer testutil.AfterTest(t)

clus := integration.NewClusterV3(t, &integration.ClusterConfig{Size: 1})
defer clus.Terminate(t)

capi := clus.RandClient()
tt := [][]string{
// missing protocol scheme
{"://127.0.0.1:2379"},
// unsupported scheme
{"mailto://127.0.0.1:2379"},
// not conform to host:port
{"http://127.0.0.1"},
// contain a path
{"http://127.0.0.1:2379/path"},
// first path segment in URL cannot contain colon
{"127.0.0.1:1234"},
// URL scheme must be http, https, unix, or unixs
{"localhost:1234"},
}
for i := range tt {
_, err := capi.MemberAdd(context.Background(), tt[i])
if err == nil {
t.Errorf("#%d: MemberAdd err = nil, but error", i)
}
_, err = capi.MemberUpdate(context.Background(), 0, tt[i])
if err == nil {
t.Errorf("#%d: MemberUpdate err = nil, but error", i)
}
}
}

0 comments on commit b590d52

Please sign in to comment.