diff --git a/plugins/commando.c b/plugins/commando.c index 5803d79cb..8baaf999f 100644 --- a/plugins/commando.c +++ b/plugins/commando.c @@ -365,6 +365,7 @@ static void try_command(struct node_id *peer, const jsmntok_t *toks, *method, *params, *rune, *id; const char *buf = (const char *)msg, *failmsg; struct out_req *req; + const char *cmdid_prefix; incoming->peer = *peer; incoming->id = idnum; @@ -395,10 +396,17 @@ static void try_command(struct node_id *peer, } rune = json_get_member(buf, toks, "rune"); id = json_get_member(buf, toks, "id"); - if (!id && !deprecated_apis) { - commando_error(incoming, COMMANDO_ERROR_REMOTE, - "missing id field"); - return; + if (!id) { + if (!deprecated_apis) { + commando_error(incoming, COMMANDO_ERROR_REMOTE, + "missing id field"); + return; + } + cmdid_prefix = NULL; + } else { + cmdid_prefix = tal_fmt(tmpctx, "%.*s/", + id->end - id->start, + buf + id->start); } failmsg = check_rune(tmpctx, incoming, peer, buf, method, params, rune); @@ -412,6 +420,7 @@ static void try_command(struct node_id *peer, req = jsonrpc_request_whole_object_start(plugin, NULL, json_strdup(tmpctx, buf, method), + cmdid_prefix, cmd_done, incoming); if (params) { size_t i; diff --git a/plugins/libplugin.c b/plugins/libplugin.c index 9d78ccc85..ed9bd46c2 100644 --- a/plugins/libplugin.c +++ b/plugins/libplugin.c @@ -172,25 +172,30 @@ static void disable_request_cb(struct command *cmd, struct out_req *out) out->cmd = NULL; } -static const char *get_json_id(const tal_t *ctx, - struct plugin *plugin, - const char *cmd_id, - const char *method) +const char *json_id_prefix(const tal_t *ctx, const struct command *cmd) { - const char *prefix; + if (!cmd) + return ""; - if (cmd_id) { - /* Strip quotes! */ - if (strstarts(cmd_id, "\"")) { - assert(strlen(cmd_id) >= 2); - assert(strends(cmd_id, "\"")); - prefix = tal_fmt(tmpctx, "%.*s/", - (int)strlen(cmd_id) - 2, cmd_id + 1); - } else - prefix = tal_fmt(tmpctx, "%s/", cmd_id); - } else - prefix = ""; + /* Notifications have no cmd->id, use methodname */ + if (!cmd->id) + return tal_fmt(ctx, "%s/", cmd->methodname); + /* Strip quotes! */ + if (strstarts(cmd->id, "\"")) { + assert(strlen(cmd->id) >= 2); + assert(strends(cmd->id, "\"")); + return tal_fmt(ctx, "%.*s/", + (int)strlen(cmd->id) - 2, cmd->id + 1); + } + return tal_fmt(ctx, "%s/", cmd->id); +} + +static const char *append_json_id(const tal_t *ctx, + struct plugin *plugin, + const char *method, + const char *prefix) +{ return tal_fmt(ctx, "\"%s%s:%s#%"PRIu64"\"", prefix, plugin->id, method, plugin->next_outreq_id++); } @@ -205,6 +210,7 @@ static void destroy_out_req(struct out_req *out_req, struct plugin *plugin) struct out_req * jsonrpc_request_start_(struct plugin *plugin, struct command *cmd, const char *method, + const char *id_prefix, struct command_result *(*cb)(struct command *command, const char *buf, const jsmntok_t *result, @@ -218,7 +224,7 @@ jsonrpc_request_start_(struct plugin *plugin, struct command *cmd, struct out_req *out; out = tal(cmd, struct out_req); - out->id = get_json_id(out, plugin, cmd ? cmd->id : NULL, method); + out->id = append_json_id(out, plugin, method, id_prefix); out->cmd = cmd; out->cb = cb; out->errcb = errcb; @@ -556,7 +562,7 @@ static const jsmntok_t *sync_req(const tal_t *ctx, const jsmntok_t *contents; int reqlen; struct json_out *jout = json_out_new(tmpctx); - const char *id = get_json_id(tmpctx, plugin, "init", method); + const char *id = append_json_id(tmpctx, plugin, method, "init/"); json_out_start(jout, NULL, '{'); json_out_addstr(jout, "jsonrpc", "2.0"); diff --git a/plugins/libplugin.h b/plugins/libplugin.h index b5d2ae5e4..57fd44609 100644 --- a/plugins/libplugin.h +++ b/plugins/libplugin.h @@ -110,6 +110,7 @@ const struct feature_set *plugin_feature_set(const struct plugin *p); struct out_req *jsonrpc_request_start_(struct plugin *plugin, struct command *cmd, const char *method, + const char *id_prefix, struct command_result *(*cb)(struct command *command, const char *buf, const jsmntok_t *result, @@ -124,6 +125,7 @@ struct out_req *jsonrpc_request_start_(struct plugin *plugin, * "error" members. */ #define jsonrpc_request_start(plugin, cmd, method, cb, errcb, arg) \ jsonrpc_request_start_((plugin), (cmd), (method), \ + json_id_prefix(tmpctx, (cmd)), \ typesafe_cb_preargs(struct command_result *, void *, \ (cb), (arg), \ struct command *command, \ @@ -139,8 +141,8 @@ struct out_req *jsonrpc_request_start_(struct plugin *plugin, /* This variant has callbacks received whole obj, not "result" or * "error" members. It also doesn't start params{}. */ -#define jsonrpc_request_whole_object_start(plugin, cmd, method, cb, arg) \ - jsonrpc_request_start_((plugin), (cmd), (method), \ +#define jsonrpc_request_whole_object_start(plugin, cmd, method, id_prefix, cb, arg) \ + jsonrpc_request_start_((plugin), (cmd), (method), (id_prefix), \ typesafe_cb_preargs(struct command_result *, void *, \ (cb), (arg), \ struct command *command, \ @@ -470,6 +472,9 @@ struct createonion_response *json_to_createonion_response(const tal_t *ctx, struct route_hop *json_to_route(const tal_t *ctx, const char *buffer, const jsmntok_t *toks); +/* Create a prefix (ending in /) for this cmd_id, if any. */ +const char *json_id_prefix(const tal_t *ctx, const struct command *cmd); + #if DEVELOPER struct htable; void plugin_set_memleak_handler(struct plugin *plugin, diff --git a/plugins/test/run-route-overlong.c b/plugins/test/run-route-overlong.c index ba87bb1cd..a20778629 100644 --- a/plugins/test/run-route-overlong.c +++ b/plugins/test/run-route-overlong.c @@ -107,6 +107,9 @@ void json_array_start(struct json_stream *js UNNEEDED, const char *fieldname UNN const jsmntok_t *json_get_member(const char *buffer UNNEEDED, const jsmntok_t tok[] UNNEEDED, const char *label UNNEEDED) { fprintf(stderr, "json_get_member called!\n"); abort(); } +/* Generated stub for json_id_prefix */ +const char *json_id_prefix(const tal_t *ctx UNNEEDED, const struct command *cmd UNNEEDED) +{ fprintf(stderr, "json_id_prefix called!\n"); abort(); } /* Generated stub for json_next */ const jsmntok_t *json_next(const jsmntok_t *tok UNNEEDED) { fprintf(stderr, "json_next called!\n"); abort(); } @@ -184,6 +187,7 @@ bool json_tok_streq(const char *buffer UNNEEDED, const jsmntok_t *tok UNNEEDED, struct out_req *jsonrpc_request_start_(struct plugin *plugin UNNEEDED, struct command *cmd UNNEEDED, const char *method UNNEEDED, + const char *id_prefix UNNEEDED, struct command_result *(*cb)(struct command *command UNNEEDED, const char *buf UNNEEDED, const jsmntok_t *result UNNEEDED, diff --git a/tests/test_plugin.py b/tests/test_plugin.py index 7f127d75d..c2d06580c 100644 --- a/tests/test_plugin.py +++ b/tests/test_plugin.py @@ -2608,7 +2608,8 @@ def test_plugin_shutdown(node_factory): def test_commando(node_factory, executor): - l1, l2 = node_factory.line_graph(2, fundchannel=False) + l1, l2 = node_factory.line_graph(2, fundchannel=False, + opts={'log-level': 'io'}) # Nothing works until we've issued a rune. fut = executor.submit(l2.rpc.call, method='commando', @@ -2634,6 +2635,11 @@ def test_commando(node_factory, executor): assert len(res['peers']) == 1 assert res['peers'][0]['id'] == l2.info['id'] + # Check JSON id is as expected (unfortunately pytest does not use a reliable name + # for itself: with -k it calls itself `-c` here, instead of `pytest`). + l2.daemon.wait_for_log(r'plugin-commando: "[^:/]*:commando#[0-9]*/cln:commando#[0-9]*"\[OUT\]') + l1.daemon.wait_for_log(r'jsonrpc#[0-9]*: "[^:/]*:commando#[0-9]*/cln:commando#[0-9]*/commando:listpeers#[0-9]*"\[IN\]') + res = l2.rpc.call(method='commando', payload={'peer_id': l1.info['id'], 'rune': rune,