Skip to content

Commit

Permalink
Replace go-shellwords with own shellescape.Split
Browse files Browse the repository at this point in the history
Signed-off-by: Kimmo Lehto <[email protected]>
  • Loading branch information
kke committed Mar 11, 2024
1 parent eb09155 commit 81de8f1
Show file tree
Hide file tree
Showing 8 changed files with 152 additions and 8 deletions.
1 change: 0 additions & 1 deletion go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ require (
github.com/davidmz/go-pageant v1.0.2
github.com/kevinburke/ssh_config v1.2.0
github.com/masterzen/winrm v0.0.0-20231128182143-52a9e15d5730
github.com/mattn/go-shellwords v1.0.12
github.com/stretchr/testify v1.9.0
golang.org/x/crypto v0.21.0
golang.org/x/term v0.18.0
Expand Down
2 changes: 0 additions & 2 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -50,8 +50,6 @@ github.com/masterzen/simplexml v0.0.0-20190410153822-31eea3082786 h1:2ZKn+w/BJeL
github.com/masterzen/simplexml v0.0.0-20190410153822-31eea3082786/go.mod h1:kCEbxUJlNDEBNbdQMkPSp6yaKcRXVI6f4ddk8Riv4bc=
github.com/masterzen/winrm v0.0.0-20231128182143-52a9e15d5730 h1:zLVOWGRxX/IsRpqHjl0hjVq6BORcs7ubih+G2dGhTEs=
github.com/masterzen/winrm v0.0.0-20231128182143-52a9e15d5730/go.mod h1:qfAjztAGRm7J7Ci10OA9vrx8WRDM0mlhdsFu7gBtMK8=
github.com/mattn/go-shellwords v1.0.12 h1:M2zGm7EW6UQJvDeQxo4T51eKPurbeFbe8WtebGE2xrk=
github.com/mattn/go-shellwords v1.0.12/go.mod h1:EZzvwXDESEeg03EKmM+RmDnNOPKG4lLtQsUlTZDWQ8Y=
github.com/pmezard/go-difflib v1.0.0 h1:4DBwDE0NGyQoBHbLQYPwSUPoCMWR5BEzIk/f1lZbAQM=
github.com/pmezard/go-difflib v1.0.0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4=
github.com/stretchr/objx v0.1.0/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+wExME=
Expand Down
5 changes: 2 additions & 3 deletions protocol/localhost/connection.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ import (
"runtime"

"github.com/k0sproject/rig/v2/protocol"
"github.com/mattn/go-shellwords"
"github.com/k0sproject/rig/v2/sh/shellescape"
)

// Connection is a direct localhost connection.
Expand Down Expand Up @@ -132,8 +132,7 @@ func (c *Connection) ExecInteractive(cmd string, stdin io.Reader, stdout, stderr
Dir: cwd,
}

// TODO shellwords local implementation (sh/shellescape.Split)
parts, err := shellwords.Parse(cmd)
parts, err := shellescape.Split(cmd)
if err != nil {
return fmt.Errorf("failed to parse command: %w", err)
}
Expand Down
4 changes: 2 additions & 2 deletions protocol/ssh/connection.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,8 @@ import (
"github.com/k0sproject/rig/v2/protocol"
"github.com/k0sproject/rig/v2/protocol/ssh/agent"
"github.com/k0sproject/rig/v2/protocol/ssh/hostkey"
"github.com/k0sproject/rig/v2/sh/shellescape"
"github.com/kevinburke/ssh_config"
"github.com/mattn/go-shellwords"
ssh "golang.org/x/crypto/ssh"
"golang.org/x/term"
)
Expand Down Expand Up @@ -315,7 +315,7 @@ func (c *Connection) hostkeyCallback() (ssh.HostKeyCallback, error) {
kfs := c.getConfigAll("UserKnownHostsFile")
// splitting the result as for some reason ssh_config sometimes seems to
// return a single string containing space separated paths
if files, err := shellwords.Parse(strings.Join(kfs, " ")); err == nil {
if files, err := shellescape.Split(strings.Join(kfs, " ")); err == nil {
for _, f := range files {
log.Trace(context.Background(), "trying known_hosts file from ssh config", log.KeyHost, c, log.KeyFile, f)
exp, err := homedir.Expand(f)
Expand Down
60 changes: 60 additions & 0 deletions sh/shellescape/split.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,60 @@
package shellescape

import (
"fmt"
"strings"
)

// Split splits the input string respecting shell-like quoted segments.
func Split(input string) ([]string, error) { //nolint:cyclop
var segments []string

currentSegment, ok := builderPool.Get().(*strings.Builder)
if !ok {
currentSegment = &strings.Builder{}
}
defer builderPool.Put(currentSegment)
defer currentSegment.Reset()

var inDoubleQuotes, inSingleQuotes, isEscaped bool

for i := 0; i < len(input); i++ {
currentChar := input[i]

if isEscaped {
currentSegment.WriteByte(currentChar)
isEscaped = false
continue
}

switch {
case currentChar == '\\' && !inSingleQuotes:
isEscaped = true
case currentChar == '"' && !inSingleQuotes:
inDoubleQuotes = !inDoubleQuotes
case currentChar == '\'' && !inDoubleQuotes:
inSingleQuotes = !inSingleQuotes
case currentChar == ' ' && !inDoubleQuotes && !inSingleQuotes:
// Space outside quotes; delimiter for a new segment
segments = append(segments, currentSegment.String())
currentSegment.Reset()
default:
currentSegment.WriteByte(currentChar)
}
}

if inDoubleQuotes || inSingleQuotes {
return nil, fmt.Errorf("split `%q`: %w", input, ErrMismatchedQuotes)
}

if isEscaped {
return nil, fmt.Errorf("split `%q`: %w", input, ErrTrailingBackslash)
}

// Add the last segment if present
if currentSegment.Len() > 0 {
segments = append(segments, currentSegment.String())
}

return segments, nil
}
75 changes: 75 additions & 0 deletions sh/shellescape/split_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,75 @@
package shellescape_test

import (
"testing"

"github.com/k0sproject/rig/v2/sh/shellescape"
"github.com/stretchr/testify/assert"
)

func TestSplit(t *testing.T) {
testCases := []struct {
name string
input string
wantOutput []string
wantErr bool
}{
{
name: "Simple Double Quotes Single Value",
input: `"value"`,
wantOutput: []string{"value"},
wantErr: false,
},
{
name: "Simple Unquoted Simple Value",
input: "value",
wantOutput: []string{"value"},
wantErr: false,
},
{
name: "Two unquoted values",
input: "value1 value2",
wantOutput: []string{"value1", "value2"},
wantErr: false,
},
{
name: "One unquoted and one single quoted value",
input: `value1 'value2 with quotes'`,
wantOutput: []string{"value1", "value2 with quotes"},
wantErr: false,
},
{
name: "One unquoted and one double quoted value",
input: `value1 "value2 with quotes"`,
wantOutput: []string{"value1", "value2 with quotes"},
wantErr: false,
},
{
name: "Double quoted value inside single quotes",
input: `value1 'value2 "with" quotes'`,
wantOutput: []string{"value1", `value2 "with" quotes`},
wantErr: false,
},
{
name: "Escaped Single Quote in Single Quotes",
input: `value1 'escaped \'single\' quote'`,
wantOutput: nil,
// expecting an error due to unmatched quotes, first \' is treated as a literal and ' closes the
// quotes. the second \ is treated as an escape for the ' and gets parsed as ' literal.
wantErr: true,
},
}

for _, tc := range testCases {
t.Run(tc.name, func(t *testing.T) {
gotOutput, err := shellescape.Split(tc.input)
if tc.wantErr {
assert.Nil(t, gotOutput, "Split(%q)", tc.input)
assert.Error(t, err, "Split(%q)", tc.input)
} else {
assert.NoError(t, err, "Split(%q)", tc.input)
assert.Equal(t, tc.wantOutput, gotOutput, "Split(%q)", tc.input)
}
})
}
}
7 changes: 7 additions & 0 deletions sh/shellescape/unquote.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,9 @@ var (

// ErrMismatchedQuotes is returned when the input string has mismatched quotes when unquoting.
ErrMismatchedQuotes = errors.New("mismatched quotes")

// ErrTrailingBackslash is returned when the input string ends with a trailing backslash.
ErrTrailingBackslash = errors.New("trailing backslash")
)

// Unquote is a mostly POSIX compliant implementation of unquoting a string the same way a shell would.
Expand Down Expand Up @@ -67,5 +70,9 @@ func Unquote(input string) (string, error) { //nolint:cyclop
return "", fmt.Errorf("unquote `%q`: %w", input, ErrMismatchedQuotes)
}

if isEscaped {
return "", fmt.Errorf("unquote `%q`: %w", input, ErrTrailingBackslash)
}

return sb.String(), nil
}
6 changes: 6 additions & 0 deletions sh/shellescape/unquote_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,12 @@ func TestUnquote(t *testing.T) {
wantOutput: `value "with" quotes`,
wantErr: false,
},
{
name: "Double quotes inside single quotes",
input: `'value "with" quotes'`,
wantOutput: `value "with" quotes`,
wantErr: false,
},
{
name: "Single Quoted And Non-Quoted",
input: `'value 'with' quotes'`,
Expand Down

0 comments on commit 81de8f1

Please sign in to comment.