Skip to content

Commit

Permalink
etcdctl/ctlv3: fix watch with exec commands
Browse files Browse the repository at this point in the history 
Following command was failing because the parser incorrectly
picks up the second "watch" string in exec command, thus
passing wrong exec commands.

```
ETCDCTL_API=3 ./bin/etcdctl watch aaa -- echo watch event received

panic: runtime error: slice bounds out of range

goroutine 1 [running]:
github.com/coreos/etcd/etcdctl/ctlv3/command.parseWatchArgs(0xc42002e080, 0x8, 0x8, 0xc420206a20, 0x5, 0x6, 0x0, 0x0, 0x0, 0x0, ...)
	/home/gyuho/go/src/github.com/coreos/etcd/etcdctl/ctlv3/command/watch_command.go:303 +0xbed
github.com/coreos/etcd/etcdctl/ctlv3/command.watchCommandFunc(0xc4202a7180, 0xc420206a20, 0x5, 0x6)
	/home/gyuho/go/src/github.com/coreos/etcd/etcdctl/ctlv3/command/watch_command.go:73 +0x11d
github.com/coreos/etcd/vendor/github.com/spf13/cobra.(*Command).execute(0xc4202a7180, 0xc420206960, 0x6, 0x6, 0xc4202a7180, 0xc420206960)
	/home/gyuho/go/src/github.com/coreos/etcd/vendor/github.com/spf13/cobra/command.go:766 +0x2c1
github.com/coreos/etcd/vendor/github.com/spf13/cobra.(*Command).ExecuteC(0x1363de0, 0xc420128638, 0xc420185e01, 0xc420185ee8)
	/home/gyuho/go/src/github.com/coreos/etcd/vendor/github.com/spf13/cobra/command.go:852 +0x30a
github.com/coreos/etcd/vendor/github.com/spf13/cobra.(*Command).Execute(0x1363de0, 0x0, 0x0)
	/home/gyuho/go/src/github.com/coreos/etcd/vendor/github.com/spf13/cobra/command.go:800 +0x2b
github.com/coreos/etcd/etcdctl/ctlv3.Start()
	/home/gyuho/go/src/github.com/coreos/etcd/etcdctl/ctlv3/ctl_nocov.go:25 +0x8e
main.main()
	/home/gyuho/go/src/github.com/coreos/etcd/etcdctl/main.go:40 +0x17b
```

Signed-off-by: Gyuho Lee <gyuhox@gmail.com>
  • Loading branch information
