Merge branch 'mh/unify-xml-in-imap-send-and-http-push'

Update imap-send to reuse xml quoting code from http-push codepath,
clean up some code, and fix a small bug.

* mh/unify-xml-in-imap-send-and-http-push:
  wrap_in_html(): process message in bulk rather than line-by-line
  wrap_in_html(): use strbuf_addstr_xml_quoted()
  imap-send: change msg_data from storing (ptr, len) to storing strbuf
  imap-send: correctly report errors reading from stdin
  imap-send: store all_msgs as a strbuf
  lf_to_crlf(): NUL-terminate msg_data::data
  xml_entities(): use function strbuf_addstr_xml_quoted()
  Add new function strbuf_add_xml_quoted()

http-push.c

@@ -172,28 +172,7 @@ enum dav_header_flag {
 static char *xml_entities(const char *s)
 {
 	struct strbuf buf = STRBUF_INIT;
-	while (*s) {
+	strbuf_addstr_xml_quoted(&buf, s);
-		size_t len = strcspn(s, "\"<>&");
-		strbuf_add(&buf, s, len);
-		s += len;
-		switch (*s) {
-		case '"':
-			strbuf_addstr(&buf, "&quot;");
-			break;
-		case '<':
-			strbuf_addstr(&buf, "&lt;");
-			break;
-		case '>':
-			strbuf_addstr(&buf, "&gt;");
-			break;
-		case '&':
-			strbuf_addstr(&buf, "&amp;");
-			break;
-		case 0:
-			return strbuf_detach(&buf, NULL);
-		}
-		s++;
-	}
 	return strbuf_detach(&buf, NULL);
 }
 

imap-send.c

@@ -69,8 +69,7 @@ struct store {
 };
 
 struct msg_data {
-	char *data;
+	struct strbuf data;
-	int len;
 	unsigned char flags;
 };
 
@@ -1264,45 +1263,49 @@ static int imap_make_flags(int flags, char *buf)
 	return d;
 }
 
