trace2: remove use of xstrfmt()

We continue removing the possibility of a die() in the trace2 API by
banning xstrfmt(), which calls die() during a failure to format. Instead
of allowing a die(), perform a soft failure by failing to output the
trace2 data when such a failure occurs.

This requires carefully concatenating strings using memcpy() to
construct redacted data to avoid copying password information in traced
URLs.

Update t0212 to more carefully test this behavior to explicitly include
the ":<REDACTED>" string in the appropriate context.

Helped-by: Elijah Newren <newren@gmail.com>
Signed-off-by: Derrick Stolee <stolee@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

banned-die.h

@@ -17,6 +17,9 @@
 #undef xstrdup
 #define xstrdup(str) BANNED(xstrdup)
 
+#undef xstrfmt
+#define xstrfmt(...) BANNED(xstrfmt)
+
 #undef ALLOC_ARRAY
 #define ALLOC_ARRAY(x, alloc) BANNED(ALLOC_ARRAY)
 

t/t0212-trace2-event.sh

@@ -332,7 +332,8 @@ test_expect_success 'unsafe URLs are redacted by default in cmd_start events' '
 
 	GIT_TRACE2_EVENT="$(pwd)/trace.event" \
 		test-tool trace2 300redact_start git clone https://user:pwd@example.com/ clone2 &&
-	test_grep ! user:pwd trace.event
+	test_grep ! user:pwd trace.event &&
+	test_grep "user:<REDACTED>@example.com/" trace.event
 '
 
 test_expect_success 'unsafe URLs are redacted by default in child_start events' '
@@ -341,7 +342,8 @@ test_expect_success 'unsafe URLs are redacted by default in child_start events'
 
 	GIT_TRACE2_EVENT="$(pwd)/trace.event" \
 		test-tool trace2 301redact_child_start git clone https://user:pwd@example.com/ clone2 &&
-	test_grep ! user:pwd trace.event
+	test_grep ! user:pwd trace.event &&
+	test_grep "user:<REDACTED>@example.com/" trace.event
 '
 
 test_expect_success 'unsafe URLs are redacted by default in exec events' '
@@ -350,7 +352,8 @@ test_expect_success 'unsafe URLs are redacted by default in exec events' '
 
 	GIT_TRACE2_EVENT="$(pwd)/trace.event" \
 		test-tool trace2 302redact_exec git clone https://user:pwd@example.com/ clone2 &&
-	test_grep ! user:pwd trace.event
+	test_grep ! user:pwd trace.event &&
+	test_grep "user:<REDACTED>@example.com/" trace.event
 '
 
 test_expect_success 'unsafe URLs are redacted by default in def_param events' '
@@ -359,7 +362,8 @@ test_expect_success 'unsafe URLs are redacted by default in def_param events' '
 
 	GIT_TRACE2_EVENT="$(pwd)/trace.event" \
 		test-tool trace2 303redact_def_param url https://user:pwd@example.com/ &&
-	test_grep ! user:pwd trace.event
+	test_grep ! user:pwd trace.event &&
+	test_grep "user:<REDACTED>@example.com/" trace.event
 '
 
 test_done

trace2.c

@@ -261,7 +261,10 @@ int trace2_is_enabled(void)
 static const char *redact_arg(const char *arg)
 {
 	const char *p, *colon;
+	const char *redact = ":<REDACTED>";
+	char *redacted;
 	size_t at;
+	size_t prefix_len, suffix_len, redacted_len, redact_len;
 
 	if (!trace2_redact ||
 	    (!skip_prefix(arg, "https://", &p) &&
@@ -276,7 +279,25 @@ static const char *redact_arg(const char *arg)
 	if (!colon)
 		return arg;
 
-	return xstrfmt("%.*s:<REDACTED>%s", (int)(colon - arg), arg, p + at);
+	redact_len = strlen(redact);
+	prefix_len = colon - arg;
+	suffix_len = strlen(p + at);
+
+	if (unsigned_add_overflows(prefix_len, suffix_len) ||
+	    unsigned_add_overflows(prefix_len + suffix_len, redact_len) ||
+	    unsigned_add_overflows(prefix_len + suffix_len + redact_len, 1))
+		return NULL;
+
+	redacted_len = prefix_len + suffix_len + redact_len + 1;
+
+	redacted = malloc(redacted_len);
+	if (!redacted)
+		return NULL;
+
+	memcpy(redacted, arg, prefix_len);
+	memcpy(redacted + prefix_len, redact, redact_len);
+	memcpy(redacted + prefix_len + redact_len, p + at, suffix_len + 1);
+	return redacted;
 }
 
 /*
@@ -301,6 +322,8 @@ static const char **redact_argv(const char **argv)
 
 	if (!argv[i])
 		return argv;
+	if (!redacted)
+		return NULL;
 
 	for (j = 0; argv[j]; j++)
 		; /* keep counting */
@@ -317,7 +340,14 @@ static const char **redact_argv(const char **argv)
 	ret[i] = redacted;
 	for (++i; argv[i]; i++) {
 		redacted = redact_arg(argv[i]);
-		ret[i] = redacted ? redacted : argv[i];
+		if (!redacted) {
+			for (j = 0; j < i; j++)
+				if (ret[j] != argv[j])
+					free((void *)ret[j]);
+			free(ret);
+			return NULL;
+		}
+		ret[i] = redacted;
 	}
 
 	return ret;