From b5a9e435c6dfb40df0a27521c1c6590c8f68ffb2 Mon Sep 17 00:00:00 2001 From: Jeff King Date: Wed, 11 Jan 2017 09:02:03 -0500 Subject: Revert "vreportf: avoid intermediate buffer" This reverts commit f4c3edc0b156362a92bf9de4f0ec794e90a757fc. The purpose of that commit was to let us write errors of arbitrary length to stderr by skipping the intermediate buffer and sending our varargs straight to fprintf. That works, but it comes with a downside: we do not get access to the varargs before they are sent to stderr. On balance, it's not a good tradeoff. Error messages larger than our 4K buffer are quite uncommon, and we've lost the ability to make any modifications to the output (e.g., to remove non-printable characters). The only way to have both would be one of: 1. Write into a dynamic buffer. But this is a bad idea for a low-level function that may be called when malloc() has failed. 2. Do our own printf-format varargs parsing. This is too complex to be worth the trouble. Let's just revert that change and go back to a fixed buffer. Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- usage.c | 15 +++------------ 1 file changed, 3 insertions(+), 12 deletions(-) (limited to 'usage.c') diff --git a/usage.c b/usage.c index 82ff13163b..e4fa6d2f03 100644 --- a/usage.c +++ b/usage.c @@ -7,21 +7,13 @@ #include "cache.h" static FILE *error_handle; -static int tweaked_error_buffering; void vreportf(const char *prefix, const char *err, va_list params) { + char msg[4096]; FILE *fh = error_handle ? error_handle : stderr; - - fflush(fh); - if (!tweaked_error_buffering) { - setvbuf(fh, NULL, _IOLBF, 0); - tweaked_error_buffering = 1; - } - - fputs(prefix, fh); - vfprintf(fh, err, params); - fputc('\n', fh); + vsnprintf(msg, sizeof(msg), err, params); + fprintf(fh, "%s%s\n", prefix, msg); } static NORETURN void usage_builtin(const char *err, va_list params) @@ -78,7 +70,6 @@ void set_die_is_recursing_routine(int (*routine)(void)) void set_error_handle(FILE *fh) { error_handle = fh; - tweaked_error_buffering = 0; } void NORETURN usagef(const char *err, ...) -- cgit v1.2.3 From f290089879501a855df2eb41db5b38cb0035a765 Mon Sep 17 00:00:00 2001 From: Jeff King Date: Wed, 11 Jan 2017 09:02:23 -0500 Subject: vreport: sanitize ASCII control chars Our error() and die() calls may report messages with arbitrary data (e.g., filenames or even data from a remote server). Let's make it harder to cause confusion with mischievous filenames. E.g., try: git rev-parse "$(printf "\rfatal: this argument is too sneaky")" -- or git rev-parse "$(printf "\x1b[5mblinky\x1b[0m")" -- Let's block all ASCII control characters, with the exception of TAB and LF. We use both in our own messages (and we are necessarily sanitizing the complete output of snprintf here, as we do not have access to the individual varargs). And TAB and LF are unlikely to cause confusion (you could put "\nfatal: sneaky\n" in your filename, but it would at least not _cover up_ the message leading to it, unlike "\r"). We'll replace the characters with a "?", which is similar to how "ls" behaves. It might be nice to do something less lossy, like converting them to "\x" hex codes. But replacing with a single character makes it easy to do in-place and without worrying about length limitations. This feature should kick in rarely enough that the "?" marks are almost never seen. We'll leave high-bit characters as-is, as they are likely to be UTF-8 (though there may be some Unicode mischief you could cause, which may require further patches). Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- usage.c | 6 ++++++ 1 file changed, 6 insertions(+) (limited to 'usage.c') diff --git a/usage.c b/usage.c index e4fa6d2f03..50a6ccee44 100644 --- a/usage.c +++ b/usage.c @@ -12,7 +12,13 @@ void vreportf(const char *prefix, const char *err, va_list params) { char msg[4096]; FILE *fh = error_handle ? error_handle : stderr; + char *p; + vsnprintf(msg, sizeof(msg), err, params); + for (p = msg; *p; p++) { + if (iscntrl(*p) && *p != '\t' && *p != '\n') + *p = '?'; + } fprintf(fh, "%s%s\n", prefix, msg); } -- cgit v1.2.3