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 <[email protected]>
  • Loading branch information
@gyuho
gyuho committed May 3, 2018
1 parent cac6ed6 commit d974e7a
Show file tree
Hide file tree
Showing 2 changed files with 428 additions and 188 deletions.
171 changes: 106 additions & 65 deletions etcdctl/ctlv3/command/watch_command.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -179,118 +179,159 @@ 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
watchPrefix, watchRev, watchPrevKey = false, 0, false
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
}
// 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
}
} else {
for execIdx = range watchArgs {
if watchArgs[execIdx] == "--" {
execExist = true
break
}
}
if execExist && execIdx == len(watchArgs)-1 {
// "watch foo bar --" should error
watchPrefix, watchRev, watchPrevKey = false, 0, false
return nil, nil, errBadArgsNumSeparator
}

// remove preceding commands (e.g. ./bin/etcdctl watch)
for idx = range osArgs {
if osArgs[idx] == "watch" {
break
flagset := NewWatchCommand().Flags()
if err := flagset.Parse(watchArgs); err != nil {
watchPrefix, watchRev, watchPrevKey = false, 0, false
return nil, nil, err
}
pArgs := flagset.Args()

// "watch" with no argument should error
if !execExist && envKey == "" && len(pArgs) < 1 {
watchPrefix, watchRev, watchPrevKey = false, 0, false
return nil, nil, errBadArgsNum
}
// check conflicting arguments
// e.g. "watch --rev 1 -- echo Hello World" has no conflict
if !execExist && len(pArgs) > 0 && envKey != "" {
// "ETCDCTL_WATCH_KEY=foo watch foo" should error
// (watchArgs==["foo"])
watchPrefix, watchRev, watchPrevKey = false, 0, false
return nil, nil, errBadArgsNumConflictEnv
}
}
if idx < len(osArgs)-1 {
osArgs = osArgs[idx+1:]
} else if envKey == "" {
return nil, nil, errBadArgsNum
}

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 {
return nil, nil, err
}
watchArgs = flagset.Args()

watchPrefix, err = flagset.GetBool("prefix")
if err != nil {
return nil, nil, err
}
watchRev, err = flagset.GetInt64("rev")
if err != nil {
return nil, nil, err
}
watchPrevKey, err = flagset.GetBool("prev-kv")
if err != nil {
return nil, nil, err
}
}

// "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
Loading

0 comments on commit d974e7a

Please sign in to comment.