From c54e5ad9cc60bc81ffdcd31e7d4d007d4bd592e1 Mon Sep 17 00:00:00 2001 From: Willy Tarreau Date: Thu, 25 Jun 2020 09:15:40 +0200 Subject: [PATCH] MINOR: cfgparse: sanitize the output a little bit With the rework of the config line parser, we've started to emit a dump of the initial line underlined by a caret character indicating the error location. But with extremely large lines it starts to take time and can even cause trouble to slow terminals (e.g. over ssh), and this becomes useless. In addition, control characters could be dumped as-is which is bad, especially when the input file is accidently wrong (an executable). This patch adds a string sanitization function which isolates an area around the error position in order to report only that area if the string is too large. The limit was set to 80 characters, which will result in roughly 40 chars around the error being reported only, prefixed and suffixed with "..." as needed. In addition, non-printable characters in the line are now replaced with '?' so as not to corrupt the terminal. This way invalid variable names, unmatched quotes etc will be easier to spot. A typical output is now: [ALERT] 176/092336 (23852) : parsing [bad.cfg:8]: forbidden first char in environment variable name at position 811957: ...c$PATH$PATH$d(xlc`%?$PATH$PATH$dgc?T$%$P?AH?$PATH$PATH$d(?$PATH$PATH$dgc?%... ^ --- include/haproxy/tools.h | 1 + src/cfgparse.c | 24 ++++++++++++++-------- src/tools.c | 45 +++++++++++++++++++++++++++++++++++++++++ 3 files changed, 62 insertions(+), 8 deletions(-) diff --git a/include/haproxy/tools.h b/include/haproxy/tools.h index 6bbbb4612..6c9f38dbc 100644 --- a/include/haproxy/tools.h +++ b/include/haproxy/tools.h @@ -862,6 +862,7 @@ int my_unsetenv(const char *name); */ char *env_expand(char *in); uint32_t parse_line(char *in, char *out, size_t *outlen, char **args, int *nbargs, uint32_t opts, char **errptr); +size_t sanitize_for_printing(char *line, size_t pos, size_t width); /* debugging macro to emit messages using write() on fd #-1 so that strace sees * them. diff --git a/src/cfgparse.c b/src/cfgparse.c index 6525806e0..92f17691c 100644 --- a/src/cfgparse.c +++ b/src/cfgparse.c @@ -1945,32 +1945,40 @@ next_line: PARSE_OPT_BKSLASH | PARSE_OPT_SHARP, &errptr); if (err & PARSE_ERR_QUOTE) { - ha_alert("parsing [%s:%d]: unmatched quote below:\n" - " %s\n %*s\n", file, linenum, line, (int)(errptr-line+1), "^"); + size_t newpos = sanitize_for_printing(line, errptr - line, 80); + + ha_alert("parsing [%s:%d]: unmatched quote at position %d:\n" + " %s\n %*s\n", file, linenum, (int)(errptr-thisline+1), line, (int)(newpos+1), "^"); err_code |= ERR_ALERT | ERR_FATAL; fatal++; goto next_line; } if (err & PARSE_ERR_BRACE) { - ha_alert("parsing [%s:%d]: unmatched brace in environment variable name below:\n" - " %s\n %*s\n", file, linenum, line, (int)(errptr-line+1), "^"); + size_t newpos = sanitize_for_printing(line, errptr - line, 80); + + ha_alert("parsing [%s:%d]: unmatched brace in environment variable name at position %d:\n" + " %s\n %*s\n", file, linenum, (int)(errptr-thisline+1), line, (int)(newpos+1), "^"); err_code |= ERR_ALERT | ERR_FATAL; fatal++; goto next_line; } if (err & PARSE_ERR_VARNAME) { - ha_alert("parsing [%s:%d]: forbidden first char in environment variable name below:\n" - " %s\n %*s\n", file, linenum, line, (int)(errptr-line+1), "^"); + size_t newpos = sanitize_for_printing(line, errptr - line, 80); + + ha_alert("parsing [%s:%d]: forbidden first char in environment variable name at position %d:\n" + " %s\n %*s\n", file, linenum, (int)(errptr-thisline+1), line, (int)(newpos+1), "^"); err_code |= ERR_ALERT | ERR_FATAL; fatal++; goto next_line; } if (err & PARSE_ERR_HEX) { - ha_alert("parsing [%s:%d]: truncated or invalid hexadecimal sequence below:\n" - " %s\n %*s\n", file, linenum, line, (int)(errptr-line+1), "^"); + size_t newpos = sanitize_for_printing(line, errptr - line, 80); + + ha_alert("parsing [%s:%d]: truncated or invalid hexadecimal sequence at position %d:\n" + " %s\n %*s\n", file, linenum, (int)(errptr-thisline+1), line, (int)(newpos+1), "^"); err_code |= ERR_ALERT | ERR_FATAL; fatal++; goto next_line; diff --git a/src/tools.c b/src/tools.c index 964170065..fa92db58b 100644 --- a/src/tools.c +++ b/src/tools.c @@ -4990,6 +4990,51 @@ uint32_t parse_line(char *in, char *out, size_t *outlen, char **args, int *nbarg } #undef EMIT_CHAR +/* This is used to sanitize an input line that's about to be used for error reporting. + * It will adjust to print approximately chars around , trying to + * preserve the beginning, with leading or trailing "..." when the line is truncated. + * If non-printable chars are present in the output. It returns the new offset + * in the modified line. Non-printable characters are replaced with '?'. must + * be at least 6 to support two "..." otherwise the result is undefined. The line + * itself must have at least 7 chars allocated for the same reason. + */ +size_t sanitize_for_printing(char *line, size_t pos, size_t width) +{ + size_t shift = 0; + char *out = line; + char *in = line; + char *end = line + width; + + if (pos >= width) { + /* if we have to shift, we'll be out of context, so let's + * try to put at the center of width. + */ + shift = pos - width / 2; + in += shift + 3; + end = out + width - 3; + out[0] = out[1] = out[2] = '.'; + out += 3; + } + + while (out < end && *in) { + if (isspace((unsigned char)*in)) + *out++ = ' '; + else if (isprint((unsigned char)*in)) + *out++ = *in; + else + *out++ = '?'; + in++; + } + + if (end < line + width) { + out[0] = out[1] = out[2] = '.'; + out += 3; + } + + *out++ = 0; + return pos - shift; +} + /* * Local variables: * c-indent-level: 8