From bd5bf1f1688ccbd0d03ad0ff62d2b795fb3dff77 Mon Sep 17 00:00:00 2001 From: Mark Beckwith Date: Thu, 9 Aug 2018 21:15:30 -0500 Subject: [PATCH] Enhanced param parsing [ Squashed into single commit --RR ] This adds two new macros, `p_req_tal()` and `p_opt_tal()`. These support callbacks that take a `struct command *` context. Example: static bool json_tok_label_x(struct command *cmd, const char *name, const char *buffer, const jsmntok_t *tok, struct json_escaped **label) The above is taken from the run-param unit test (near the bottom of the diff). The return value is true on success, or false (and it calls command_fail itself). We can pretty much remove all remaining usage of `json_tok_tok` in the codebase with this type of callback. --- lightningd/param.c | 65 +++++++++++------ lightningd/param.h | 49 +++++++++++++ lightningd/test/run-param.c | 137 ++++++++++++++++++++++++++++++++++-- 3 files changed, 224 insertions(+), 27 deletions(-) diff --git a/lightningd/param.c b/lightningd/param.c index 95db6042a..e1e02d6b5 100644 --- a/lightningd/param.c +++ b/lightningd/param.c @@ -12,16 +12,18 @@ struct param { bool is_set; bool required; param_cb cb; + param_cbx cbx; void *arg; size_t argsize; }; static bool param_add(struct param **params, - const char *name, bool required, param_cb cb, void *arg, + const char *name, bool required, param_cb cb, + param_cbx cbx, void *arg, size_t argsize) { #if DEVELOPER - if (!(name && cb && arg)) + if (!(name && (cb || cbx) && arg)) return false; #endif struct param *last; @@ -33,6 +35,7 @@ static bool param_add(struct param **params, last->name = name; last->required = required; last->cb = cb; + last->cbx = cbx; last->arg = arg; last->argsize = argsize; /* Non-0 means we are supposed to allocate iff found */ @@ -72,30 +75,44 @@ static const char *find_fail_format(param_cb cb) return fmt->format; } +/* Create a json_result out of a jsmntok_t. */ +static struct json_result *make_result(tal_t *ctx, const char *name, const char *buffer, + const jsmntok_t *tok) +{ + struct json_result *data = new_json_result(ctx); + const char *val = tal_fmt(ctx, "%.*s", tok->end - tok->start, + buffer + tok->start); + json_object_start(data, NULL); + json_add_string(data, name, val); + json_object_end(data); + return data; +} + static bool make_callback(struct command *cmd, struct param *def, - const char *buffer, const jsmntok_t * tok) + const char *buffer, const jsmntok_t *tok) { void *arg; def->is_set = true; - if (def->argsize && def->cb != (param_cb)json_tok_tok) { - *(void **)def->arg - = arg - = tal_arr_label(cmd, char, def->argsize, "param"); - } else - arg = def->arg; - if (!def->cb(buffer, tok, arg)) { - struct json_result *data = new_json_result(cmd); - const char *val = tal_fmt(cmd, "%.*s", tok->end - tok->start, - buffer + tok->start); - json_object_start(data, NULL); - json_add_string(data, def->name, val); - json_object_end(data); - command_fail_detailed(cmd, JSONRPC2_INVALID_PARAMS, data, - find_fail_format(def->cb), def->name, - tok->end - tok->start, - buffer + tok->start); - return false; + + if (def->cb) { + if (def->argsize && def->cb != (param_cb)json_tok_tok) { + *(void **)def->arg + = arg + = tal_arr_label(cmd, char, def->argsize, "param"); + } else + arg = def->arg; + + if (!def->cb(buffer, tok, arg)) { + command_fail_detailed(cmd, JSONRPC2_INVALID_PARAMS, + make_result(cmd, def->name, buffer, tok), + find_fail_format(def->cb), def->name, + tok->end - tok->start, + buffer + tok->start); + return false; + } + } else { + return def->cbx(cmd, def->name, buffer, tok, def->arg); } return true; } @@ -308,10 +325,12 @@ bool param(struct command *cmd, const char *buffer, va_start(ap, tokens); while ((name = va_arg(ap, const char *)) != NULL) { bool required = va_arg(ap, int); - param_cb cb = va_arg(ap, param_cb); + bool advanced = va_arg(ap, int); + param_cb cb = advanced ? NULL : va_arg(ap, param_cb); + param_cbx cbx = advanced ? va_arg(ap, param_cbx) : NULL; void *arg = va_arg(ap, void *); size_t argsize = va_arg(ap, size_t); - if (!param_add(¶ms, name, required, cb, arg, argsize)) { + if (!param_add(¶ms, name, required, cb, cbx, arg, argsize)) { command_fail(cmd, PARAM_DEV_ERROR, "developer error"); va_end(ap); return false; diff --git a/lightningd/param.h b/lightningd/param.h index 2c5c37f6c..383dfe2d7 100644 --- a/lightningd/param.h +++ b/lightningd/param.h @@ -54,6 +54,15 @@ bool param(struct command *cmd, const char *buffer, */ typedef bool(*param_cb)(const char *buffer, const jsmntok_t *tok, void *arg); +/* + * Advanced callback. Returns NULL on success, error message on failure. + */ +typedef bool(*param_cbx)(struct command *cmd, + const char *name, + const char *buffer, + const jsmntok_t *tok, + void **arg); + /* * Add a handler to unmarshal a required json token into @arg. The handler must * return true on success and false on failure. Upon failure, command_fail will be @@ -65,12 +74,25 @@ typedef bool(*param_cb)(const char *buffer, const jsmntok_t *tok, void *arg); #define p_req(name, cb, arg) \ name"", \ true, \ + false, \ (cb), \ (arg) + 0*sizeof((cb)((const char *)NULL, \ (const jsmntok_t *)NULL, \ (arg)) == true), \ (size_t)0 +#define p_req_tal(name, cbx, arg) \ + name"", \ + true, \ + true, \ + (cbx), \ + (arg) + 0*sizeof((cbx)((struct command *)NULL,\ + (const char *)NULL, \ + (const char *)NULL, \ + (const jsmntok_t *)NULL, \ + (arg)) == true), \ + (size_t)0 + /* * Similar to above but for optional parameters. * @arg must be the address of a pointer. If found during parsing, it will be @@ -79,12 +101,25 @@ typedef bool(*param_cb)(const char *buffer, const jsmntok_t *tok, void *arg); #define p_opt(name, cb, arg) \ name"", \ false, \ + false, \ (cb), \ (arg) + 0*sizeof((cb)((const char *)NULL, \ (const jsmntok_t *)NULL, \ *(arg)) == true), \ sizeof(**(arg)) +#define p_opt_tal(name, cbx, arg) \ + name"", \ + false, \ + true, \ + (cbx), \ + (arg) + 0*sizeof((cbx)((struct command *)NULL,\ + (const char *)NULL, \ + (const char *)NULL, \ + (const jsmntok_t *)NULL, \ + (arg)) == true), \ + sizeof(**(arg)) + /* * Similar to p_req but for optional parameters with defaults. * @arg will be set to @def if it isn't found during parsing. @@ -92,12 +127,25 @@ typedef bool(*param_cb)(const char *buffer, const jsmntok_t *tok, void *arg); #define p_opt_def(name, cb, arg, def) \ name"", \ false, \ + false, \ (cb), \ (arg) + 0*sizeof((cb)((const char *)NULL, \ (const jsmntok_t *)NULL, \ (arg)) == true), \ ((void)((*arg) = (def)), (size_t)0) +#define p_opt_def_tal(name, cbx, arg, def) \ + name"", \ + false, \ + true, \ + (cbx), \ + (arg) + 0*sizeof((cbx)((struct command *)NULL,\ + (const char *)NULL, \ + (const char *)NULL, \ + (const jsmntok_t *)NULL, \ + (arg)) == true), \ + ({ (*arg) = tal((cmd), typeof(**arg)); (**arg) = (def); (size_t)0;}) + /* * For when you want an optional raw token. * @@ -106,6 +154,7 @@ typedef bool(*param_cb)(const char *buffer, const jsmntok_t *tok, void *arg); #define p_opt_tok(name, arg) \ name"", \ false, \ + false, \ json_tok_tok, \ (arg) + 0*sizeof(*(arg) == (jsmntok_t *)NULL), \ sizeof(const jsmntok_t *) diff --git a/lightningd/test/run-param.c b/lightningd/test/run-param.c index 1c35b0e83..3be30b128 100644 --- a/lightningd/test/run-param.c +++ b/lightningd/test/run-param.c @@ -17,10 +17,6 @@ static bool check_fail(void) { if (!failed) return false; failed = false; - if (taken(fail_msg)) { - tal_free(fail_msg); - fail_msg = NULL; - } return true; } @@ -324,6 +320,7 @@ static void add_members(struct param **params, &ints[i], const char *, const jsmntok_t *), + NULL, &ints[i], 0); } } @@ -406,6 +403,136 @@ static void sendpay_nulltok(void) assert(msatoshi == NULL); } +static bool json_tok_msat(struct command *cmd, + const char *name, + const char *buffer, + const jsmntok_t * tok, + u64 **msatoshi_val) +{ + if (json_tok_streq(buffer, tok, "any")) { + *msatoshi_val = NULL; + return true; + } + *msatoshi_val = tal(cmd, u64); + + if (json_tok_u64(buffer, tok, *msatoshi_val) && *msatoshi_val != 0) + return true; + + command_fail(cmd, JSONRPC2_INVALID_PARAMS, + "'%s' should be a positive number or 'any', not '%.*s'", + name, + tok->end - tok->start, + buffer + tok->start); + return false; +} + +/* This can eventually replace json_tok_tok and we can remove the special p_opt_tok() + * macro. */ +static bool json_tok_tok_x(struct command *cmd, + const char *name, + const char *buffer, + const jsmntok_t *tok, + const jsmntok_t **arg) +{ + return (*arg = tok); +} + +/* + * New version of json_tok_label conforming to advanced style. This can eventually + * replace the existing json_tok_label. + */ +static bool json_tok_label_x(struct command *cmd, + const char *name, + const char *buffer, + const jsmntok_t *tok, + struct json_escaped **label) +{ + if ((*label = json_tok_escaped_string(cmd, buffer, tok))) + return true; + + /* Allow literal numbers */ + if (tok->type != JSMN_PRIMITIVE) + goto fail; + + for (int i = tok->start; i < tok->end; i++) + if (!cisdigit(buffer[i])) + goto fail; + + if ((*label = json_escaped_string_(cmd, buffer + tok->start, + tok->end - tok->start))) + return true; + +fail: + command_fail(cmd, JSONRPC2_INVALID_PARAMS, + "'%s' should be a string or number, not '%.*s'", + name, tok->end - tok->start, buffer + tok->start); + return false; +} + +static void advanced(void) +{ + { + struct json *j = json_parse(cmd, "[ 'lightning', 24, 'tok', 543 ]"); + + struct json_escaped *label; + u64 *msat; + u64 *msat_opt1, *msat_opt2; + const jsmntok_t *tok; + + assert(param(cmd, j->buffer, j->toks, + p_req_tal("description", json_tok_label_x, &label), + p_req_tal("msat", json_tok_msat, &msat), + p_req_tal("tok", json_tok_tok_x, &tok), + p_opt_tal("msat_opt1", json_tok_msat, &msat_opt1), + p_opt_tal("msat_opt2", json_tok_msat, &msat_opt2), + NULL)); + assert(label != NULL); + assert(!strcmp(label->s, "lightning")); + assert(*msat == 24); + assert(tok); + assert(msat_opt1); + assert(*msat_opt1 == 543); + assert(msat_opt2 == NULL); + } + { + struct json *j = json_parse(cmd, "[ 3 'foo' ]"); + struct json_escaped *label, *foo; + assert(param(cmd, j->buffer, j->toks, + p_req_tal("label", json_tok_label_x, &label), + p_opt_tal("foo", json_tok_label_x, &foo), + NULL)); + assert(!strcmp(label->s, "3")); + assert(!strcmp(foo->s, "foo")); + } + { + u64 *msat; + u64 *msat2; + struct json *j = json_parse(cmd, "[ 3 ]"); + assert(param(cmd, j->buffer, j->toks, + p_opt_def_tal("msat", json_tok_msat, &msat, 23), + p_opt_def_tal("msat2", json_tok_msat, &msat2, 53), + NULL)); + assert(*msat == 3); + assert(msat2); + assert(*msat2 == 53); + } +} + +static void advanced_fail(void) +{ + { + struct json *j = json_parse(cmd, "[ 'anyx' ]"); + u64 *msat; + assert(!param(cmd, j->buffer, j->toks, + p_req_tal("msat", json_tok_msat, &msat), + NULL)); + assert(check_fail()); + assert(strstr(fail_msg, "'msat' should be a positive" + " number or 'any', not 'anyx'")); + } +} + + int main(void) { setup_locale(); @@ -424,6 +551,8 @@ int main(void) five_hundred_params(); sendpay(); sendpay_nulltok(); + advanced(); + advanced_fail(); tal_free(tmpctx); printf("run-params ok\n"); }