commando: correctly replace the id field in responses.

This was reported a while ago: now do it properly.

Fixes: #5637
Changelog-Fixed: Plugins: `commando` now responds to remote JSON calls with the correct JSON `id` field.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
This commit is contained in:
Rusty Russell 2023-01-03 14:53:28 +10:30
parent b75ada7017
commit 1250806060
2 changed files with 28 additions and 16 deletions

View File

@ -152,10 +152,6 @@ static struct command_result *send_response(struct command *command UNUSED,
if (msglen > 65000) {
msglen = 65000;
msgtype = COMMANDO_MSG_REPLY_CONTINUES;
/* We need to make a copy first time before we call back, since
* plugin will reuse it! */
if (!result)
reply->buf = tal_dup_talarr(reply, char, reply->buf);
} else {
if (msglen == 0) {
tal_free(reply);
@ -188,12 +184,32 @@ static struct command_result *cmd_done(struct command *command,
{
struct reply *reply = tal(plugin, struct reply);
reply->incoming = tal_steal(reply, incoming);
reply->buf = (char *)buf;
/* result is contents of "error" or "response": we want top-leve
* object */
reply->off = obj->start;
reply->len = obj->end;
/* We make a copy, but substititing the original id! */
if (incoming->json_id) {
const char *id_start, *id_end;
const jsmntok_t *id = json_get_member(buf, obj, "id");
size_t off;
/* Old id we're going to omit */
id_start = json_tok_full(buf, id);
id_end = id_start + json_tok_full_len(id);
reply->len = obj->end - obj->start
- (id_end - id_start)
+ strlen(incoming->json_id);
reply->buf = tal_arr(reply, char, reply->len);
memcpy(reply->buf, buf + obj->start,
id_start - (buf + obj->start));
off = id_start - (buf + obj->start);
memcpy(reply->buf + off, incoming->json_id, strlen(incoming->json_id));
off += strlen(incoming->json_id);
memcpy(reply->buf + off, id_end, (buf + obj->end) - id_end);
} else {
reply->len = obj->end - obj->start;
reply->buf = tal_strndup(reply, buf + obj->start, reply->len);
}
reply->off = 0;
return send_response(command, NULL, NULL, reply);
}

View File

@ -2609,9 +2609,7 @@ def test_plugin_shutdown(node_factory):
def test_commando(node_factory, executor):
l1, l2 = node_factory.line_graph(2, fundchannel=False,
opts={'log-level': 'io',
# FIXME: Currently, our JSON ids in replies are wrong, hence BROKEN!
'allow_broken_log': True})
opts={'log-level': 'io'})
# Nothing works until we've issued a rune.
fut = executor.submit(l2.rpc.call, method='commando',
@ -2706,8 +2704,7 @@ def test_commando(node_factory, executor):
def test_commando_rune(node_factory):
# FIXME: Currently, our JSON ids in replies are wrong, hence BROKEN!
l1, l2 = node_factory.get_nodes(2, opts={'allow_broken_log': True})
l1, l2 = node_factory.get_nodes(2)
# Force l1's commando secret
l1.rpc.datastore(key=['commando', 'secret'], hex='1241faef85297127c2ac9bde95421b2c51e5218498ae4901dc670c974af4284b')
@ -2942,8 +2939,7 @@ def test_commando_rune(node_factory):
def test_commando_stress(node_factory, executor):
"""Stress test to slam commando with many large queries"""
# FIXME: Currently, our JSON ids in replies are wrong, hence BROKEN!
nodes = node_factory.get_nodes(5, opts={'allow_broken_log': True})
nodes = node_factory.get_nodes(5)
rune = nodes[0].rpc.commando_rune()['rune']
for n in nodes[1:]: