← Watch feed
lightningd: internal cleanups since all non-command JSON IDs are strings.
Public commit record
73/100 · Adequate
What the developer wrote
Authored by Rusty Russell
lightningd: internal cleanups since all non-command JSON IDs are strings.
In particular, "struct jsonrpc_request"'s id is always a string.
cmd->id isn't, though.
We can also remove the now-unused json_get_id.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
✓ Specific, descriptive subject✓ Names a concrete action or component✓ Provides detailed explanatory context
What changed, and why it matters
This is a routine internal cleanup commit in Core Lightning. It removes an unused helper function and simplifies how JSON-RPC request IDs are stored and formatted, since the project now always uses string IDs for non-command JSON traffic. The changes are refactor-style and do not appear to fix a security bug.
Recommended action
No security action required. Treat as normal code maintenance. Reviewers may optionally verify that plugin RPC ID forwarding still produces valid JSON string IDs in end-to-end tests.
Security signals we found
01
No security-relevant keywords in commit title or message
02
No CVE, advisory, or bug reference present
03
Changes are refactor/cleanup in nature
04
No input validation or boundary changes beyond ID formatting
05
No memory safety fixes or privilege changes
Evidence from the diff
The commit removes json_get_id() from common/json_parse_simple and drops the id_is_string boolean from struct jsonrpc_request. Request IDs are now stored without surrounding quotes and added as proper JSON strings via json_add_string(). Plugin response forwarding gains a make_new_id_a_string flag so plugin-bound requests get string IDs while command forwarding preserves cmd->id as a raw JSON token. Test log regexes are updated to match the unquoted internal ID representation.
Changed components
lightningd/jsonrpc.clightningd/jsonrpc.hlightningd/plugin.ccommon/json_parse_simple.ccommon/json_parse_simple.hInspect captured patch +25 / −37
diff --git a/common/json_parse_simple.c b/common/json_parse_simple.c
index fbb2b12c..ce6ab606 100644
--- a/common/json_parse_simple.c
+++ b/common/json_parse_simple.c
@@ -228,17 +228,6 @@ const jsmntok_t *json_get_arr(const jsmntok_t tok[], size_t index)
return NULL;
}
-const char *json_get_id(const tal_t *ctx,
- const char *buffer, const jsmntok_t *obj)
-{
- const jsmntok_t *idtok = json_get_member(buffer, obj, "id");
- if (!idtok)
- return NULL;
- return tal_strndup(ctx,
- json_tok_full(buffer, idtok),
- json_tok_full_len(idtok));
-}
-
/*-----------------------------------------------------------------------------
JSMN Result Validation Starts
-----------------------------------------------------------------------------*/
diff --git a/common/json_parse_simple.h b/common/json_parse_simple.h
index 56f97e2c..653a3679 100644
--- a/common/json_parse_simple.h
+++ b/common/json_parse_simple.h
@@ -83,10 +83,6 @@ static inline const jsmntok_t *json_get_member(const char *buffer,
/* Get index'th array member. */
const jsmntok_t *json_get_arr(const jsmntok_t tok[], size_t index);
-/* Helper to get "id" field from object (including any quotes!). */
-const char *json_get_id(const tal_t *ctx,
- const char *buffer, const jsmntok_t *obj);
-
/* Allocate a starter array of tokens for json_parse_input */
jsmntok_t *toks_alloc(const tal_t *ctx);
diff --git a/lightningd/jsonrpc.c b/lightningd/jsonrpc.c
index 81bcb999..fb7bec10 100644
--- a/lightningd/jsonrpc.c
+++ b/lightningd/jsonrpc.c
@@ -1553,7 +1553,6 @@ struct jsonrpc_request *jsonrpc_request_start_(
struct jsonrpc_request *r = tal(ctx, struct jsonrpc_request);
static u64 next_request_id = 0;
- r->id_is_string = true;
if (id_prefix) {
/* Strip "" and otherwise sanity-check */
if (strstarts(id_prefix, "\"")
@@ -1567,10 +1566,10 @@ struct jsonrpc_request *jsonrpc_request_start_(
if (json_escape_needed(id_prefix, strlen(id_prefix)))
id_prefix = "weird-id";
- r->id = tal_fmt(r, "\"%s/cln:%s#%"PRIu64"\"",
+ 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);
+ r->id = tal_fmt(r, "cln:%s#%"PRIu64, method, next_request_id);
}
tal_free_if_taken(id_prefix);
next_request_id++;
@@ -1586,7 +1585,7 @@ struct jsonrpc_request *jsonrpc_request_start_(
if (add_header) {
json_object_start(r->stream, NULL);
json_add_string(r->stream, "jsonrpc", "2.0");
- json_add_id(r->stream, r->id);
+ json_add_string(r->stream, "id", r->id);
json_add_string(r->stream, "method", method);
json_object_start(r->stream, "params");
}
diff --git a/lightningd/jsonrpc.h b/lightningd/jsonrpc.h
index e5a5edfa..ab3b07ee 100644
--- a/lightningd/jsonrpc.h
+++ b/lightningd/jsonrpc.h
@@ -78,7 +78,6 @@ struct jsonrpc_notification {
struct jsonrpc_request {
const char *id;
- bool id_is_string;
const char *method;
struct json_stream *stream;
void (*notify_cb)(const char *buffer,
diff --git a/lightningd/plugin.c b/lightningd/plugin.c
index 9edb27b0..036903e9 100644
--- a/lightningd/plugin.c
+++ b/lightningd/plugin.c
@@ -233,7 +233,7 @@ static void plugin_terminated_fail_req(struct plugin *plugin,
buf = tal_fmt(plugin,
"{\"jsonrpc\": \"2.0\","
- "\"id\": %s,"
+ "\"id\": \"%s\","
"\"error\":"
" {\"code\":%i, \"message\":\"%s\"}"
"}\n\n",
@@ -550,10 +550,9 @@ 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->pending_requests,
- json_tok_full(buffer, idtok),
- json_tok_full_len(idtok));
+ buffer + idtok->start,
+ idtok->end - idtok->start);
if (!request) {
return NULL;
}
@@ -676,8 +675,8 @@ static void plugin_response_handle(struct plugin *plugin,
const tal_t *ctx;
request = strmap_getn(&plugin->pending_requests,
- json_tok_full(buffer, idtok),
- json_tok_full_len(idtok));
+ buffer + idtok->start,
+ idtok->end - idtok->start);
/* Can happen if request was freed before plugin responded */
if (!request) {
return;
@@ -1189,8 +1188,8 @@ static void json_stream_forward_change_id(struct json_stream *stream,
const char *buffer,
const jsmntok_t *toks,
const jsmntok_t *idtok,
- /* Full token, including "" */
- const char *new_id)
+ const char *new_id,
+ bool make_new_id_a_string)
{
/* We copy everything, but replace the id. Special care has to
* be taken when the id that is being replaced is a string. If
@@ -1204,7 +1203,11 @@ static void json_stream_forward_change_id(struct json_stream *stream,
json_stream_append(stream, buffer + toks->start,
id_start - (buffer + toks->start));
+ if (make_new_id_a_string)
+ json_stream_append(stream, "\"", 1);
json_stream_append(stream, new_id, strlen(new_id));
+ if (make_new_id_a_string)
+ json_stream_append(stream, "\"", 1);
json_stream_append(stream, id_end, (buffer + toks->end) - id_end);
}
@@ -1216,7 +1219,8 @@ 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 a complete JSON token, quotes and all (if a string) */
+ json_stream_forward_change_id(response, buffer, toks, idtok, cmd->id, false);
json_stream_double_cr(response);
command_raw_complete(cmd, response);
}
@@ -1237,8 +1241,9 @@ static void plugin_notify_cb(const char *buffer,
json_add_string(response, "jsonrpc", "2.0");
json_add_tok(response, "method", methodtok, buffer);
json_stream_append(response, ",\"params\":", strlen(",\"params\":"));
+ /* cmd->id is a complete JSON token, quotes and all (if a string) */
json_stream_forward_change_id(response, buffer,
- paramtoks, idtok, cmd->id);
+ paramtoks, idtok, cmd->id, false);
json_object_end(response);
json_stream_double_cr(response);
@@ -1294,7 +1299,7 @@ static struct command_result *plugin_rpcmethod_check(struct command *cmd,
plugin_notify_cb,
plugin_rpcmethod_cb, cmd);
- json_stream_forward_change_id(req->stream, buffer, toks, idtok, req->id);
+ json_stream_forward_change_id(req->stream, buffer, toks, idtok, req->id, true);
json_stream_double_cr(req->stream);
plugin_request_send(plugin, req);
req->stream = NULL;
@@ -1338,7 +1343,7 @@ static struct command_result *plugin_rpcmethod_dispatch(struct command *cmd,
plugin_notify_cb,
plugin_rpcmethod_cb, cmd);
- json_stream_forward_change_id(req->stream, buffer, toks, idtok, req->id);
+ json_stream_forward_change_id(req->stream, buffer, toks, idtok, req->id, true);
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 92077b65..f85c3023 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.decode(inv['bolt11'])
diff --git a/tests/test_plugin.py b/tests/test_plugin.py
index 783470ac..10b97d9d 100644
--- a/tests/test_plugin.py
+++ b/tests/test_plugin.py
@@ -1929,7 +1929,7 @@ def test_libplugin(node_factory):
myname = os.path.splitext(os.path.basename(sys.argv[0]))[0]
# getmanifest assumes everyone handles string-based JSON ids:
- l1.daemon.wait_for_log(r'test_libplugin: "[-A-Za-z0-9:#]*/cln:getmanifest#[0-9]*"\[OUT\]')
+ l1.daemon.wait_for_log(r'test_libplugin: [-A-Za-z0-9:#]*/cln:getmanifest#[0-9]*\[OUT\]')
l1.daemon.wait_for_log("String name from datastore:.*object does not have member string")
l1.daemon.wait_for_log("Hex name from datastore: 00010203")
@@ -1956,7 +1956,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"]))
@@ -3224,7 +3224,7 @@ def test_commando(node_factory, executor):
# 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\]')
+ 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',
diff --git a/tests/test_wallet.py b/tests/test_wallet.py
index 16c27b40..a69ed03f 100644
--- a/tests/test_wallet.py
+++ b/tests/test_wallet.py
@@ -69,7 +69,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)
Risk score
Our methodology →Why this scored 17/100
Human-validated context
Community notes
Notes can correct, qualify, or add evidence to the AI analysis. Every note shown here has been validated by a human moderator.
No validated notes yet.
The AI analysis stands alone for now. Submit a note if you can add evidence or important context.