On 5/9/23 11:51, Richard W.M. Jones wrote: > eg. { '\r', '\n' } -> { '\\', 'n', '\\', 'r' } > --- > common/utils/utils.h | 1 + > common/utils/quote.c | 58 ++++++++++++++++++++++++++++++++++++++++++++ > 2 files changed, 59 insertions(+) > > diff --git a/common/utils/utils.h b/common/utils/utils.h > index 42288a5cd..1e6d8972b 100644 > --- a/common/utils/utils.h > +++ b/common/utils/utils.h > @@ -35,6 +35,7 @@ > > extern void shell_quote (const char *str, FILE *fp); > extern void uri_quote (const char *str, FILE *fp); > +extern void c_string_quote (const char *str, FILE *fp); > extern int exit_status_to_nbd_error (int status, const char *cmd); > extern int set_cloexec (int fd); > extern int set_nonblock (int fd); > diff --git a/common/utils/quote.c b/common/utils/quote.c > index 863e08a8e..877035387 100644 > --- a/common/utils/quote.c > +++ b/common/utils/quote.c > @@ -37,6 +37,9 @@ > #include <string.h> > #include <unistd.h> > > +#include "ascii-ctype.h" > +#include "hexdigit.h" > + > /* Print str to fp, shell quoting if necessary. This comes from > * libguestfs, but was written by me so I'm relicensing it to a BSD > * license for nbdkit. > @@ -98,3 +101,58 @@ uri_quote (const char *str, FILE *fp) > fprintf (fp, "%%%02X", str[i] & 0xff); > } > } > + > +/* Print str to fp, quoting in a way similar to C strings, for example > + * '\n' -> "\n". > + * > + * Note that we do not emit quotes around the string, and double > + * quotes within the string are not escaped. > + */ > +void > +c_string_quote (const char *str, FILE *fp) > +{ > + size_t i; > + char c; > + > + for (i = 0; c = str[i], c != '\0'; ++i) { For the controlling expression, do we like (c = str[i]) != '\0' more?

Another question regarding brace style: > + if (ascii_isprint (c)) > + fputc (c, fp); > + else { > + switch (c) { > + case '\a': > + fputc ('\\', fp); > + fputc ('a', fp); > + break; > + case '\b': > + fputc ('\\', fp); > + fputc ('b', fp); > + break; > + case '\f': > + fputc ('\\', fp); > + fputc ('f', fp); > + break; > + case '\n': > + fputc ('\\', fp); > + fputc ('n', fp); > + break; > + case '\r': > + fputc ('\\', fp); > + fputc ('r', fp); > + break; > + case '\t': > + fputc ('\\', fp); > + fputc ('t', fp); > + break; > + case '\v': > + fputc ('\\', fp); > + fputc ('v', fp); > + break; > + default: > + fputc ('\\', fp); > + fputc ('x', fp); > + fputc (hexchar (c >> 4), fp); > + fputc (hexchar (c), fp); > + } > + } > + } In theory, we don't have to use any brace characters here except within the "switch" statement -- the "for" and "else" parts don't need braces. libnbd / nbdkit usually (?) forbids using braces unless we *must* braces. But this looks like an exception. What's the rule? (Sorry if I asked the same question before -- I don't remember exactly, but I might have. It just doesn't congeal into a consistent rule for me.)

Looks OK otherwise of course. Thanks! Laszlo

Debug strings contain all kinds of information including some under user control. Previously we simply sent everything to stderr, but this is potentially insecure, as well as not dealing well with non-printable characters. Escape these strings when printing. --- server/debug.c | 52 ++++++++++++++++++++++++++++++-------------------- 1 file changed, 31 insertions(+), 21 deletions(-) diff --git a/server/debug.c b/server/debug.c index 2231d40a0..1fc56ec50 100644 --- a/server/debug.c +++ b/server/debug.c @@ -42,6 +42,7 @@ #include "ansi-colours.h" #include "open_memstream.h" +#include "utils.h" #include "internal.h" @@ -69,19 +70,22 @@ prologue (FILE *fp) static void debug_common (bool in_server, const char *fs, va_list args) { - int err = errno; -#ifndef WIN32 - int tty; -#endif - CLEANUP_FREE char *str = NULL; - size_t len = 0; - FILE *fp; - if (!verbose) return; - fp = open_memstream (&str, &len); - if (fp == NULL) { + int err = errno; + +#ifndef WIN32 + int tty; +#endif + CLEANUP_FREE char *str_inner = NULL; + CLEANUP_FREE char *str_outer = NULL; + FILE *fp_inner, *fp_outer; + size_t len_inner = 0, len_outer = 0; + + /* The "inner" string is the debug string before escaping. */ + fp_inner = open_memstream (&str_inner, &len_inner); + if (fp_inner == NULL) { fail: /* Try to emit what we can. */ errno = err; @@ -89,28 +93,34 @@ debug_common (bool in_server, const char *fs, va_list args) fprintf (stderr, "\n"); return; } + errno = err; + vfprintf (fp_inner, fs, args); + close_memstream (fp_inner); + + /* The "outer" string contains the prologue, escaped debug string and \n. */ + fp_outer = open_memstream (&str_outer, &len_outer); + if (fp_outer == NULL) goto fail; #ifndef WIN32 tty = isatty (fileno (stderr)); - if (!in_server && tty) ansi_force_colour (ANSI_FG_BOLD_BLACK, fp); + if (!in_server && tty) ansi_force_colour (ANSI_FG_BOLD_BLACK, fp_outer); #endif - prologue (fp); - - errno = err; - vfprintf (fp, fs, args); + prologue (fp_outer); + c_string_quote (str_inner, fp_outer); #ifndef WIN32 - if (!in_server && tty) ansi_force_restore (fp); + if (!in_server && tty) ansi_force_restore (fp_outer); #endif - fprintf (fp, "\n"); - close_memstream (fp); + fprintf (fp_outer, "\n"); + close_memstream (fp_outer); - if (str) - fputs (str, stderr); - else + if (!str_outer) goto fail; + /* Send it to stderr as atomically as possible. */ + fputs (str_outer, stderr); + errno = err; }