Merge pull request #5340 from cli/cmbrose/scp-args

`gh cs cp` parses additional scp args
This commit is contained in:
Caleb Brose 2022-03-24 09:53:54 -05:00 committed by GitHub
commit 1b01733ce5
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
3 changed files with 140 additions and 58 deletions

View file

@ -29,30 +29,18 @@ func Shell(ctx context.Context, p printer, sshArgs []string, port int, destinati
return cmd.Run()
}
// Copy runs an scp command over the specified port. The arguments may
// include flags and non-flags, optionally separated by "--".
// Copy runs an scp command over the specified port. scpArgs should contain both scp flags
// as well as the list of files to copy, with the flags first.
//
// Remote files indicated by a "remote:" prefix are resolved relative
// to the remote user's home directory, and are subject to shell expansion
// on the remote host; see https://lwn.net/Articles/835962/.
func Copy(ctx context.Context, scpArgs []string, port int, destination string) error {
// Beware: invalid syntax causes scp to exit 1 with
// no error message, so don't let that happen.
cmd := exec.CommandContext(ctx, "scp",
"-P", strconv.Itoa(port),
"-o", "NoHostAuthenticationForLocalhost=yes",
"-C", // compression
)
for _, arg := range scpArgs {
// Replace "remote:" prefix with (e.g.) "root@localhost:".
if rest := strings.TrimPrefix(arg, "remote:"); rest != arg {
arg = destination + ":" + rest
}
cmd.Args = append(cmd.Args, arg)
cmd, err := newSCPCommand(ctx, port, destination, scpArgs)
if err != nil {
return fmt.Errorf("failed to create scp command: %w", err)
}
cmd.Stdin = nil
cmd.Stdout = os.Stderr
cmd.Stderr = os.Stderr
return cmd.Run()
}
@ -95,9 +83,54 @@ func newSSHCommand(ctx context.Context, port int, dst string, cmdArgs []string)
return cmd, connArgs, nil
}
// parseSSHArgs parses SSH arguments into two distinct slices of flags and command.
// It returns an error if a unary flag is provided without an argument.
func parseSSHArgs(args []string) (cmdArgs, command []string, err error) {
return parseArgs(args, "bcDeFIiLlmOopRSWw")
}
// newSCPCommand populates an exec.Cmd to run an scp command for the files specified in cmdArgs.
// cmdArgs is parsed such that scp flags precede the files to copy in the command.
// For example: scp -F ./config local/file remote:file
func newSCPCommand(ctx context.Context, port int, dst string, cmdArgs []string) (*exec.Cmd, error) {
connArgs := []string{
"-P", strconv.Itoa(port),
"-o", "NoHostAuthenticationForLocalhost=yes",
"-C", // compression
}
cmdArgs, command, err := parseSCPArgs(cmdArgs)
if err != nil {
return nil, err
}
cmdArgs = append(cmdArgs, connArgs...)
for _, arg := range command {
// Replace "remote:" prefix with (e.g.) "root@localhost:".
if rest := strings.TrimPrefix(arg, "remote:"); rest != arg {
arg = dst + ":" + rest
}
cmdArgs = append(cmdArgs, arg)
}
// Beware: invalid syntax causes scp to exit 1 with
// no error message, so don't let that happen.
cmd := exec.CommandContext(ctx, "scp", cmdArgs...)
cmd.Stdin = nil
cmd.Stdout = os.Stderr
cmd.Stderr = os.Stderr
return cmd, nil
}
func parseSCPArgs(args []string) (cmdArgs, command []string, err error) {
return parseArgs(args, "cFiJloPS")
}
// parseArgs parses arguments into two distinct slices of flags and command. Parsing stops
// as soon as a non-flag argument is found assuming the remaining arguments are the command.
// It returns an error if a unary flag is provided without an argument.
func parseArgs(args []string, unaryFlags string) (cmdArgs, command []string, err error) {
for i := 0; i < len(args); i++ {
arg := args[i]
@ -108,9 +141,9 @@ func parseSSHArgs(args []string) (cmdArgs, command []string, err error) {
}
cmdArgs = append(cmdArgs, arg)
if len(arg) == 2 && strings.Contains("bcDeFIiLlmOopRSWw", arg[1:2]) {
if len(arg) == 2 && strings.Contains(unaryFlags, arg[1:2]) {
if i++; i == len(args) {
return nil, nil, fmt.Errorf("ssh flag: %s requires an argument", arg)
return nil, nil, fmt.Errorf("flag: %s requires an argument", arg)
}
cmdArgs = append(cmdArgs, args[i])

View file

@ -5,15 +5,15 @@ import (
"testing"
)
func TestParseSSHArgs(t *testing.T) {
type testCase struct {
Args []string
ParsedArgs []string
Command []string
Error string
}
type parseTestCase struct {
Args []string
ParsedArgs []string
Command []string
Error string
}
testCases := []testCase{
func TestParseSSHArgs(t *testing.T) {
testCases := []parseTestCase{
{}, // empty test case
{
Args: []string{"-X", "-Y"},
@ -69,37 +69,85 @@ func TestParseSSHArgs(t *testing.T) {
Args: []string{"-b"},
ParsedArgs: nil,
Command: nil,
Error: "ssh flag: -b requires an argument",
Error: "flag: -b requires an argument",
},
}
for _, tcase := range testCases {
args, command, err := parseSSHArgs(tcase.Args)
if tcase.Error != "" {
if err == nil {
t.Errorf("expected error and got nil: %#v", tcase)
}
if err.Error() != tcase.Error {
t.Errorf("error does not match expected error, got: '%s', expected: '%s'", err.Error(), tcase.Error)
}
continue
}
if err != nil {
t.Errorf("unexpected error: %v on test case: %#v", err, tcase)
continue
}
argsStr, parsedArgsStr := fmt.Sprintf("%s", args), fmt.Sprintf("%s", tcase.ParsedArgs)
if argsStr != parsedArgsStr {
t.Errorf("args do not match parsed args. got: '%s', expected: '%s'", argsStr, parsedArgsStr)
}
commandStr, parsedCommandStr := fmt.Sprintf("%s", command), fmt.Sprintf("%s", tcase.Command)
if commandStr != parsedCommandStr {
t.Errorf("command does not match parsed command. got: '%s', expected: '%s'", commandStr, parsedCommandStr)
}
checkParseResult(t, tcase, args, command, err)
}
}
func TestParseSCPArgs(t *testing.T) {
testCases := []parseTestCase{
{}, // empty test case
{
Args: []string{"-X", "-Y"},
ParsedArgs: []string{"-X", "-Y"},
Command: nil,
},
{
Args: []string{"-X", "-Y", "-o", "someoption=test"},
ParsedArgs: []string{"-X", "-Y", "-o", "someoption=test"},
Command: nil,
},
{
Args: []string{"-X", "-Y", "-o", "someoption=test", "local/file", "remote:file"},
ParsedArgs: []string{"-X", "-Y", "-o", "someoption=test"},
Command: []string{"local/file", "remote:file"},
},
{
Args: []string{"-X", "-Y", "-o", "someoption=test", "local/file", "remote:file"},
ParsedArgs: []string{"-X", "-Y", "-o", "someoption=test"},
Command: []string{"local/file", "remote:file"},
},
{
Args: []string{"local/file", "remote:file"},
ParsedArgs: []string{},
Command: []string{"local/file", "remote:file"},
},
{
Args: []string{"-c"},
ParsedArgs: nil,
Command: nil,
Error: "flag: -c requires an argument",
},
}
for _, tcase := range testCases {
args, command, err := parseSCPArgs(tcase.Args)
checkParseResult(t, tcase, args, command, err)
}
}
func checkParseResult(t *testing.T, tcase parseTestCase, gotArgs, gotCmd []string, gotErr error) {
if tcase.Error != "" {
if gotErr == nil {
t.Errorf("expected error and got nil: %#v", tcase)
}
if gotErr.Error() != tcase.Error {
t.Errorf("error does not match expected error, got: '%s', expected: '%s'", gotErr.Error(), tcase.Error)
}
return
}
if gotErr != nil {
t.Errorf("unexpected error: %v on test case: %#v", gotErr, tcase)
return
}
argsStr, parsedArgsStr := fmt.Sprintf("%s", gotArgs), fmt.Sprintf("%s", tcase.ParsedArgs)
if argsStr != parsedArgsStr {
t.Errorf("args do not match parsed args. got: '%s', expected: '%s'", argsStr, parsedArgsStr)
}
commandStr, parsedCommandStr := fmt.Sprintf("%s", gotCmd), fmt.Sprintf("%s", tcase.Command)
if commandStr != parsedCommandStr {
t.Errorf("command does not match parsed command. got: '%s', expected: '%s'", commandStr, parsedCommandStr)
}
}

View file

@ -343,7 +343,7 @@ func newCpCmd(app *App) *cobra.Command {
var opts cpOptions
cpCmd := &cobra.Command{
Use: "cp [-e] [-r] <sources>... <dest>",
Use: "cp [-e] [-r] [-- [<scp flags>...]] <sources>... <dest>",
Short: "Copy files between local and remote file systems",
Long: heredoc.Docf(`
The cp command copies files between the local and remote file systems.
@ -368,6 +368,7 @@ func newCpCmd(app *App) *cobra.Command {
$ gh codespace cp -e README.md 'remote:/workspaces/$RepositoryName/'
$ gh codespace cp -e 'remote:~/*.go' ./gofiles/
$ gh codespace cp -e 'remote:/workspaces/myproj/go.{mod,sum}' ./gofiles/
$ gh codespace cp -e -- -F ~/.ssh/codespaces_config 'remote:~/*.go' ./gofiles/
`),
RunE: func(cmd *cobra.Command, args []string) error {
return app.Copy(cmd.Context(), args, opts)
@ -392,7 +393,7 @@ func (a *App) Copy(ctx context.Context, args []string, opts cpOptions) error {
if opts.recursive {
opts.scpArgs = append(opts.scpArgs, "-r")
}
opts.scpArgs = append(opts.scpArgs, "--")
hasRemote := false
for _, arg := range args {
if rest := strings.TrimPrefix(arg, "remote:"); rest != arg {