commit 95afdb005cce04cfb87df6a75980944173c15ed7 Author: Nick Mathewson nickm@torproject.org Date: Mon Apr 8 16:19:09 2019 -0400

Use new parser logic for SETCONF/RESETCONF code.

Here we get to throw away a LOT of unused code, since most of the old parsing was redundant with kvline. --- src/feature/control/control_cmd.c | 108 +++++++++----------------------------- src/feature/control/control_fmt.c | 14 ----- src/feature/control/control_fmt.h | 2 - 3 files changed, 26 insertions(+), 98 deletions(-)

diff --git a/src/feature/control/control_cmd.c b/src/feature/control/control_cmd.c index 184855535..cfd35e809 100644 --- a/src/feature/control/control_cmd.c +++ b/src/feature/control/control_cmd.c @@ -54,8 +54,8 @@ #include "feature/rend/rend_encoded_v2_service_descriptor_st.h" #include "feature/rend/rend_service_descriptor_st.h"

-static int control_setconf_helper(control_connection_t *conn, uint32_t len, - char *body, +static int control_setconf_helper(control_connection_t *conn, + const control_cmd_args_t *args, int use_defaults);

/** Yield true iff <b>s</b> is the state of a control_connection_t that has @@ -195,22 +195,36 @@ config_lines_contain_flag(const config_line_t *lines, const char *flag) return line && !strcmp(line->value, ""); }

+static const control_cmd_syntax_t setconf_syntax = { + .max_args=0, + .accept_keywords=true, + .kvline_flags=KV_OMIT_VALS|KV_QUOTED, +}; + /** Called when we receive a SETCONF message: parse the body and try * to update our configuration. Reply with a DONE or ERROR message. * Modifies the contents of body.*/ static int -handle_control_setconf(control_connection_t *conn, uint32_t len, char *body) +handle_control_setconf(control_connection_t *conn, + const control_cmd_args_t *args) { - return control_setconf_helper(conn, len, body, 0); + return control_setconf_helper(conn, args, 0); }

+static const control_cmd_syntax_t resetconf_syntax = { + .max_args=0, + .accept_keywords=true, + .kvline_flags=KV_OMIT_VALS|KV_QUOTED, +}; + /** Called when we receive a RESETCONF message: parse the body and try * to update our configuration. Reply with a DONE or ERROR message. * Modifies the contents of body. */ static int -handle_control_resetconf(control_connection_t *conn, uint32_t len, char *body) +handle_control_resetconf(control_connection_t *conn, + const control_cmd_args_t *args) { - return control_setconf_helper(conn, len, body, 1); + return control_setconf_helper(conn, args, 1); }

static const control_cmd_syntax_t getconf_syntax = { @@ -524,73 +538,17 @@ get_stream(const char *id) * contents of body. */ static int -control_setconf_helper(control_connection_t *conn, uint32_t len, char *body, +control_setconf_helper(control_connection_t *conn, + const control_cmd_args_t *args, int use_defaults) { setopt_err_t opt_err; - config_line_t *lines=NULL; - char *start = body; char *errstring = NULL; const unsigned flags = CAL_CLEAR_FIRST | (use_defaults ? CAL_USE_DEFAULTS : 0);

- char *config; - smartlist_t *entries = smartlist_new(); - - /* We have a string, "body", of the format '(key(=val|="val")?)' entries - * separated by space. break it into a list of configuration entries. */ - while (*body) { - char *eq = body; - char *key; - char *entry; - while (!TOR_ISSPACE(*eq) && *eq != '=') - ++eq; - key = tor_strndup(body, eq-body); - body = eq+1; - if (*eq == '=') { - char *val=NULL; - size_t val_len=0; - if (*body != '"') { - char *val_start = body; - while (!TOR_ISSPACE(*body)) - body++; - val = tor_strndup(val_start, body-val_start); - val_len = strlen(val); - } else { - body = (char*)extract_escaped_string(body, (len - (body-start)), - &val, &val_len); - if (!body) { - connection_write_str_to_buf("551 Couldn't parse string\r\n", conn); - SMARTLIST_FOREACH(entries, char *, cp, tor_free(cp)); - smartlist_free(entries); - tor_free(key); - return 0; - } - } - tor_asprintf(&entry, "%s %s", key, val); - tor_free(key); - tor_free(val); - } else { - entry = key; - } - smartlist_add(entries, entry); - while (TOR_ISSPACE(*body)) - ++body; - } - - smartlist_add_strdup(entries, ""); - config = smartlist_join_strings(entries, "\n", 0, NULL); - SMARTLIST_FOREACH(entries, char *, cp, tor_free(cp)); - smartlist_free(entries); - - if (config_get_lines(config, &lines, 0) < 0) { - log_warn(LD_CONTROL,"Controller gave us config lines we can't parse."); - connection_write_str_to_buf("551 Couldn't parse configuration\r\n", - conn); - tor_free(config); - return 0; - } - tor_free(config); + // We need a copy here, since confparse.c wants to canonicalize cases. + config_line_t *lines = config_lines_dup(args->kwargs);

opt_err = options_trial_assign(lines, flags, &errstring); { @@ -2276,7 +2234,6 @@ handle_control_obsolete(control_connection_t *conn, **/ typedef enum handler_type_t { hnd_legacy, - hnd_legacy_mut, hnd_parsed, } handler_type_t;

@@ -2297,15 +2254,6 @@ typedef union handler_fn_t { uint32_t arg_len, const char *args); /** - * A "legacy_mut" handler is the same as a "legacy" one, except that it may - * change the contents of the command's arguments -- for example, by - * inserting NULs. It may not deallocate them. - */ - int (*legacy_mut)(control_connection_t *conn, - uint32_t arg_len, - char *args); - - /** * A "parsed" handler expects its arguments in a pre-parsed format, in * an immutable control_cmd_args_t *object. **/ @@ -2400,8 +2348,8 @@ typedef struct control_cmd_def_t { **/ static const control_cmd_def_t CONTROL_COMMANDS[] = { - ONE_LINE(setconf, legacy_mut, 0), - ONE_LINE(resetconf, legacy_mut, 0), + ONE_LINE_PARSED(setconf, 0), + ONE_LINE_PARSED(resetconf, 0), ONE_LINE_PARSED(getconf, 0), MULTLINE_PARSED(loadconf, 0), ONE_LINE_PARSED(setevents, 0), @@ -2452,10 +2400,6 @@ handle_single_control_command(const control_cmd_def_t *def, if (def->handler.legacy(conn, cmd_data_len, args)) rv = -1; break; - case hnd_legacy_mut: - if (def->handler.legacy_mut(conn, cmd_data_len, args)) - rv = -1; - break; case hnd_parsed: { control_cmd_args_t *parsed_args; char *err=NULL; diff --git a/src/feature/control/control_fmt.c b/src/feature/control/control_fmt.c index 71f9d8216..427f4288f 100644 --- a/src/feature/control/control_fmt.c +++ b/src/feature/control/control_fmt.c @@ -342,20 +342,6 @@ get_escaped_string_length(const char *start, size_t in_len_max, return (int)(cp - start+1); }

-/** As decode_escaped_string, but does not decode the string: copies the - * entire thing, including quotation marks. */ -const char * -extract_escaped_string(const char *start, size_t in_len_max, - char **out, size_t *out_len) -{ - int length = get_escaped_string_length(start, in_len_max, NULL); - if (length<0) - return NULL; - *out_len = length; - *out = tor_strndup(start, *out_len); - return start+length; -} - /** Given a pointer to a string starting at <b>start</b> containing * <b>in_len_max</b> characters, decode a string beginning with one double * quote, containing any number of non-quote characters or characters escaped diff --git a/src/feature/control/control_fmt.h b/src/feature/control/control_fmt.h index 74545eb30..08acf8518 100644 --- a/src/feature/control/control_fmt.h +++ b/src/feature/control/control_fmt.h @@ -25,8 +25,6 @@ char *circuit_describe_status_for_controller(origin_circuit_t *circ);

size_t write_escaped_data(const char *data, size_t len, char **out); size_t read_escaped_data(const char *data, size_t len, char **out); -const char *extract_escaped_string(const char *start, size_t in_len_max, - char **out, size_t *out_len); const char *decode_escaped_string(const char *start, size_t in_len_max, char **out, size_t *out_len); void send_control_done(control_connection_t *conn);