lightning-cli: don't produce bad JSON if fields contain ".

The user can explicitly create such things (within [] or ") as we paste those cases literally, but not for the simple cases. Fixes: ElementsProject#2550 Signed-off-by: Rusty Russell <[email protected]>
sha-265 · Apr 11, 2019 · 77b859e · 77b859e
1 parent 27afc80
commit 77b859e
Show file tree

Hide file tree

Showing 6 changed files with 27 additions and 8 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -30,6 +30,7 @@ changes.
   `option_data_loss_protect` properly.
 - `--bind-addr=<path>` fixed for nodes using local sockets (eg. testing).
 - Unannounced local channels were forgotten for routing on restart until reconnection occurred.
+- lightning-cli: arguments containing `"` now succeed, rather than causing JSON errors.
 
 ### Security
 

diff --git a/cli/Makefile b/cli/Makefile
@@ -4,6 +4,7 @@ LIGHTNING_CLI_OBJS := $(LIGHTNING_CLI_SRC:.c=.o)
 LIGHTNING_CLI_COMMON_OBJS :=			\
 	common/configdir.o			\
 	common/json.o				\
+	common/json_escaped.o			\
 	common/memleak.o			\
 	common/utils.o				\
 	common/version.o

diff --git a/cli/lightning-cli.c b/cli/lightning-cli.c
@@ -8,6 +8,7 @@
 #include <ccan/tal/str/str.h>
 #include <common/configdir.h>
 #include <common/json.h>
+#include <common/json_escaped.h>
 #include <common/memleak.h>
 #include <common/utils.h>
 #include <common/version.h>
@@ -177,7 +178,7 @@ static void add_input(char **cmd, const char *input,
 	if (is_literal(input))
 		tal_append_fmt(cmd, "%s", input);
 	else
-		tal_append_fmt(cmd, "\"%s\"", input);
+		tal_append_fmt(cmd, "\"%s\"", json_escape(*cmd, input)->s);
 	if (i != argc - 1)
 		tal_append_fmt(cmd, ", ");
 }
@@ -355,7 +356,7 @@ int main(int argc, char *argv[])
 	idstr = tal_fmt(ctx, "lightning-cli-%i", getpid());
 	cmd = tal_fmt(ctx,
 		      "{ \"jsonrpc\" : \"2.0\", \"method\" : \"%s\", \"id\" : \"%s\", \"params\" :",
-		      method, idstr);
+		      json_escape(ctx, method)->s, idstr);
 
 	if (input == DEFAULT_INPUT) {
 		/* Hacky autodetect; only matters if more than single arg */

diff --git a/cli/test/Makefile b/cli/test/Makefile
@@ -12,6 +12,7 @@ CLI_TEST_COMMON_OBJS :=				\
 	common/daemon_conn.o			\
 	common/htlc_state.o			\
 	common/json.o				\
+	common/json_escaped.o			\
 	common/pseudorand.o			\
 	common/memleak.o			\
 	common/msg_queue.o			\

diff --git a/cli/test/run-large-input.c b/cli/test/run-large-input.c
@@ -25,12 +25,6 @@ int test_printf(const char *format, ...);
 #undef main
 
 /* AUTOGENERATED MOCKS START */
-/* Generated stub for amount_sat_eq */
-bool amount_sat_eq(struct amount_sat a UNNEEDED, struct amount_sat b UNNEEDED)
-{ fprintf(stderr, "amount_sat_eq called!\n"); abort(); }
-/* Generated stub for amount_sat_less */
-bool amount_sat_less(struct amount_sat a UNNEEDED, struct amount_sat b UNNEEDED)
-{ fprintf(stderr, "amount_sat_less called!\n"); abort(); }
 /* Generated stub for version_and_exit */
 char *version_and_exit(const void *unused UNNEEDED)
 { fprintf(stderr, "version_and_exit called!\n"); abort(); }

diff --git a/tests/test_misc.py b/tests/test_misc.py
@@ -790,6 +790,27 @@ def test_cli(node_factory):
     except Exception:
         pass
 
+    # Test it escapes JSON properly in both method and params.
+    out = subprocess.run(['cli/lightning-cli',
+                          '--lightning-dir={}'
+                          .format(l1.daemon.lightning_dir),
+                          'x"[]{}'],
+                         stdout=subprocess.PIPE)
+    assert 'Unknown command \'x\\"[]{}\'' in out.stdout.decode('utf-8')
+
+    subprocess.check_output(['cli/lightning-cli',
+                             '--lightning-dir={}'
+                             .format(l1.daemon.lightning_dir),
+                             'invoice', '123000', 'l"[]{}', 'd"[]{}']).decode('utf-8')
+    # Check label is correct, and also that cli's keyword parsing works.
+    out = subprocess.check_output(['cli/lightning-cli',
+                                   '--lightning-dir={}'
+                                   .format(l1.daemon.lightning_dir),
+                                   '-k',
+                                   'listinvoices', 'label=l"[]{}']).decode('utf-8')
+    j = json.loads(out)
+    assert only_one(j['invoices'])['label'] == 'l"[]{}'
+
 
 def test_daemon_option(node_factory):
     """