Don't quote ~[username]/ in scp paths.

[gnachman]

In order for tilde expansion to work, a leading tilde and
any subsequent alphanumeric characters up to and including
the first slash must not be quoted.

This commit changes the quoting algorithm to forego quoting
the initial tilde (if present), any subsequent
alphanumerics, and the first slash. If the first character
is not a tilde then there is no change.

PATH       BEFORE       AFTER
~          '~'          ~
~user      '~user'      ~user
foo~bar    'foo~bar'    'foo~bar'
~/foo      '~/foo'      ~/'foo'
~ foo bar  '~ foo bar'  ~' foo bar'

See issue #349

[gnachman]

Not sure why Travis is failing. It seems not to have updated after I fixed the lint problem on line 231.

[willco007]

Just wondering if there was a reason you went with this approach over escaping whitespace and not using quotes at all which you talked about in the initial bug report.

[gnachman]

After taking a closer look at the implementation, I became concerned about introducing bugs because it's not clear exactly which characters besides whitespace would need to be escaped. My understanding is that the paths are evaluated by the shell, which means characters like ! would need special escaping for tcsh. I doubt anyone knows all the rules.

[gnachman]

Would you prefer I change it to use the method we talked about originally?

[willco007]

This is fine with me, anyone using SCP have a chance to test this patch out?

[gnachman]

gnachman/iTerm2@d1ccf10 applies this patch to the nightly build of iTerm2. I'll see if anyone hollers. I've tested it on my Ubuntu machine. Not sure how testing usually works on this project?

[tiennou]

I'll humbly point to #243, which is waiting for a merge, but has the necessary infrastructure (or at least easier to use). And I'll gladly take care of any additional testcases, as long as they are green 😉. Right now I've only added things I'm sure of and/or have been able to write non-bogus testcases for (writing the harness is also somewhat a WIP). There's also a ci/more-testcases branch in my fork which has the "tentative" ones (sftp, agent, and scp).

[gnachman]

@tiennou That's a big diff. If I merge that into my fork, how do I run the tests and where's the right place to add mine?

[tiennou]

Here's a few "documentation" points, sorry for the terseness :

There should be a libssh2_clar executable built if you perform a "vanilla" cmake (the test suite is built by default).
The test list is maintained by tests/generate.py, which should automatically refresh the caches clar uses (I'm mentioning it because sometimes Xcode misses updates. YMMV using other generators).
A clar test suite is a .c file under tests/testcases/$system/$suite.c, and consists of two (optional) initialize & cleanup callbacks to perform shared setup, and then any number of void test_$system__$testname(void) functions.
The filesystem location is used to construct "specifiers", eg. tests/testcases/agent/crashy/local.c becomes agent::crashy::local. You can pass that as -sagent::crashy::local::broken_test_against_thing, to select a specific test function. Use -x to yank suites.
See tests/libssh2_clar.h for support helpers (mostly managing Docker and setting up sessions correctly so the trace gets dumped on failures).

I just noticed I have some rebasing to do (a test was added for agent forwarding), so I'm currently having to bring in some of the helpers added afterwards to be able to support the new test.

[stale]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

[willco007]

@gnachman Any reports on this PR after landing it in iTerm?

[stale]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.