mirrors/core-lightning
1
0
Fork 0
mirror of https://github.com/ElementsProject/lightning.git synced 2025-03-03 10:46:58 +01:00
Code Issues Projects Releases Wiki Activity

lightningd: treat JSON ids as direct tokens.

Browse source 
This avoids any confusion between primitive and string ids, and in
particular stops an issue with commando once it starts chaining ids,
that weird ids can be double-escaped and commando will not recognize
the response, leaving the client hanging.  It's the client's fault for
using a weird id, but it's still rude (and triggered by our tests!).

It also makes substituting the id in passthrough simpler, FTW.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
This commit is contained in:
Rusty Russell 2023-01-03 14:52:42 +10:30
parent 2d8fff6b57
commit a1e894a445
6 changed files with 52 additions and 56 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

46
lightningd/jsonrpc.c
View file

@ -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");
}

54
lightningd/plugin.c
View file

@ -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;

2
tests/test_invoices.py
View file

@ -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'])

2
tests/test_misc.py
View file

@ -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',

2
tests/test_plugin.py
View file

@ -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"]))

2
tests/test_wallet.py
View file

@ -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)