[PATCH v4 0/4] Fix imap-send with reordered message headers

["Harald Nordgren via GitGitGadget" <gitgitgadget@gmail.com>]

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 <haraldnordgren@gmail.com>
      
       ## 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 <from@example.com>
     ++	date: Thu, 1 Jan 1970 00:00:00 +0000
     ++	subject: [PATCH] lowercase headers
     ++
     ++	body
     ++	EOF
     ++	git imap-send --folder=INBOX <lowercase.mbox 2>err &&
     ++	test_grep "^Sending 1 message to INBOX folder" err &&
     ++	test_grep "(1/1) done" err
     ++'
     ++
     + test_done

-- 
gitgitgadget