From: Harald Nordgren via GitGitGadget Date: Tue, 27 Oct 2026 08:31:15 GMT Subject: [PATCH v4 0/4] Fix imap-send with reordered message headers Message-ID: In-Reply-To: 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 v4: * Store the tunnel as an allocated pointer, so it is only finished when it was started, and drop the local tunnel variable * Add a test for lowercase from:, date: and subject: headers * Explain in the commit message why dropping the From git-send-email special case is fine for mboxes written by other scripts 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 | 60 ++++++++++++++++++++++--------------------- t/t9001-send-email.sh | 50 ++++++++++++++++++++++++++++++++++++ 2 files changed, 81 insertions(+), 29 deletions(-) base-commit: 7e34d8174dcda3435d1404c9a30bef46afe6c13e Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-2456%2FHaraldNordgren%2Ffix%2Fimap-send-header-order-v4 Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-2456/HaraldNordgren/fix/imap-send-header-order-v4 Pull-Request: https://github.com/git/git/pull/2456 Range-diff vs v3: 1: 0f446c3c12 ! 1: 8941ae12bb imap-send: wait for the tunnel command to finish @@ 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 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); ++ if (imap->tunnel) { ++ finish_command(imap->tunnel); ++ free(imap->tunnel); ++ } free(imap); } @@ imap-send.c: static struct imap_store *imap_open_store(struct imap_server_conf * 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); @@ imap-send.c: static struct imap_store *imap_open_store(struct imap_server_conf * - 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)) ++ imap->tunnel = xmalloc(sizeof(*imap->tunnel)); ++ child_process_init(imap->tunnel); ++ strvec_push(&imap->tunnel->args, srvc->tunnel); ++ imap->tunnel->use_shell = 1; ++ imap->tunnel->in = -1; ++ imap->tunnel->out = -1; ++ if (start_command(imap->tunnel)) die("cannot start proxy %s", srvc->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->buf.sock.fd[0] = imap->tunnel->out; ++ imap->buf.sock.fd[1] = imap->tunnel->in; imap_info("OK\n"); } else { 2: 38214d9abf = 2: ad3158b336 imap-send: accept messages with Subject before Date 3: 213c4950e0 ! 3: 919e38c31d imap-send: check git send-email copies like other messages @@ Commit message the check for other messages. A To header is no longer required, the same as for any other message. + This makes the check stricter when some other script writes an mbox + with a "From git-send-email" line. RFC 5322 requires a Date header but + only recommends Subject, so a valid message from such a script may + lack a Subject and would no longer be counted. imap-send has always + required a Subject under any other "From " line, though, and only + refuses an mbox when none of its messages are counted, so treating + these copies like the rest should not affect anyone in practice. + Signed-off-by: Harald Nordgren ## imap-send.c ## 4: 01add80af2 ! 4: 5b4a08bbda imap-send: match header names case insensitively @@ imap-send.c: static bool has_required_headers(const char *line) subject = true; } return from && date && subject; + + ## t/t9001-send-email.sh ## +@@ t/t9001-send-email.sh: test_expect_success $PREREQ '--imap-sent-folder stores a copy of the sent patch' + test_grep "(1/1) done" err + ' + ++test_expect_success $PREREQ 'imap-send matches header names in any case' ' ++ test_config imap.tunnel ./fake-imap-server && ++ cat >lowercase.mbox <<-\EOF && ++ From mboxrd@z Thu Jan 1 00:00:00 1970 ++ from: Example ++ date: Thu, 1 Jan 1970 00:00:00 +0000 ++ subject: [PATCH] lowercase headers ++ ++ body ++ EOF ++ git imap-send --folder=INBOX err && ++ test_grep "^Sending 1 message to INBOX folder" err && ++ test_grep "(1/1) done" err ++' ++ + test_done -- gitgitgadget