git/list[1] front-page[2] threads[3] people[4] search[5] about
 

[PATCH 2/5] log_write_email_headers: use strbufs

From
Jeff King <peff@peff.net>
Date
May 19, 2018, 01:57 UTC
Message-ID
<20180519015743.GB32492@sigill.intra.peff.net>
In-Reply-To
<20180519015444.GA12080@sigill.intra.peff.net>

When we write a MIME attachment, we write the mime headers into fixed-size buffers. These are likely to be big enough in practice, but technically the input could be arbitrarily large (e.g., if the caller provided a lot of content in the extra_headers string), in which case we'd quietly truncate it and generate bogus output. Let's convert these buffers to strbufs.

The memory ownership here is a bit funny. The original fixed buffers were static, and we merely pass out pointers to them to be used by the caller (and in one case, we even just stuff our value into the opt->diffopt.stat_sep value). Ideally we'd actually pass back heap buffers, and the caller would be responsible for freeing them.

This patch punts on that cleanup for now, and instead just marks the strbufs as static. That means we keep ownership in this function, making it not a complete leak. This also takes us one step closer to fixing it in the long term (since we can eventually use strbuf_detach() to hand ownership to the caller, once it's ready).

Signed-off-by: Jeff King <peff@peff.net>
---
The rest of that cleanup is a possible #leftoverbits.
 log-tree.c | 16 ++++++++++------
 1 file changed, 10 insertions(+), 6 deletions(-)
diff --git a/log-tree.c b/log-tree.c
index d1c0bedf24..1173fdb057 100644
--- a/log-tree.c
+++ b/log-tree.c
@@ -386,11 +386,15 @@ void log_write_email_headers(struct rev_info *opt, struct commit *commit,
 		graph_show_oneline(opt->graph);
 	}
 	if (opt->mime_boundary) {
-		static char subject_buffer[1024];
-		static char buffer[1024];
+		static struct strbuf subject_buffer = STRBUF_INIT;
+		static struct strbuf buffer = STRBUF_INIT;
 		struct strbuf filename =  STRBUF_INIT;
 		*need_8bit_cte_p = -1; /* NEVER */
-		snprintf(subject_buffer, sizeof(subject_buffer) - 1,
+
+		strbuf_reset(&subject_buffer);
+		strbuf_reset(&buffer);
+
+		strbuf_addf(&subject_buffer,
 			 "%s"
 			 "MIME-Version: 1.0\n"
 			 "Content-Type: multipart/mixed;"
@@ -405,13 +409,13 @@ void log_write_email_headers(struct rev_info *opt, struct commit *commit,
 			 extra_headers ? extra_headers : "",
 			 mime_boundary_leader, opt->mime_boundary,
 			 mime_boundary_leader, opt->mime_boundary);
-		extra_headers = subject_buffer;
+		extra_headers = subject_buffer.buf;
 
 		if (opt->numbered_files)
 			strbuf_addf(&filename, "%d", opt->nr);
 		else
 			fmt_output_commit(&filename, commit, opt);
-		snprintf(buffer, sizeof(buffer) - 1,
+		strbuf_addf(&buffer,
 			 "\n--%s%s\n"
 			 "Content-Type: text/x-patch;"
 			 " name=\"%s\"\n"
@@ -422,7 +426,7 @@ void log_write_email_headers(struct rev_info *opt, struct commit *commit,
 			 filename.buf,
 			 opt->no_inline ? "attachment" : "inline",
 			 filename.buf);
-		opt->diffopt.stat_sep = buffer;
+		opt->diffopt.stat_sep = buffer.buf;
 		strbuf_release(&filename);
 	}
 	*extra_headers_p = extra_headers;
-- 
2.17.0.1052.g7d69f75dbf
Previous: Stefan BellerNext: Jeff King
Message 6 of 13 in “snprintf truncation fixes”
  1. 0/5 snprintf truncation fixesJeff King, May 19, 2018
  2. 1/5 http: use strbufs instead of fixed buffersJeff King, May 19, 2018
  3. Stefan BellerMay 21, 2018
  4. Jeff KingMay 21, 2018
  5. Stefan BellerMay 21, 2018
  6. 2/5 log_write_email_headers: use strbufsJeff King, May 19, 2018
  7. 3/5 query_fsmonitor: use xsnprintf for formatting integersJeff King, May 19, 2018
  8. René ScharfeMay 19, 2018
  9. Jeff KingMay 20, 2018
  10. Junio C HamanoMay 21, 2018
  11. Ben PeartMay 21, 2018
  12. 4/5 shorten_unambiguous_ref: use xsnprintfJeff King, May 19, 2018
  13. 5/5 fmt_with_err: add a comment that truncation is OKJeff King, May 19, 2018

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.