lightningd: accept options array in plugin start - #9391
Conversation
daywalker90
added
the
Status::Ready for Review
| * of "keyword=value" strings. This merges both forms into a single JSON | ||
| * object of name/value pairs, suitable for plugin_add_params(). An element | ||
| * without an '=' is treated as a boolean flag. */ | ||
| static jsmntok_t *plugin_start_params(const tal_t *ctx, const char *buffer, |
There was a problem hiding this comment.
Nice!! As i understood when plugin_start_params() builds a new json string (mod_buffer), the code always threads mod_buffer through to plugin_dynamic_start() instead of the original buffer. Using the original buffer with fresh synthesized token offsets would have caused out-of-bounds reads
| * since plugin options must be explicitly named!). */ | ||
| if (params->type == JSMN_ARRAY) { | ||
| if (params->size != 2) | ||
| if (params->size > 2 && !options) |
There was a problem hiding this comment.
Is the intent of params->size > 2 && !options really just "reject a null in the options slot", or was it meant to also reject trailing garbage after a valid options array in a positional call?
There was a problem hiding this comment.
It was also meant to reject trailing stuff loudly... fixed.
| return command_fail(cmd, JSONRPC2_INVALID_PARAMS, | ||
| "Extra parameters must be in object"); | ||
| mod_params = NULL; | ||
| if (options) { |
There was a problem hiding this comment.
Should options array elements be required to be JSON strings?
There was a problem hiding this comment.
Yes, per the schema, fixed.
The plugin start command only accepted plugin options flattened onto the RPC call (e.g. via -k/--keyword), but the plugin RPC schema documents an explicit 'options' array. This mismatch meant that callers using named parameters against generated RPC bindings (cln-rpc, grpc, protobuf), which cannot flatten arbitrary options, failed with 'unknown parameter options'. Options without a value are treated as boolean flags, matching the flattened form. Changelog-Fixed: JSON-RPC: `plugin start` now accepts plugin options as an `options` array of `keyword=value` strings, as documented in the `plugin` schema.
82d1516 to
0539ebc
Compare
|
I reworked the PR to be more strict about mixing stuff. I don't think there is a legit use case to mix flattened AND explicit options array at the same time so i rather reject it. I extended the test with all option types and cases of mixed option passing. |
2 participants
The plugin start command only accepted plugin options flattened onto the RPC call (e.g. via -k/--keyword), but the plugin RPC schema documents an explicit 'options' array. This mismatch meant that callers using named parameters against generated RPC bindings (cln-rpc, grpc, protobuf), which cannot flatten arbitrary options, failed with 'unknown parameter options'.
Handle 'options' as an array of keyword=value strings, merging them with any flattened extra parameters before passing them to plugin_add_params(). Options without a value are treated as boolean flags, matching the flattened form.
Changelog-Fixed: JSON-RPC:
plugin startnow accepts plugin options as anoptionsarray ofkeyword=valuestrings, as documented in thepluginschema.Fixes: #9171
Important
26.09 FREEZE August 5th: Non-bugfix PRs not ready by this date will wait for 26.12.
RC1 is scheduled on August 17th
The final release is scheduled for September 7th.
Checklist
Before submitting the PR, ensure the following tasks are completed. If an item is not applicable to your PR, please mark it as checked:
tools/lightning-downgrade