From bugs@openbsd.org: From: Ward Wouts <ward@wizeazz.nl> To: bugs@openbsd.org Subject: problem with sftp Date: Tue, 18 Mar 2003 10:26:03 +0100 Hello, I'm seeing a problem with sftp where it's not possible to transfer files with both 's and "s and spaces in the name. I think it's a problem with the way get_pathname in sftp-int.c works, but my C is nowhere near good enough to fix this. To reproduce try transfering a file named: this"is' asillyname with sftp. Ward
Created attachment 257 [details] Patch to fix problems fetching filenames with quotes in sftp
The above patch replaces strchr with strrchr so that the get_pathname will look for the last ' or " for the filepath and not the first " or ' it comes to. Test/Proof: (Current SFTP) sftp> dir silly""file''2 this"is' asillyname sftp> get "silly\"\"file\'\'2" Couldn't stat remote file: No such file or directory File "/home/jason/silly\" not found. sftp> get "thCouldn't stat remote file: No such file or directory File "/home/jason/this\" not found. is\"is\' asillyname" (Patched SFTP) sftp> dir silly""file''2 sftp> get "silly\"\"file\'\'2" silly""file''2 100% 3 0.4KB/s 00:00 Fetching /home/jason/this"is' asillyname to this"is' asillyname sftp> exit Only thing is that the filename seems to mess up the progress meter. This changed didn't seem to break any fetches and solves every " or ' problem I could throw at it.
Created attachment 258 [details] Test case showing results of patched sftp This is the results of sftp operations before and after the sftp patch. The originals I put into the comment came out all funny.
As we are currently discussion on irc this fails to work for get "foo" "bar" case. I think the strchr() is correct, just needs to be wrapped into a loop until you find (*end) != '\\'. - Ben
hm, maybe I should rewrite the line parser using lex/yacc
Or maybe I shouldn't make silly assumptions or have brain farts as to the syntax of get and put. Ben's got a better fix.
Created attachment 259 [details] More correct version I believe that this is more what we want. I've only lightly tested it, but it support get "foo\"bar" "dog" correctly. The logic is just to loop through and skip the char quot; if the character before it is a \.
This fails to handle cases like "foo\\" where the quote is preceded by a backslash but should not be escaped.
Created attachment 260 [details] Patch to fix escapes of escapes I think this *might* be the winner (?). It seems to pass all the following filename tests: test test test"\ "\ test"one test\ test\"2 test\\\"""' test\\\\ Can anyone come up with another test?
Created attachment 261 [details] Patch to fix escaping of quotes Ummm.. a little tired here and uploaded the wrong .patch file. This is the correct one that passes all of the following tests: test test test"\ "\ test"one test\ test\"2 test\\\"""' test\\\\
Comment on attachment 261 [details] Patch to fix escaping of quotes No valid in several cases.
Created attachment 264 [details] Parsing filenames surrounded in quotes. Please test this patch. Works for all above tests as well as doesn't break on non-terminated quotes and filenames ending with a \.
Created attachment 265 [details] Patch for bux - minor fixes Replaces check for " with quot for correct behavior.
Created attachment 299 [details] More concise patch Here is a more concise patch
Created attachment 302 [details] Concise & correct patch I just realised that we have never handled escape characters at all. This diff adds escaping for quotes and the backslash character.
patch applied.
Mass change of RESOLVED bugs to CLOSED