git imap-send rejected patches sent with git send-email, such as ones fetched from lore with b4, because it expected Date before Subject.
Changes in v3:
* Split concerns into different commits, commit message explains what they
do.
* Handle tunnel leak in the proper place.
Changes in v2:
* Case insensitive header match according to RFC 822/2822/5322.
Harald Nordgren (4):
imap-send: wait for the tunnel command to finish
imap-send: accept messages with Subject before Date
imap-send: check git send-email copies like other messages
imap-send: match header names case insensitively
imap-send.c | 57 ++++++++++++++++++++++---------------------
t/t9001-send-email.sh | 35 ++++++++++++++++++++++++++
2 files changed, 64 insertions(+), 28 deletions(-)
base-commit: 47fdf2208c5b94c8cc01c106b8f7c85efb91c926
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-2456%2FHaraldNordgren%2Ffix%2Fimap-send-header-order-v3
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-2456/HaraldNordgren/fix/imap-send-header-order-v3
Pull-Request: https://github.com/git/git/pull/2456
Range-diff vs v2:
1: b170b1f6d3 ! 1: 0f446c3c12 imap-send: avoid leaking the tunnel command
@@ Metadata
Author: Harald Nordgren <haraldnordgren@gmail.com>
## Commit message ##
- imap-send: avoid leaking the tunnel command
+ imap-send: wait for the tunnel command to finish
- When imap.tunnel is set, imap_open_store() starts the tunnel but never
- releases the child_process used for it, which leaks its argument list.
- No test sets imap.tunnel, so the leak checker has not caught this.
+ When imap.tunnel is set, git imap-send starts the tunnel command but
+ never waits for it to exit, and leaks the argument list it was started
+ with. The isync code that imap-send was derived from forked the tunnel
+ without ever reaping it, and c94d2dd080 (imap-send: use run-command
+ API for tunneling, 2009-10-19) carried that over.
- Only the pipes to the tunnel are needed once it is running, so release
- the rest right after starting it.
+ Wait for the tunnel when closing the connection. By then imap-send has
+ closed its end of the pipes, so the tunnel sees the end of its input
+ and exits. No test sets imap.tunnel, so the leak checker has not
+ caught this.
Signed-off-by: Harald Nordgren <haraldnordgren@gmail.com>
## imap-send.c ##
+@@ imap-send.c: struct imap {
+ /* command queue */
+ int nexttag, num_in_progress, literal_pending;
+ struct imap_cmd *in_progress, **in_progress_append;
++ struct child_process tunnel;
+ struct imap_buffer buf; /* this is BIG, so put it last */
+ };
+
+@@ imap-send.c: static void imap_close_server(struct imap_store *ictx)
+ imap_exec(ictx, NULL, "LOGOUT");
+ socket_shutdown(&imap->buf.sock);
+ }
++ if (ictx->cfg->tunnel)
++ finish_command(&imap->tunnel);
+ free(imap);
+ }
+
@@ imap-send.c: static struct imap_store *imap_open_store(struct imap_server_conf *srvc, const c
+ /* open connection to IMAP server */
+
+ if (srvc->tunnel) {
+- struct child_process tunnel = CHILD_PROCESS_INIT;
++ struct child_process *tunnel = &imap->tunnel;
+
+ imap_info("Starting tunnel '%s'... ", srvc->tunnel);
+
+- strvec_push(&tunnel.args, srvc->tunnel);
+- tunnel.use_shell = 1;
+- tunnel.in = -1;
+- tunnel.out = -1;
+- if (start_command(&tunnel))
++ child_process_init(tunnel);
++ strvec_push(&tunnel->args, srvc->tunnel);
++ tunnel->use_shell = 1;
++ tunnel->in = -1;
++ tunnel->out = -1;
++ if (start_command(tunnel))
+ die("cannot start proxy %s", srvc->tunnel);
- imap->buf.sock.fd[0] = tunnel.out;
- imap->buf.sock.fd[1] = tunnel.in;
-+ child_process_clear(&tunnel);
+- imap->buf.sock.fd[0] = tunnel.out;
+- imap->buf.sock.fd[1] = tunnel.in;
++ imap->buf.sock.fd[0] = tunnel->out;
++ imap->buf.sock.fd[1] = tunnel->in;
imap_info("OK\n");
} else {
2: c42827a122 ! 2: 38214d9abf imap-send: accept messages with Subject before Date
@@ Commit message
and git send-email writes Subject before Date.
Accept the three headers in any order within the header block of each
- message, and match their names case insensitively as RFC 5322
- specifies. The copy that git send-email --imap-sent-folder pipes to
- imap-send then passes the same check, so the special case added for it
- in 04133f5bc4 (send-email: add ability to send a copy of sent emails to
- an IMAP folder, 2025-08-12) is no longer needed.
+ message.
Signed-off-by: Harald Nordgren <haraldnordgren@gmail.com>
@@ imap-send.c: static void wrap_in_html(struct strbuf *msg)
+ bool from = false, date = false, subject = false;
+
+ while ((line = strchr(line, '\n')) && *++line != '\n') {
-+ if (istarts_with(line, "From: "))
++ if (starts_with(line, "From: "))
+ from = true;
-+ else if (istarts_with(line, "Date: "))
++ else if (starts_with(line, "Date: "))
+ date = true;
-+ else if (istarts_with(line, "Subject: "))
++ else if (starts_with(line, "Subject: "))
+ subject = true;
+ }
+ return from && date && subject;
@@ imap-send.c: static void wrap_in_html(struct strbuf *msg)
static int count_messages(struct strbuf *all_msgs)
{
int count = 0;
- char *p = all_msgs->buf;
-
- while (1) {
-- if (starts_with(p, "From ")) {
-- if (starts_with(p, "From git-send-email")) {
-- p = strstr(p+5, "\nFrom: ");
-- if (!p) break;
-- p += 7;
-- p = strstr(p, "\nTo: ");
-- if (!p) break;
-- p += 5;
-- count++;
+@@ imap-send.c: static int count_messages(struct strbuf *all_msgs)
+ if (!p) break;
+ p += 5;
+ count++;
- } else {
- p = strstr(p+5, "\nFrom: ");
- if (!p) break;
@@ imap-send.c: static void wrap_in_html(struct strbuf *msg)
- p = strstr(p+7, "\nSubject: ");
- if (!p) break;
- p += 10;
-- count++;
-- }
-- }
-+ if (starts_with(p, "From ") && has_required_headers(p))
-+ count++;
- p = strstr(p+5, "\nFrom ");
- if (!p)
- break;
++ } else if (has_required_headers(p)) {
+ count++;
+ }
+ }
## t/t9001-send-email.sh ##
@@ t/t9001-send-email.sh: test_expect_success $PREREQ '--compose handles to headers' '
@@ t/t9001-send-email.sh: test_expect_success $PREREQ '--compose handles to headers
+ test_grep "^Sending 1 message to INBOX folder" err &&
+ test_grep "(1/1) done" err
+'
-+
-+test_expect_success $PREREQ '--imap-sent-folder stores a copy of the sent patch' '
-+ test_config imap.tunnel ./fake-imap-server &&
-+ clean_fake_sendmail &&
-+ git send-email \
-+ --from="Example <from@example.com>" \
-+ --to=nobody@example.com \
-+ --smtp-server="$(pwd)/fake.sendmail" \
-+ --imap-sent-folder=Sent \
-+ HEAD^ 2>err &&
-+ test_grep "^Sending 1 message to Sent folder" err &&
-+ test_grep "(1/1) done" err
-+'
+
test_done
-: ---------- > 3: 213c4950e0 imap-send: check git send-email copies like other messages
-: ---------- > 4: 01add80af2 imap-send: match header names case insensitively
Harald Nordgren via GitGitGadget
