0

I've built a function in my .bashrc that breaks when it tries to scp files with spaces in their names, but if I run the generated command output from the function on in the shell directly it seems to work fine.

I've tried escaping spaces, and several variations of single and double quotes, the version below is the closest I've gotten to working and I don't understand why it fails.

From .bashrc

push2() {
    # parse args, build file list, get suffix from final arg
    files=""
    i=1
    orig_IFS=$IFS; IFS=":"
    for arg in $*; do
        if [ "$i" = "$#" ]; then
            suffix=$arg
        else
            files="$files $(echo $arg | sed -r 's/ /\\ /')" #escape spaces
        fi
        i=$(($i+1))
    done
    IFS=$orig_IFS
    # determine prefix and proxy
    gen_prefix $suffix
    # output generated command for debugging
    echo "scp $scp_proxy$files testuser@$prefix$suffix:"
    # run command
    scp $scp_proxy$files testuser@$prefix$suffix:
}

Running the function still seems to fail even though the output command string appears properly escaped

root@DHCP-137:~$ push2 temp* 42
scp  temp temp\ file testuser@10.3.3.42:
temp                                          100% 6008     1.1MB/s   00:00 
temp\: No such file or directory
file: No such file or directory

Running the command it generates works as expected

root@DHCP-137:~$ scp  temp temp\ file testuser@10.3.3.42:
temp                                          100% 6008   896.4KB/s   00:00 
temp file                                     100%    0     0.0KB/s   00:00 
root@DHCP-137:~$ 

Additional Info: GNU bash, version 4.4.12(1)-release (x86_64-pc-linux-gnu) - running on Debian 9

RyanArr
  • 73
  • 6
  • 2
    The shell parses quotes and escapes before expanding variables, so any quotes or escapes embedded in the variables' values never get parsed and applied. See [this](https://stackoverflow.com/questions/12136948/why-does-shell-ignore-quotes-in-arguments-passed-to-it-through-variables). Always put double-quotes around variable references, use `"$@"` instead of `$*`, and to store multiple arguments in a variable, use an array and expand it with `"${arrayname[@]}"`. – Gordon Davisson Dec 19 '18 at 17:24
  • What does `scp_proxy` look like when set? (None of your examples appear to do so.) You *should* be able to do something as simple as `suffix=$1; scp "$@" testuser@$prefix$suffix`, with some minor additions based on what `gen_prefix` actually does, if you change your function to take the suffix *first* instead of *last*. – chepner Dec 19 '18 at 17:36
  • $scp_proxy looks like "-o ProxyJump=user@server", depending on what subnet I'm pushing to. In this example I'm staying on my local subnet so it's just blank. Thanks, I'm new to arrays so I'm working on figuring out how to pass them to scp as an argument list now. – RyanArr Dec 19 '18 at 17:43
  • OK, then since `gen_prefix` is setting a global variable, it can just as easily set a global array variable. See my answer once I post it. – chepner Dec 19 '18 at 17:44

2 Answers2

4

First, change your calling signature so that the suffix comes first:

push2 42 ./temp*

Then the function should be defined simply as

push2 () {
  local -a scpProxy
  local prefix suffix
  suffix=$1
  shift

  gen_prefix "$suffix"

  scp "${scpProxy[@]}" "$@" "testuser@$prefix.$suffix:"
}

where gen_prefix looks something like

gen_prefix () {
  case $1 in
     42) scpProxy=()
         prefix=10.3.3
         ;;
     89) scpProxy=(-o ProxyJump=user@server)
         prefix=123.456.789
         ;;
  esac
}

After calling shift, $@ contains just the files you want to transfer. scpProxy is an array that holds multiple individual arguments to pass to scp; if it is empty, then "${scpProxy[@]}" will expand to 0 arguments, not the empty string.

(Using ./temp* instead of temp* guards against matches that contain : and could thus be mistaken for a remote file name.)

Although gen_prefix appears to define its variables "globally", it's really just defining them in whatever scope gen_prefix is called from (bash uses dynamic scoping, not lexical scoping, like most other common languages). The two calls to local ensure that whatever gen_prefix assigns stays inside push2, and not visible after push2 exits.


As an additional note, much of this function can go away with a suitable ssh configuration. Consider this in your .ssh/config file:

Host somehost
    User testuser
    Hostname 10.3.3.42

Host someotherhost
    User testuser
    Hostname 123.456.789.89
    ProxyJump user@server

Now you don't need push2 at all; just run

scp temp* somehost:

or

scp temp* someotherhost:

and the correct addresses and options will be used automatically. The ssh configuration replaces everything gen_prefix did, and without the need to call gen_prefix, there's no longer any need to wrap scp.

chepner
  • 497,756
  • 71
  • 530
  • 681
  • 2
    If the suffix really needs to be last, you can use `suffix=${!#}`, remove the `shift`, and then use `"${@:1:$#-1}"` in the arg list for `scp`. But, yeah, putting it first is cleaner. – Gordon Davisson Dec 19 '18 at 17:54
  • Thanks, I had no idea about `shift`. This is much cleaner. Works with some minor tweaks to fit my `$prefix` and `$suffix` output format. – RyanArr Dec 19 '18 at 17:59
  • Be sure to see the updated answer, which suggests you don't need a wrapper around `scp` at all. – chepner Dec 19 '18 at 18:02
  • Another advantage of the .ssh/config file option is that it works using `scp` to receive files as well as send them, *and* it works with `ssh` itself, and `sftp`, and `rsync`-over-`ssh`, and... – Gordon Davisson Jan 02 '19 at 10:39
-1

The whole thing was fixed by changing the last line

scp $scp_proxy$files testuser@$prefix$suffix:

and wrapping it in an eval like this

eval "scp $scp_proxy$files testuser@$prefix$suffix:"
RyanArr
  • 73
  • 6
  • 2
    Don't use `eval` -- it's an enormous bug magnet. Use arrays and proper quoting (*around* data, not *in* data) instead. – Gordon Davisson Dec 19 '18 at 17:25
  • `eval` has its place, but it's *usually* in th trash. Avoid when possible - and it's almost always possible. (...almost...) – Paul Hodges Dec 19 '18 at 17:53