diff --git a/lightningd/jsonrpc.c b/lightningd/jsonrpc.c index 9aec11114..fe4e2d759 100644 --- a/lightningd/jsonrpc.c +++ b/lightningd/jsonrpc.c @@ -203,7 +203,9 @@ static struct command_result *json_stop(struct command *cmd, jout = json_out_new(tmpctx); json_out_start(jout, NULL, '{'); json_out_addstr(jout, "jsonrpc", "2.0"); - json_out_add(jout, "id", cmd->id_is_string, "%s", cmd->id); + /* Copy input id token exactly */ + memcpy(json_out_member_direct(jout, "id", strlen(cmd->id)), + cmd->id, strlen(cmd->id)); json_out_addstr(jout, "result", "Shutdown complete"); json_out_end(jout, '}'); json_out_finished(jout); @@ -557,10 +559,7 @@ void json_notify_fmt(struct command *cmd, json_add_string(js, "jsonrpc", "2.0"); json_add_string(js, "method", "message"); json_object_start(js, "params"); - if (cmd->id_is_string) - json_add_string(js, "id", cmd->id); - else - json_add_jsonstr(js, "id", cmd->id, strlen(cmd->id)); + json_add_id(js, cmd->id); json_add_string(js, "level", log_level_name(level)); json_add_string(js, "message", tal_vfmt(tmpctx, fmt, ap)); json_object_end(js); @@ -605,10 +604,7 @@ static struct json_stream *json_start(struct command *cmd) json_object_start(js, NULL); json_add_string(js, "jsonrpc", "2.0"); - if (cmd->id_is_string) - json_add_string(js, "id", cmd->id); - else - json_add_jsonstr(js, "id", cmd->id, strlen(cmd->id)); + json_add_id(js, cmd->id); return js; } @@ -934,7 +930,10 @@ parse_request(struct json_connection *jcon, const jsmntok_t tok[]) c->pending = false; c->json_stream = NULL; c->id_is_string = (id->type == JSMN_STRING); - c->id = json_strdup(c, jcon->buffer, id); + /* Include "" around string */ + c->id = tal_strndup(c, + json_tok_full(jcon->buffer, id), + json_tok_full_len(id)); c->mode = CMD_NORMAL; c->filter = NULL; list_add_tail(&jcon->commands, &c->list); @@ -1068,7 +1067,7 @@ again: json_command_malformed( jcon, "null", tal_fmt(tmpctx, "Invalid token in json input: '%s'", - tal_strndup(tmpctx, jcon->buffer, jcon->used))); + tal_hexstr(tmpctx, jcon->buffer, jcon->used))); if (in_transaction) db_commit_transaction(jcon->ld->wallet->db); return io_halfclose(conn); @@ -1400,11 +1399,23 @@ struct jsonrpc_request *jsonrpc_request_start_( r->id_is_string = id_as_string; if (r->id_is_string) { - if (id_prefix) - r->id = tal_fmt(r, "%s/cln:%s#%"PRIu64, + if (id_prefix) { + /* Strip "" and otherwise sanity-check */ + if (strstarts(id_prefix, "\"") + && strlen(id_prefix) > 1 + && strends(id_prefix, "\"")) { + id_prefix = tal_strndup(tmpctx, id_prefix + 1, + strlen(id_prefix) - 2); + } + /* We could try escaping, but TBH they're + * messing with us at this point! */ + if (json_escape_needed(id_prefix, strlen(id_prefix))) + id_prefix = "weird-id"; + + r->id = tal_fmt(r, "\"%s/cln:%s#%"PRIu64"\"", id_prefix, method, next_request_id); - else - r->id = tal_fmt(r, "cln:%s#%"PRIu64, method, next_request_id); + } else + r->id = tal_fmt(r, "\"cln:%s#%"PRIu64"\"", method, next_request_id); } else { r->id = tal_fmt(r, "%"PRIu64, next_request_id); } @@ -1423,10 +1434,7 @@ struct jsonrpc_request *jsonrpc_request_start_( if (add_header) { json_object_start(r->stream, NULL); json_add_string(r->stream, "jsonrpc", "2.0"); - if (r->id_is_string) - json_add_string(r->stream, "id", r->id); - else - json_add_primitive(r->stream, "id", r->id); + json_add_id(r->stream, r->id); json_add_string(r->stream, "method", method); json_object_start(r->stream, "params"); } diff --git a/lightningd/plugin.c b/lightningd/plugin.c index 8a883e220..fcd6caef9 100644 --- a/lightningd/plugin.c +++ b/lightningd/plugin.c @@ -448,13 +448,16 @@ static const char *plugin_notify_handle(struct plugin *plugin, "JSON-RPC notify \"id\"-field is not present"); } + /* Include any "" in id */ request = strmap_getn(&plugin->plugins->pending_requests, - plugin->buffer + idtok->start, - idtok->end - idtok->start); + json_tok_full(plugin->buffer, idtok), + json_tok_full_len(idtok)); if (!request) { return tal_fmt( plugin, - "Received a JSON-RPC notify for non-existent request"); + "Received a JSON-RPC notify for non-existent request '%.*s'", + json_tok_full_len(idtok), + json_tok_full(plugin->buffer, idtok)); } /* Ignore if they don't have a callback */ @@ -572,12 +575,14 @@ static const char *plugin_response_handle(struct plugin *plugin, struct jsonrpc_request *request; request = strmap_getn(&plugin->plugins->pending_requests, - plugin->buffer + idtok->start, - idtok->end - idtok->start); + json_tok_full(plugin->buffer, idtok), + json_tok_full_len(idtok)); if (!request) { return tal_fmt( plugin, - "Received a JSON-RPC response for non-existent request"); + "Received a JSON-RPC response for non-existent request '%.*s'", + json_tok_full_len(idtok), + json_tok_full(plugin->buffer, idtok)); } /* We expect the request->cb to copy if needed */ @@ -1028,37 +1033,23 @@ static void json_stream_forward_change_id(struct json_stream *stream, const char *buffer, const jsmntok_t *toks, const jsmntok_t *idtok, - const char *new_id, - bool new_id_is_str) + /* Full token, including "" */ + const char *new_id) { /* We copy everything, but replace the id. Special care has to * be taken when the id that is being replaced is a string. If * we don't crop the quotes off we'll transform a numeric * new_id into a string, or even worse, quote a string id * twice. */ - size_t offset = 0; - bool add_quotes = false; + const char *id_start, *id_end; - if (idtok->type == JSMN_STRING) { - if (new_id_is_str) - add_quotes = false; - else - offset = 1; - } else { - if (new_id_is_str) - add_quotes = true; - } + id_start = json_tok_full(buffer, idtok); + id_end = id_start + json_tok_full_len(idtok); json_stream_append(stream, buffer + toks->start, - idtok->start - toks->start - offset); - - if (add_quotes) - json_stream_append(stream, "\"", 1); + id_start - (buffer + toks->start)); json_stream_append(stream, new_id, strlen(new_id)); - if (add_quotes) - json_stream_append(stream, "\"", 1); - json_stream_append(stream, buffer + idtok->end + offset, - toks->end - idtok->end - offset); + json_stream_append(stream, id_end, (buffer + toks->end) - id_end); } static void plugin_rpcmethod_cb(const char *buffer, @@ -1070,8 +1061,7 @@ static void plugin_rpcmethod_cb(const char *buffer, struct json_stream *response; response = json_stream_raw_for_cmd(cmd); - json_stream_forward_change_id(response, buffer, toks, idtok, cmd->id, - cmd->id_is_string); + json_stream_forward_change_id(response, buffer, toks, idtok, cmd->id); json_stream_double_cr(response); command_raw_complete(cmd, response); @@ -1097,8 +1087,7 @@ static void plugin_notify_cb(const char *buffer, json_add_tok(response, "method", methodtok, buffer); json_stream_append(response, ",\"params\":", strlen(",\"params\":")); json_stream_forward_change_id(response, buffer, - paramtoks, idtok, cmd->id, - cmd->id_is_string); + paramtoks, idtok, cmd->id); json_object_end(response); json_stream_double_cr(response); @@ -1155,8 +1144,7 @@ static struct command_result *plugin_rpcmethod_dispatch(struct command *cmd, call->plugin = plugin; list_add_tail(&plugin->pending_rpccalls, &call->list); - json_stream_forward_change_id(req->stream, buffer, toks, idtok, req->id, - req->id_is_string); + json_stream_forward_change_id(req->stream, buffer, toks, idtok, req->id); json_stream_double_cr(req->stream); plugin_request_send(plugin, req); req->stream = NULL; diff --git a/tests/test_invoices.py b/tests/test_invoices.py index 195864d44..5df57e843 100644 --- a/tests/test_invoices.py +++ b/tests/test_invoices.py @@ -21,7 +21,7 @@ def test_invoice(node_factory, chainparams): # Side note: invoice calls out to listincoming, so check JSON id is as expected myname = os.path.splitext(os.path.basename(sys.argv[0]))[0] - l1.daemon.wait_for_log(r": {}:invoice#[0-9]*/cln:listincoming#[0-9]*\[OUT\]".format(myname)) + l1.daemon.wait_for_log(r': "{}:invoice#[0-9]*/cln:listincoming#[0-9]*"\[OUT\]'.format(myname)) after = int(time.time()) b11 = l1.rpc.decodepay(inv['bolt11']) diff --git a/tests/test_misc.py b/tests/test_misc.py index 8dfe56c46..13316dd32 100644 --- a/tests/test_misc.py +++ b/tests/test_misc.py @@ -874,7 +874,7 @@ def test_cli(node_factory): assert 'help [command]\n List available commands, or give verbose help on one {command}' in out # Check JSON id is as expected - l1.daemon.wait_for_log(r"jsonrpc#[0-9]*: cli:help#[0-9]*\[IN\]") + l1.daemon.wait_for_log(r'jsonrpc#[0-9]*: "cli:help#[0-9]*"\[IN\]') # Test JSON output. out = subprocess.check_output(['cli/lightning-cli', diff --git a/tests/test_plugin.py b/tests/test_plugin.py index aeb54cef5..7f127d75d 100644 --- a/tests/test_plugin.py +++ b/tests/test_plugin.py @@ -1539,7 +1539,7 @@ def test_libplugin(node_factory): # Test hooks and notifications (add plugin, so we can test hook id) l2 = node_factory.get_node(options={"plugin": plugin, 'log-level': 'io'}) l2.connect(l1) - l2.daemon.wait_for_log(r": {}:connect#[0-9]*/cln:peer_connected#[0-9]*\[OUT\]".format(myname)) + l2.daemon.wait_for_log(r': "{}:connect#[0-9]*/cln:peer_connected#[0-9]*"\[OUT\]'.format(myname)) l1.daemon.wait_for_log("{} peer_connected".format(l2.info["id"])) l1.daemon.wait_for_log("{} connected".format(l2.info["id"])) diff --git a/tests/test_wallet.py b/tests/test_wallet.py index a90fca091..be6275a17 100644 --- a/tests/test_wallet.py +++ b/tests/test_wallet.py @@ -62,7 +62,7 @@ def test_withdraw(node_factory, bitcoind): # Side note: sendrawtransaction will trace back to withdrawl myname = os.path.splitext(os.path.basename(sys.argv[0]))[0] - l1.daemon.wait_for_log(r": {}:withdraw#[0-9]*/cln:withdraw#[0-9]*/txprepare:sendpsbt#[0-9]*/cln:sendrawtransaction#[0-9]*\[OUT\]".format(myname)) + l1.daemon.wait_for_log(r': "{}:withdraw#[0-9]*/cln:withdraw#[0-9]*/txprepare:sendpsbt#[0-9]*/cln:sendrawtransaction#[0-9]*"\[OUT\]'.format(myname)) # Make sure bitcoind received the withdrawal unspent = l1.bitcoin.rpc.listunspent(0)