-static void lf_to_crlf(struct msg_data *msg)
+static void lf_to_crlf(struct strbuf *msg)
 {
+	size_t new_len;
 	char *new;
 	int i, j, lfnum = 0;
 
-	if (msg->data[0] == '\n')
+	if (msg->buf[0] == '\n')
 		lfnum++;
 	for (i = 1; i < msg->len; i++) {
-		if (msg->data[i - 1] != '\r' && msg->data[i] == '\n')
+		if (msg->buf[i - 1] != '\r' && msg->buf[i] == '\n')
 			lfnum++;
 	}
 
-	new = xmalloc(msg->len + lfnum);
+	new_len = msg->len + lfnum;
-	if (msg->data[0] == '\n') {
+	new = xmalloc(new_len + 1);
+	if (msg->buf[0] == '\n') {
 		new[0] = '\r';
 		new[1] = '\n';
 		i = 1;
 		j = 2;
 	} else {
-		new[0] = msg->data[0];
+		new[0] = msg->buf[0];
 		i = 1;
 		j = 1;
 	}
 	for ( ; i < msg->len; i++) {
-		if (msg->data[i] != '\n') {
+		if (msg->buf[i] != '\n') {
-			new[j++] = msg->data[i];
+			new[j++] = msg->buf[i];
 			continue;
 		}
-		if (msg->data[i - 1] != '\r')
+		if (msg->buf[i - 1] != '\r')
 			new[j++] = '\r';
 		/* otherwise it already had CR before */
 		new[j++] = '\n';
 	}
-	msg->len += lfnum;
+	strbuf_attach(msg, new, new_len, new_len + 1);
-	free(msg->data);
-	msg->data = new;
 }
 
-static int imap_store_msg(struct store *gctx, struct msg_data *data)
+/*
+ * Store msg to IMAP.  Also detach and free the data from msg->data,
+ * leaving msg->data empty.
+ */
+static int imap_store_msg(struct store *gctx, struct msg_data *msg)
 {
 	struct imap_store *ctx = (struct imap_store *)gctx;
 	struct imap *imap = ctx->imap;
@@ -1311,16 +1314,15 @@ static int imap_store_msg(struct store *gctx, struct msg_data *data)
 	int ret, d;
 	char flagstr[128];
 
-	lf_to_crlf(data);
+	lf_to_crlf(&msg->data);
 	memset(&cb, 0, sizeof(cb));
 
-	cb.dlen = data->len;
+	cb.dlen = msg->data.len;
-	cb.data = xmalloc(cb.dlen);
+	cb.data = strbuf_detach(&msg->data, NULL);
-	memcpy(cb.data, data->data, data->len);
 
 	d = 0;
-	if (data->flags) {
+	if (msg->flags) {
-		d = imap_make_flags(data->flags, flagstr);
+		d = imap_make_flags(msg->flags, flagstr);
 		flagstr[d++] = ' ';
 	}
 	flagstr[d] = 0;
@@ -1337,75 +1339,46 @@ static int imap_store_msg(struct store *gctx, struct msg_data *data)
 	return DRV_OK;
 }
 
-static void encode_html_chars(struct strbuf *p)
+static void wrap_in_html(struct strbuf *msg)
-{
-	int i;
-	for (i = 0; i < p->len; i++) {
-		if (p->buf[i] == '&')
-			strbuf_splice(p, i, 1, "&amp;", 5);
-		if (p->buf[i] == '<')
-			strbuf_splice(p, i, 1, "&lt;", 4);
-		if (p->buf[i] == '>')
-			strbuf_splice(p, i, 1, "&gt;", 4);
-		if (p->buf[i] == '"')
-			strbuf_splice(p, i, 1, "&quot;", 6);
-	}
-}
-static void wrap_in_html(struct msg_data *msg)
 {
 	struct strbuf buf = STRBUF_INIT;
-	struct strbuf **lines;
-	struct strbuf **p;
 	static char *content_type = "Content-Type: text/html;\n";
 	static char *pre_open = "<pre>\n";
 	static char *pre_close = "</pre>\n";
-	int added_header = 0;
+	const char *body = strstr(msg->buf, "\n\n");
 
-	strbuf_attach(&buf, msg->data, msg->len, msg->len);
+	if (!body)
-	lines = strbuf_split(&buf, '\n');
+		return; /* Headers but no body; no wrapping needed */
-	strbuf_release(&buf);
+
-	for (p = lines; *p; p++) {
+	body += 2;
-		if (! added_header) {
+
-			if ((*p)->len == 1 && *((*p)->buf) == '\n') {
+	strbuf_add(&buf, msg->buf, body - msg->buf - 1);
-				strbuf_addstr(&buf, content_type);
+	strbuf_addstr(&buf, content_type);
-				strbuf_addbuf(&buf, *p);
+	strbuf_addch(&buf, '\n');
-				strbuf_addstr(&buf, pre_open);
+	strbuf_addstr(&buf, pre_open);
-				added_header = 1;
+	strbuf_addstr_xml_quoted(&buf, body);
-				continue;
-			}
-		}
-		else
-			encode_html_chars(*p);
-		strbuf_addbuf(&buf, *p);
-	}
 	strbuf_addstr(&buf, pre_close);
-	strbuf_list_free(lines);
+
-	msg->len  = buf.len;
+	strbuf_release(msg);
-	msg->data = strbuf_detach(&buf, NULL);
+	*msg = buf;
 }
 
 #define CHUNKSIZE 0x1000
 
-static int read_message(FILE *f, struct msg_data *msg)
+static int read_message(FILE *f, struct strbuf *all_msgs)
 {
-	struct strbuf buf = STRBUF_INIT;
-
-	memset(msg, 0, sizeof(*msg));
-
 	do {
-		if (strbuf_fread(&buf, CHUNKSIZE, f) <= 0)
+		if (strbuf_fread(all_msgs, CHUNKSIZE, f) <= 0)
 			break;
 	} while (!feof(f));
 
-	msg->len  = buf.len;
+	return ferror(f) ? -1 : 0;
-	msg->data = strbuf_detach(&buf, NULL);
-	return msg->len;
 }
 
-static int count_messages(struct msg_data *msg)
+static int count_messages(struct strbuf *all_msgs)
 {
 	int count = 0;
-	char *p = msg->data;
+	char *p = all_msgs->buf;
 
 	while (1) {
 		if (!prefixcmp(p, "From ")) {
@@ -1426,34 +1399,39 @@ static int count_messages(struct msg_data *msg)
 	return count;
 }
 
-static int split_msg(struct msg_data *all_msgs, struct msg_data *msg, int *ofs)
+/*
+ * Copy the next message from all_msgs, starting at offset *ofs, to
+ * msg.  Update *ofs to the start of the following message.  Return
+ * true iff a message was successfully copied.
+ */
+static int split_msg(struct strbuf *all_msgs, struct strbuf *msg, int *ofs)
 {
 	char *p, *data;
+	size_t len;
 
-	memset(msg, 0, sizeof *msg);
 	if (*ofs >= all_msgs->len)
 		return 0;
 
-	data = &all_msgs->data[*ofs];
+	data = &all_msgs->buf[*ofs];
-	msg->len = all_msgs->len - *ofs;
+	len = all_msgs->len - *ofs;
 
-	if (msg->len < 5 || prefixcmp(data, "From "))
+	if (len < 5 || prefixcmp(data, "From "))
 		return 0;
 
 	p = strchr(data, '\n');
 	if (p) {
-		p = &p[1];
+		p++;
-		msg->len -= p-data;
+		len -= p - data;
-		*ofs += p-data;
+		*ofs += p - data;
 		data = p;
 	}
 
 	p = strstr(data, "\nFrom ");
 	if (p)
-		msg->len = &p[1] - data;
+		len = &p[1] - data;
 
-	msg->data = xmemdupz(data, msg->len);
+	strbuf_add(msg, data, len);
-	*ofs += msg->len;
+	*ofs += len;
 	return 1;
 }
 
@@ -1504,7 +1482,8 @@ static int git_imap_config(const char *key, const char *val, void *cb)
 
 int main(int argc, char **argv)
 {
-	struct msg_data all_msgs, msg;
+	struct strbuf all_msgs = STRBUF_INIT;
+	struct msg_data msg = {STRBUF_INIT, 0};
 	struct store *ctx = NULL;
 	int ofs = 0;
 	int r;
@@ -1537,7 +1516,12 @@ int main(int argc, char **argv)
 	}
 
 	/* read the messages */
-	if (!read_message(stdin, &all_msgs)) {
+	if (read_message(stdin, &all_msgs)) {
+		fprintf(stderr, "error reading input\n");
+		return 1;
+	}
+
+	if (all_msgs.len == 0) {
 		fprintf(stderr, "nothing to send\n");
 		return 1;
 	}
@@ -1559,11 +1543,12 @@ int main(int argc, char **argv)
 	ctx->name = imap_folder;
 	while (1) {
 		unsigned percent = n * 100 / total;
+
 		fprintf(stderr, "%4u%% (%d/%d) done\r", percent, n, total);
-		if (!split_msg(&all_msgs, &msg, &ofs))
+		if (!split_msg(&all_msgs, &msg.data, &ofs))
 			break;
 		if (server.use_html)
-			wrap_in_html(&msg);
+			wrap_in_html(&msg.data);
 		r = imap_store_msg(ctx, &msg);
 		if (r != DRV_OK)
 			break;

strbuf.c

@@ -425,6 +425,32 @@ void strbuf_add_lines(struct strbuf *out, const char *prefix,
 	strbuf_complete_line(out);
 }
 
+void strbuf_addstr_xml_quoted(struct strbuf *buf, const char *s)
+{
+	while (*s) {
+		size_t len = strcspn(s, "\"<>&");
+		strbuf_add(buf, s, len);
+		s += len;
+		switch (*s) {
+		case '"':
+			strbuf_addstr(buf, "&quot;");
+			break;
+		case '<':
+			strbuf_addstr(buf, "&lt;");
+			break;
+		case '>':
+			strbuf_addstr(buf, "&gt;");
+			break;
+		case '&':
+			strbuf_addstr(buf, "&amp;");
+			break;
+		case 0:
+			return;
+		}
+		s++;
+	}
+}
+
 static int is_rfc3986_reserved(char ch)
 {
 	switch (ch) {

strbuf.h

@@ -136,6 +136,12 @@ extern void strbuf_vaddf(struct strbuf *sb, const char *fmt, va_list ap);
 
 extern void strbuf_add_lines(struct strbuf *sb, const char *prefix, const char *buf, size_t size);
 
+/*
+ * Append s to sb, with the characters '<', '>', '&' and '"' converted
+ * into XML entities.
+ */
+extern void strbuf_addstr_xml_quoted(struct strbuf *sb, const char *s);
+
 static inline void strbuf_complete_line(struct strbuf *sb)
 {
 	if (sb->len && sb->buf[sb->len - 1] != '\n')