diff --git a/common/Makefile b/common/Makefile index e7f098d05..378f1ed9c 100644 --- a/common/Makefile +++ b/common/Makefile @@ -52,7 +52,7 @@ COMMON_SRC_NOGEN := \ COMMON_SRC_GEN := common/gen_status_wire.c common/gen_peer_status_wire.c -COMMON_HEADERS_NOGEN := $(COMMON_SRC_NOGEN:.c=.h) common/overflows.h common/htlc.h common/status_levels.h +COMMON_HEADERS_NOGEN := $(COMMON_SRC_NOGEN:.c=.h) common/overflows.h common/htlc.h common/status_levels.h common/json_command.h COMMON_HEADERS_GEN := common/gen_htlc_state_names.h common/gen_status_wire.h common/gen_peer_status_wire.h COMMON_HEADERS := $(COMMON_HEADERS_GEN) $(COMMON_HEADERS_NOGEN) diff --git a/common/json_command.h b/common/json_command.h new file mode 100644 index 000000000..27140ae71 --- /dev/null +++ b/common/json_command.h @@ -0,0 +1,24 @@ +/* These functions must be supplied by any binary linking with common/param + * so it can fail commands. */ +#ifndef LIGHTNING_COMMON_JSON_COMMAND_H +#define LIGHTNING_COMMON_JSON_COMMAND_H +#include "config.h" +#include +#include + +struct command; + +/* Caller supplied this: param assumes it can call it. */ +void PRINTF_FMT(3, 4) command_fail(struct command *cmd, int code, + const char *fmt, ...); + +/* Also caller supplied: is this invoked simply to get usage? */ +bool command_usage_only(const struct command *cmd); + +/* If so, this is called. */ +void command_set_usage(struct command *cmd, const char *usage); + +/* Also caller supplied: is this invoked simply to check parameters? */ +bool command_check_only(const struct command *cmd); + +#endif /* LIGHTNING_COMMON_JSON_COMMAND_H */ diff --git a/lightningd/jsonrpc.c b/lightningd/jsonrpc.c index 213d5de9a..36d424b4a 100644 --- a/lightningd/jsonrpc.c +++ b/lightningd/jsonrpc.c @@ -23,6 +23,7 @@ #include #include #include +#include #include #include #include @@ -551,7 +552,6 @@ static void parse_request(struct json_connection *jcon, const jsmntok_t tok[]) json_tok_len(id)); c->mode = CMD_NORMAL; c->ok = NULL; - c->allow_unused = false; list_add_tail(&jcon->commands, &c->list); tal_add_destructor(c, destroy_command); @@ -776,6 +776,21 @@ struct jsonrpc *jsonrpc_new(const tal_t *ctx, struct lightningd *ld) return jsonrpc; } +bool command_usage_only(const struct command *cmd) +{ + return cmd->mode == CMD_USAGE; +} + +void command_set_usage(struct command *cmd, const char *usage) +{ + cmd->usage = usage; +} + +bool command_check_only(const struct command *cmd) +{ + return cmd->mode == CMD_CHECK; +} + void jsonrpc_listen(struct jsonrpc *jsonrpc, struct lightningd *ld) { struct sockaddr_un addr; @@ -980,11 +995,11 @@ static void json_check(struct command *cmd, mod_params = NULL; } else { mod_params = json_tok_copy(cmd, params); - cmd->allow_unused = true; } if (!param(cmd, buffer, mod_params, p_req("command_to_check", json_tok_command, &name_tok), + p_opt_any(), NULL)) return; @@ -995,7 +1010,6 @@ static void json_check(struct command *cmd, json_tok_remove(&mod_params, (jsmntok_t *)name_tok, 1); cmd->mode = CMD_CHECK; - cmd->allow_unused = false; /* FIXME(wythe): Maybe change "ok" to "failed" since that's really what * we're after and would be more clear. */ ok = true; diff --git a/lightningd/jsonrpc.h b/lightningd/jsonrpc.h index 4b6785d65..c7c35634e 100644 --- a/lightningd/jsonrpc.h +++ b/lightningd/jsonrpc.h @@ -38,8 +38,6 @@ struct command { /* This is created if mode is CMD_USAGE */ const char *usage; bool *ok; - /* Do not report unused parameters as errors (default false). */ - bool allow_unused; /* Have we started a json stream already? For debugging. */ bool have_json_stream; }; diff --git a/lightningd/param.c b/lightningd/param.c index bc1397755..34f0999fa 100644 --- a/lightningd/param.c +++ b/lightningd/param.c @@ -1,5 +1,6 @@ #include #include +#include #include #include #include @@ -63,7 +64,8 @@ static bool post_check(struct command *cmd, struct param *params) static bool parse_by_position(struct command *cmd, struct param *params, const char *buffer, - const jsmntok_t tokens[]) + const jsmntok_t tokens[], + bool allow_extra) { const jsmntok_t *tok = tokens + 1; const jsmntok_t *end = json_next(tokens); @@ -79,7 +81,7 @@ static bool parse_by_position(struct command *cmd, } /* check for unexpected trailing params */ - if (!cmd->allow_unused && tok != end) { + if (!allow_extra && tok != end) { command_fail(cmd, JSONRPC2_INVALID_PARAMS, "too many parameters:" " got %u, expected %zu", @@ -108,7 +110,8 @@ static struct param *find_param(struct param *params, const char *start, static bool parse_by_name(struct command *cmd, struct param *params, const char *buffer, - const jsmntok_t tokens[]) + const jsmntok_t tokens[], + bool allow_extra) { const jsmntok_t *first = tokens + 1; const jsmntok_t *last = json_next(tokens); @@ -117,7 +120,7 @@ static bool parse_by_name(struct command *cmd, struct param *p = find_param(params, buffer + first->start, first->end - first->start); if (!p) { - if (!cmd->allow_unused) { + if (!allow_extra) { command_fail(cmd, JSONRPC2_INVALID_PARAMS, "unknown parameter: '%.*s'", first->end - first->start, @@ -239,7 +242,8 @@ static char *param_usage(const tal_t *ctx, static bool param_arr(struct command *cmd, const char *buffer, const jsmntok_t tokens[], - struct param *params) + struct param *params, + bool allow_extra) { #if DEVELOPER if (!check_params(params)) { @@ -248,9 +252,9 @@ static bool param_arr(struct command *cmd, const char *buffer, } #endif if (tokens->type == JSMN_ARRAY) - return parse_by_position(cmd, params, buffer, tokens); + return parse_by_position(cmd, params, buffer, tokens, allow_extra); else if (tokens->type == JSMN_OBJECT) - return parse_by_name(cmd, params, buffer, tokens); + return parse_by_name(cmd, params, buffer, tokens, allow_extra); command_fail(cmd, JSONRPC2_INVALID_PARAMS, "Expected array or object for params"); @@ -263,12 +267,17 @@ bool param(struct command *cmd, const char *buffer, struct param *params = tal_arr(cmd, struct param, 0); const char *name; va_list ap; + bool allow_extra = false; va_start(ap, tokens); while ((name = va_arg(ap, const char *)) != NULL) { bool required = va_arg(ap, int); param_cbx cbx = va_arg(ap, param_cbx); void *arg = va_arg(ap, void *); + if (streq(name, "")) { + allow_extra = true; + continue; + } if (!param_add(¶ms, name, required, cbx, arg)) { command_fail(cmd, PARAM_DEV_ERROR, "developer error: param_add %s", name); @@ -278,14 +287,13 @@ bool param(struct command *cmd, const char *buffer, } va_end(ap); - if (cmd->mode == CMD_USAGE) { - cmd->usage = param_usage(cmd, params); + if (command_usage_only(cmd)) { + command_set_usage(cmd, param_usage(cmd, params)); return false; } - /* Always return false for CMD_USAGE and CMD_CHECK, signaling the caller - * to return immediately. For CMD_NORMAL, return true if all parameters - * are valid. - */ - return param_arr(cmd, buffer, tokens, params) && cmd->mode == CMD_NORMAL; + /* Always return false if we're simply checking command parameters; + * normally this returns true if all parameters are valid. */ + return param_arr(cmd, buffer, tokens, params, allow_extra) + && !command_check_only(cmd); } diff --git a/lightningd/param.h b/lightningd/param.h index 039dde46c..0701444c6 100644 --- a/lightningd/param.h +++ b/lightningd/param.h @@ -90,4 +90,6 @@ typedef bool(*param_cbx)(struct command *cmd, (const jsmntok_t *)NULL, \ (arg)) == true); }) +/* Special flag for 'check' which allows any parameters. */ +#define p_opt_any() "", false, NULL, NULL #endif /* LIGHTNING_LIGHTNINGD_PARAM_H */ diff --git a/lightningd/test/run-param.c b/lightningd/test/run-param.c index e5b090ede..765e4b9ec 100644 --- a/lightningd/test/run-param.c +++ b/lightningd/test/run-param.c @@ -48,6 +48,21 @@ bool json_feerate_estimate(struct command *cmd UNNEEDED, { fprintf(stderr, "json_feerate_estimate called!\n"); abort(); } /* AUTOGENERATED MOCKS END */ +void command_set_usage(struct command *cmd, const char *usage) +{ + cmd->usage = usage; +} + +bool command_usage_only(const struct command *cmd) +{ + return cmd->mode == CMD_USAGE; +} + +bool command_check_only(const struct command *cmd) +{ + return cmd->mode == CMD_CHECK; +} + struct json { jsmntok_t *toks; char *buffer; @@ -353,7 +368,7 @@ static void five_hundred_params(void) /* first test object version */ struct json *j = json_parse(params, obj); - assert(param_arr(cmd, j->buffer, j->toks, params)); + assert(param_arr(cmd, j->buffer, j->toks, params, false)); for (int i = 0; i < tal_count(ints); ++i) { assert(ints[i]); assert(*ints[i] == i); @@ -362,7 +377,7 @@ static void five_hundred_params(void) /* now test array */ j = json_parse(params, arr); - assert(param_arr(cmd, j->buffer, j->toks, params)); + assert(param_arr(cmd, j->buffer, j->toks, params, false)); for (int i = 0; i < tal_count(ints); ++i) { assert(*ints[i] == i); } @@ -563,7 +578,6 @@ int main(void) setup_tmpctx(); cmd = tal(tmpctx, struct command); cmd->mode = CMD_NORMAL; - cmd->allow_unused = false; fail_msg = tal_arr(cmd, char, 10000); zero_params();