@gyuho
gyuho committed May 3, 2018
1 parent 62dfb89 commit 8189335
Show file tree
Hide file tree
Showing 2 changed files with 126 additions and 65 deletions.
143 changes: 78 additions & 65 deletions etcdctl/ctlv3/command/watch_command.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -179,89 +179,108 @@ func printWatchCh(c *clientv3.Client, ch clientv3.WatchChan, execArgs []string)
// "--" characters are invalid arguments for "spf13/cobra" library,
// so no need to handle such cases.
func parseWatchArgs(osArgs, commandArgs []string, envKey, envRange string, interactive bool) (watchArgs []string, execArgs []string, err error) {
watchArgs = commandArgs
rawArgs := make([]string, len(osArgs))
copy(rawArgs, osArgs)
watchArgs = make([]string, len(commandArgs))
copy(watchArgs, commandArgs)

// remove preceding commands (e.g. "watch foo bar" in interactive mode)
idx := 0
for idx = range watchArgs {
if watchArgs[idx] == "watch" {
// remove preceding commands (e.g. ./bin/etcdctl watch)
// handle "./bin/etcdctl watch foo -- echo watch event"
for idx := range rawArgs {
if rawArgs[idx] == "watch" {
rawArgs = rawArgs[idx+1:]
break
}
}
if idx < len(watchArgs)-1 || envKey != "" {
if idx < len(watchArgs)-1 {
watchArgs = watchArgs[idx+1:]

// remove preceding commands (e.g. "watch foo bar" in interactive mode)
// handle "./bin/etcdctl watch foo -- echo watch event"
if interactive {
if watchArgs[0] != "watch" {
// "watch" not found
return nil, nil, errBadArgsInteractiveWatch
}
watchArgs = watchArgs[1:]
}

execIdx, execExist := 0, false
for execIdx = range osArgs {
v := osArgs[execIdx]
if v == "--" && execIdx != len(osArgs)-1 {
execIdx, execExist := 0, false
if !interactive {
for execIdx = range rawArgs {
if rawArgs[execIdx] == "--" {
execExist = true
break
}
}

if idx == len(watchArgs)-1 && envKey != "" {
if len(watchArgs) > 0 && !interactive {
// "watch --rev 1 -- echo Hello World" has no conflict
if !execExist {
// "watch foo" with ETCDCTL_WATCH_KEY=foo
// (watchArgs==["foo"])
return nil, nil, errBadArgsNumConflictEnv
}
}
// otherwise, watch with no argument and environment key is set
// if interactive, first "watch" command string should be removed
if interactive {
watchArgs = []string{}
}
if execExist && execIdx == len(rawArgs)-1 {
// "watch foo bar --" should error
return nil, nil, errBadArgsNumSeparator
}

// "watch foo -- echo hello" with ETCDCTL_WATCH_KEY=foo
// (watchArgs==["foo","echo","hello"])
if envKey != "" && execExist {
widx, oidx := 0, len(osArgs)-1
for widx = len(watchArgs) - 1; widx >= 0; widx-- {
if watchArgs[widx] == osArgs[oidx] {
oidx--
// "watch" with no argument should error
if !execExist && len(rawArgs) < 1 && envKey == "" {
return nil, nil, errBadArgsNum
}
if execExist && envKey != "" {
// "ETCDCTL_WATCH_KEY=foo watch foo -- echo 1" should error
// (watchArgs==["foo","echo","1"])
widx, ridx := len(watchArgs)-1, len(rawArgs)-1
for ; widx >= 0; widx-- {
if watchArgs[widx] == rawArgs[ridx] {
ridx--
continue
}
if oidx == execIdx { // watchArgs has extra
// watchArgs has extra:
// ETCDCTL_WATCH_KEY=foo watch foo -- echo 1
// watchArgs: foo echo 1
if ridx == execIdx {
return nil, nil, errBadArgsNumConflictEnv
}
}
}
} else if interactive { // "watch" not found
return nil, nil, errBadArgsInteractiveWatch
}
if len(watchArgs) < 1 && envKey == "" {
return nil, nil, errBadArgsNum
}

// remove preceding commands (e.g. ./bin/etcdctl watch)
for idx = range osArgs {
if osArgs[idx] == "watch" {
break
} else {
for execIdx = range watchArgs {
if watchArgs[execIdx] == "--" {
execExist = true
break
}
}
if execExist && execIdx == len(watchArgs)-1 {
// "watch foo bar --" should error
return nil, nil, errBadArgsNumSeparator
}
// "watch" with no argument should error
if !execExist && len(watchArgs) < 1 && envKey == "" {
return nil, nil, errBadArgsNum
}
if !execExist && envKey != "" {
watchArgs = []string{}
}
}
if idx < len(osArgs)-1 {
osArgs = osArgs[idx+1:]
} else if envKey == "" {
return nil, nil, errBadArgsNum

// check conflicting arguments
// e.g. "watch --rev 1 -- echo Hello World" has no conflict
if !execExist && len(watchArgs) > 0 && envKey != "" {
// "ETCDCTL_WATCH_KEY=foo watch foo" should error
// (watchArgs==["foo"])
return nil, nil, errBadArgsNumConflictEnv
}

argsWithSep := osArgs
if interactive { // interactive mode pass "--" to the command args
argsWithSep := rawArgs
if interactive {
// interactive mode directly passes "--" to the command args
argsWithSep = watchArgs
}
foundSep := false

idx, foundSep := 0, false
for idx = range argsWithSep {
if argsWithSep[idx] == "--" {
foundSep = true
break
}
}
if foundSep {
execArgs = argsWithSep[idx+1:]
}

if interactive {
flagset := NewWatchCommand().Flags()
if err := flagset.Parse(argsWithSep); err != nil {
Expand All @@ -270,27 +289,21 @@ func parseWatchArgs(osArgs, commandArgs []string, envKey, envRange string, inter
watchArgs = flagset.Args()
}

// "watch -- echo hello" with ETCDCTL_WATCH_KEY=foo
// should be translated to "watch foo -- echo hello"
// "ETCDCTL_WATCH_KEY=foo watch -- echo hello"
// should translate "watch foo -- echo hello"
// (watchArgs=["echo","hello"] should be ["foo","echo","hello"])
if envKey != "" {
tmp := []string{envKey}
ranges := []string{envKey}
if envRange != "" {
tmp = append(tmp, envRange)
ranges = append(ranges, envRange)
}
watchArgs = append(tmp, watchArgs...)
watchArgs = append(ranges, watchArgs...)
}

if !foundSep {
return watchArgs, nil, nil
}

if idx == len(argsWithSep)-1 {
// "watch foo bar --" should error
return nil, nil, errBadArgsNumSeparator
}
execArgs = argsWithSep[idx+1:]

// "watch foo bar --rev 1 -- echo hello" or "watch foo --rev 1 bar -- echo hello",
// then "watchArgs" is "foo bar echo hello"
// so need ignore args after "argsWithSep[idx]", which is "--"
Expand Down
48 changes: 48 additions & 0 deletions etcdctl/ctlv3/command/watch_command_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -145,6 +145,14 @@ func Test_parseWatchArgs(t *testing.T) {
execArgs: []string{"echo", "Hello", "World"},
err: nil,
},
{
osArgs: []string{"./bin/etcdctl", "watch", "foo", "--", "echo", "watch", "event", "received"},
commandArgs: []string{"foo", "echo", "watch", "event", "received"},
interactive: false,
watchArgs: []string{"foo"},
execArgs: []string{"echo", "watch", "event", "received"},
err: nil,
},
{
osArgs: []string{"./bin/etcdctl", "watch", "foo", "--rev", "1", "--", "echo", "Hello", "World"},
commandArgs: []string{"foo", "echo", "Hello", "World"},
Expand All @@ -153,6 +161,22 @@ func Test_parseWatchArgs(t *testing.T) {
execArgs: []string{"echo", "Hello", "World"},
err: nil,
},
{
osArgs: []string{"./bin/etcdctl", "watch", "foo", "--rev", "1", "--", "echo", "watch", "event", "received"},
commandArgs: []string{"foo", "echo", "watch", "event", "received"},
interactive: false,
watchArgs: []string{"foo"},
execArgs: []string{"echo", "watch", "event", "received"},
err: nil,
},
{
osArgs: []string{"./bin/etcdctl", "watch", "--rev", "1", "foo", "--", "echo", "watch", "event", "received"},
commandArgs: []string{"foo", "echo", "watch", "event", "received"},
interactive: false,
watchArgs: []string{"foo"},
execArgs: []string{"echo", "watch", "event", "received"},
err: nil,
},
{
osArgs: []string{"./bin/etcdctl", "watch", "foo", "bar", "--", "echo", "Hello", "World"},
commandArgs: []string{"foo", "bar", "echo", "Hello", "World"},
Expand Down Expand Up @@ -185,6 +209,30 @@ func Test_parseWatchArgs(t *testing.T) {
execArgs: []string{"echo", "Hello", "World"},
err: nil,
},
{
osArgs: []string{"./bin/etcdctl", "watch", "foo", "bar", "--rev", "1", "--", "echo", "watch", "event", "received"},
commandArgs: []string{"foo", "bar", "echo", "watch", "event", "received"},
interactive: false,
watchArgs: []string{"foo", "bar"},
execArgs: []string{"echo", "watch", "event", "received"},
err: nil,
},
{
osArgs: []string{"./bin/etcdctl", "watch", "foo", "--rev", "1", "bar", "--", "echo", "Hello", "World"},
commandArgs: []string{"foo", "bar", "echo", "Hello", "World"},
interactive: false,
watchArgs: []string{"foo", "bar"},
execArgs: []string{"echo", "Hello", "World"},
err: nil,
},
{
osArgs: []string{"./bin/etcdctl", "watch", "--rev", "1", "foo", "bar", "--", "echo", "Hello", "World"},
commandArgs: []string{"foo", "bar", "echo", "Hello", "World"},
interactive: false,
watchArgs: []string{"foo", "bar"},
execArgs: []string{"echo", "Hello", "World"},
err: nil,
},
{
osArgs: []string{"./bin/etcdctl", "watch", "--rev", "1", "--", "echo", "Hello", "World"},
commandArgs: []string{"echo", "Hello", "World"},
Expand Down

0 comments on commit 8189335

Please sign in to comment.