#545: Use ssh ProxyCommand instead of multiple ssh commands --------------------------+------------------------------------------------- Reporter: eschnett | Owner: eschnett Type: enhancement | Status: review Priority: major | Milestone: Component: SimFactory | Version: Resolution: | Keywords: --------------------------+-------------------------------------------------

Comment (by eschnett):

The quote handling in this patch is off. Simfactory has a function QuoteSafe that quotes its argument, so that it can safely be passed to other commands such as bash or ssh. Instead of removing quotes or adding "" around a string, QuoteSafe should be used.

The current trampoline code is written recursively and can handle multiple levels of trampolines. I am not aware of a current machine that requires this, but it was necessary for the SiCortex test machine. If we wanted e.g. to access LONI compute nodes we would still need this. The patch seems to remove the recursion and will thus handle only a single level of trampolines. This may be fine, but there then needs to be a check to ensure that the trampoline does not require a trampoline. Can the ProxyCommand mechanism be nested? Do we want this functionality? It would be elegant, but does complicate things.

The current code is a bit convoluted, because the trampoline handling code is called both for ssh and for rsync commands. You implement this via an explicit switch (test if the argument is RSYNC); this may be a good idea, but an additional boolean argument may be a cleaner approach. Also, the current patch doesn't make it clear under which circumstances cmd would be empty or non-empty. I guess this is for "login" and "execute", respectively?

The nc command should be quoted with QuoteSafe instead of an explicit backslash.

#545: Use ssh ProxyCommand instead of multiple ssh commands --------------------------+------------------------------------------------- Reporter: eschnett | Owner: eschnett Type: enhancement | Status: review Priority: major | Milestone: Component: SimFactory | Version: Resolution: | Keywords: --------------------------+-------------------------------------------------

Comment (by eschnett):

All quoting in Simfactory (where a shell is involved) should be done via QuoteSafe. (This may currently not always be the case.) If there is a string surrounded by single quotes, then either someone "just added" them (and should have used QuoteSafe instead), or the were added via QuoteSafe, and then they don't need to be quoted again.

No, I didn't try it -- I trust you when you say it works for you. However, we need to define what corner cases we need to be treated correctly (e.g. multiple trampolines, having a remote source base directory with a space in the name, etc.). We currently don't need multiple trampolines, so we may want to drop them. Out of curiosity: How would a recursive trampolining command with ProxyCommand look like? Would the nc command be wrapped by an ssh command with another ProxyCommand?

The basic quoting rule is: whenever a command is constructed that will be passed to a shell, then each argument (except trivial ones) need to be quoted. For example, 'nc %h %p ...' is a command, so %h and %p should be quoted; however, if we assume that hostname and port number are reasonable, we can skip this. The whole nc command is an option to the ssh command, so it then needs to be quoted as a whole:

sshopts += '-o ProxyCommand=%s' % QuoteSafe(tsshcmd)

An empty string, when quoted, should not remain an empty string, it needs to become two consecutive quotes, so that (after passing it to a shell) it becomes an empty string again.

#545: Use ssh ProxyCommand instead of multiple ssh commands --------------------------+------------------------------------------------- Reporter: eschnett | Owner: eschnett Type: enhancement | Status: review Priority: major | Milestone: Component: SimFactory | Version: Resolution: | Keywords: --------------------------+-------------------------------------------------

Comment (by hinder):

At the operating system level, new processes are started via execp (and variants). A process receives a list of arguments which are C strings - i.e. null-terminated. When launching a process from the shell, the user has to be able to make a syntactic distinction between the content of the argument and the separators indicating different arguments. So in addition to the need to quote characters which would otherwise be interpreted by the shell, there is also the need to group characters into the appropriate arguments. This is done using double or single quotes so that each shell "word" is passed to the process as a separate argument. As such, it is certainly possible for one of the arguments to a process to be an empty string.

For example (in Python for ease of use, but the same would apply to any language):

#!/usr/bin/env python import sys print sys.argv

MacBook-2:~ $ ./test.py 1 2 "" 3 ['./test.py', '1', '2', '', '3']

So I weigh in on the side of Erik and say that an empty string should be quoted so that it remains an empty string when it is unquoted. This is probably a special-case, but it's important to get QuoteSafe correct, as quoting is a nightmare, and is made doubly so when it doesn't work properly!

Questions:

* Is the quoting required dependent on which shell is being used?

* Is it possible to get the shell to quote a string itself?

* Is there a standard Python way to perform quoting? Answering my own question, Stack Overflow has this to say on the matter: http://stackoverflow.com/questions/967443/python-module-to-shellquote- unshellquote. Maybe we could just use the snippet provided?

#545: Use ssh ProxyCommand instead of multiple ssh commands --------------------------+------------------------------------------------- Reporter: eschnett | Owner: eschnett Type: enhancement | Status: review Priority: major | Milestone: Component: SimFactory | Version: Resolution: | Keywords: --------------------------+-------------------------------------------------

Comment (by eschnett):

Quoting is independent of the shell. It is also rather simple, because a shell does not interpret any characters as special within single quotes. The only thing left to do is to quote single quotes themselves; this can be done either by prepending a backslash, or by enclosing it in double quotes. Both become impossible to decipher if there are more than two levels of quoting.

One also needs to know that there needs to be an unquoted space to separate arguments. For example,

Hello' 'World

is a single argument. So, to quote in a single-quote-delimited string, one first has to end the string (single quote), then add a quoted quote (backslash - single quote), then begin the string again (single quote).

The general quoting rule is: 1. replace any single quote by ''' (single quote - backslash - single quote - single quote) 2. surround the string by single quotes

We could us a shellquote module for this, but the corresponding Python function is just two lines long, so this may be overkill.