nassh: support quoting ssh arguments |
||
Issue descriptionwe should look at supporting quoting in the command line list. this isn't normally used, but can come up when doing something like: -o CanonicalDomains="domain another foo" it would also make things a bit more natural if someone wanted to do: -o "Compression yes" in this latter case, it's not a hard requirement though as you could write: -o Compression=yes this would be tricky for people who want to pass the quotes to the remote command: ls "/ a b c/" but on the upside, if we processed quotes all the time, we'd make comparing to native ssh easier as it would (largely) be the same when it comes to quote processing by the shell.
,
Aug 16 2017
The following revision refers to this bug: https://chromium.googlesource.com/apps/libapps/+/203d2bf3f8117195c488ba472032dfb717afc249 commit 203d2bf3f8117195c488ba472032dfb717afc249 Author: Mike Frysinger <vapier@chromium.org> Date: Wed Aug 16 23:28:49 2017 nassh: support quotes with ssh command line This allows for basic quote support. It works if the argument itself is entirely quoted, but does not work with nested quotes. If we want to go that route, we'll have to rewrite significantly (see the bug below for indepth discussion). Hopefully this simple form will work for most people's needs though. BUG= chromium:725625 Change-Id: Ic570ea9a3ff497f4dbf7632ea3530baa5e78c5e1 Reviewed-on: https://chromium-review.googlesource.com/612042 Reviewed-by: Brandon Gilmore <varz@google.com> Tested-by: Mike Frysinger <vapier@chromium.org> [modify] https://crrev.com/203d2bf3f8117195c488ba472032dfb717afc249/nassh/html/nassh_test.html [add] https://crrev.com/203d2bf3f8117195c488ba472032dfb717afc249/nassh/js/nassh_command_instance_tests.js [modify] https://crrev.com/203d2bf3f8117195c488ba472032dfb717afc249/nassh/js/nassh_command_instance.js
,
Aug 16 2017
lets ship with this for now and see how people feel/react. if it's sufficient, then that's that :).
,
Aug 17 2017
The following revision refers to this bug: https://chromium.googlesource.com/apps/libapps/+/7bcf456aa31d4c289db4b2ad8d4342ccf0c90f3b commit 7bcf456aa31d4c289db4b2ad8d4342ccf0c90f3b Author: Mike Frysinger <vapier@chromium.org> Date: Thu Aug 17 05:57:35 2017 nassh: fix typo in ssh command line parsing I was relying on the unittests too much. Oops. BUG= chromium:725625 Change-Id: I9db03bca16e7d96e1fab38c145c0e8d30e075946 Reviewed-on: https://chromium-review.googlesource.com/618149 Reviewed-by: Brandon Gilmore <varz@google.com> Tested-by: Mike Frysinger <vapier@chromium.org> [modify] https://crrev.com/7bcf456aa31d4c289db4b2ad8d4342ccf0c90f3b/nassh/js/nassh_command_instance.js |
||
►
Sign in to add a comment |
||
Comment 1 by vapier@chromium.org
, Aug 4 2017i think we can handle the quotes-for-the-remote easily ... if they specify a --, then we won't touch anything after that. but if they don't, we'll do quote processing. we need to decide how complete we want our processing to be. it'll impact the code complexity significantly now and isn't something we can easily take back later on once users start relying on it. today we split on whitespace. very simple, but also limited (hence this bug). the next step up is split on whitespace, except whitespace inside of double quotes. this can be accomplished with a single regex and some post processing logic. '-4 -o "foo bar" -- ls "a b"'.match(/("[^"]*"|\S+)/g) ["-4", "-o", ""foo bar"", "--", "ls", ""a b""] so we'd have to walk each element and strip off the outer quotes. then when we hit the --, we merge back the remaining elements as the remote command. this means we'll end up normalizing unquoted whitespace after the --, but not a huge deal. but think a bit harder and you'll see this doesn't handle edge cases users might be expecting: '-o"foo bar"blah' -> ['-o"foo', 'bar"', 'blah'] it also doesn't support nested double quotes (escape them?), and doesn't support single quotes. so we tweak it a bit: match(/([^"\s]*"[^"]*"[^"\s]*|\S+)/g) but that starts freaking out with slightly more pathological scenarios: '-o"foo bar"blah" cow"' -> ['-o"foo bar"blah", '" cow"'] and it ignores the post processing dequoting step (i guess we'd just strip all double quotes?). regex in this case quickly starts breaking down ... it's simply not a lexer for producing tokens (not a complaint, just a statement of fact). so our choices: - split on whitespace (today's status quo) - split on whitespace (unless double quoted) via regex - implement a proper lexer (support double quotes, escape sequences for nested quotes, single quotes, etc...) if we implement the regex step now, i don't think it supports any real world syntax we won't be able to undo in the future with the move to a proper lexer. there are some 3rd party projects out there of varying complexity/correctness. the key is to search for things like "split argv" rather than "process command line" as the latter gets you things like getopt() for processing an existing argv. https://github.com/kaelzhang/node-argv-split