| Summary: | bad "put" arg parsing | ||
|---|---|---|---|
| Product: | Portable OpenSSH | Reporter: | Damien Miller <djm> |
| Component: | sftp | Assignee: | OpenSSH Bugzilla mailing list <openssh-bugs> |
| Status: | CLOSED FIXED | ||
| Severity: | normal | ||
| Priority: | P2 | ||
| Version: | -current | ||
| Hardware: | All | ||
| OS: | Linux | ||
| Attachments: | |||
|
Description
Damien Miller
2003-03-24 15:38:57 AEDT
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 |