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

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

[Harald Nordgren via GitGitGadget]

git imap-send rejected patches sent with git send-email, such as ones
fetched from lore with b4, because it expected Date before Subject.

Harald Nordgren (2):
  imap-send: avoid leaking the tunnel command
  imap-send: accept messages with Subject before Date

 imap-send.c           | 38 ++++++++++++++++++--------------------
 t/t9001-send-email.sh | 35 +++++++++++++++++++++++++++++++++++
 2 files changed, 53 insertions(+), 20 deletions(-)


base-commit: c51408889c3b372814bc20b9431c3b74d0d268eb
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-2456%2FHaraldNordgren%2Ffix%2Fimap-send-header-order-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-2456/HaraldNordgren/fix/imap-send-header-order-v1
Pull-Request: https://github.com/git/git/pull/2456
-- 
gitgitgadget

[PATCH 1/2] imap-send: avoid leaking the tunnel command

[Harald Nordgren via GitGitGadget]

From: Harald Nordgren <haraldnordgren@gmail.com>

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.

Only the pipes to the tunnel are needed once it is running, so release
the rest right after starting it.

Signed-off-by: Harald Nordgren <haraldnordgren@gmail.com>
---
 imap-send.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/imap-send.c b/imap-send.c
index bf1d2cf74d..6f76bade35 100644
--- a/imap-send.c
+++ b/imap-send.c
@@ -1176,6 +1176,7 @@ static struct imap_store *imap_open_store(struct imap_server_conf *srvc, const c
 
 		imap->buf.sock.fd[0] = tunnel.out;
 		imap->buf.sock.fd[1] = tunnel.in;
+		child_process_clear(&tunnel);
 
 		imap_info("OK\n");
 	} else {
-- 
gitgitgadget

[PATCH 2/2] imap-send: accept messages with Subject before Date

[Harald Nordgren via GitGitGadget]

From: Harald Nordgren <haraldnordgren@gmail.com>

Patches sent with git send-email cannot be fed back into git imap-send
after fetching them from the mailing list archive, for example with

    b4 mbox --single-message -o - <url> | git imap-send

which fails with "no messages found to send". imap-send only counts a
message when its From, Date and Subject headers come in that order,
and git send-email writes Subject before Date.

Accept the three headers in any order within the header block of each
message. 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.

Signed-off-by: Harald Nordgren <haraldnordgren@gmail.com>
---
 imap-send.c           | 37 +++++++++++++++++--------------------
 t/t9001-send-email.sh | 35 +++++++++++++++++++++++++++++++++++
 2 files changed, 52 insertions(+), 20 deletions(-)

diff --git a/imap-send.c b/imap-send.c
index 6f76bade35..6563c8f693 100644
--- a/imap-send.c
+++ b/imap-send.c
@@ -1453,32 +1453,29 @@ static void wrap_in_html(struct strbuf *msg)
 	*msg = buf;
 }
 
+static bool has_required_headers(const char *line)
+{
+	bool from = false, date = false, subject = false;
+
+	while ((line = strchr(line, '\n')) && *++line != '\n') {
+		if (starts_with(line, "From: "))
+			from = true;
+		else if (starts_with(line, "Date: "))
+			date = true;
+		else if (starts_with(line, "Subject: "))
+			subject = true;
+	}
+	return from && date && subject;
+}
+
 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++;
-			} else {
-				p = strstr(p+5, "\nFrom: ");
-				if (!p) break;
-				p = strstr(p+7, "\nDate: ");
-				if (!p) break;
-				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;
diff --git a/t/t9001-send-email.sh b/t/t9001-send-email.sh
index d1393ef197..1f77c14315 100755
--- a/t/t9001-send-email.sh
+++ b/t/t9001-send-email.sh
@@ -2863,4 +2863,39 @@ test_expect_success $PREREQ '--compose handles to headers' '
 	test_cmp expect msgtxt2.to
 '
 
+test_expect_success $PREREQ 'imap-send accepts a sent patch from the list archive' '
+	write_script fake-imap-server <<-\EOF &&
+	printf "* PREAUTH [CAPABILITY IMAP4rev1 LITERAL+] ready\r\n"
+	printf "1 OK\r\n2 OK\r\n3 OK\r\n"
+	cat >/dev/null
+	EOF
+	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" \
+		HEAD^ &&
+	{
+		echo "From mboxrd@z Thu Jan  1 00:00:00 1970" &&
+		cat msgtxt1
+	} >archive.mbox &&
+	git imap-send --folder=INBOX <archive.mbox 2>err &&
+	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
-- 
gitgitgadget

Re: [PATCH 2/2] imap-send: accept messages with Subject before Date

[Junio C Hamano]

"Harald Nordgren via GitGitGadget" <gitgitgadget@gmail.com> writes:

> which fails with "no messages found to send". imap-send only counts a
> message when its From, Date and Subject headers come in that order,
> and git send-email writes Subject before Date.

Nicely analysed.

> +static bool has_required_headers(const char *line)
> +{
> +	bool from = false, date = false, subject = false;
> +
> +	while ((line = strchr(line, '\n')) && *++line != '\n') {
> +		if (starts_with(line, "From: "))
> +			from = true;
> +		else if (starts_with(line, "Date: "))
> +			date = true;
> +		else if (starts_with(line, "Subject: "))
> +			subject = true;
> +	}
> +	return from && date && subject;
> +}

So the caller is responsible for placing the cursor at the beginning
of a message (e.g., a "From " line) and this one inspects the lines
starting at the point looking for the headers we want to see.  The
cursor is local so our incrementing will not break the caller.

RFC 822/2822/5322 specifies these headers as case insensitive, so
you'd want to use istarts_with() instead, though.  I can see that
the original did not get it right, but we are making the wrongs in
the original right, so ...

>  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++;
> -			} else {
> -				p = strstr(p+5, "\nFrom: ");
> -				if (!p) break;
> -				p = strstr(p+7, "\nDate: ");
> -				if (!p) break;
> -				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;
> diff --git a/t/t9001-send-email.sh b/t/t9001-send-email.sh
> index d1393ef197..1f77c14315 100755
> --- a/t/t9001-send-email.sh
> +++ b/t/t9001-send-email.sh
> @@ -2863,4 +2863,39 @@ test_expect_success $PREREQ '--compose handles to headers' '
>  	test_cmp expect msgtxt2.to
>  '
>  
> +test_expect_success $PREREQ 'imap-send accepts a sent patch from the list archive' '
> +	write_script fake-imap-server <<-\EOF &&
> +	printf "* PREAUTH [CAPABILITY IMAP4rev1 LITERAL+] ready\r\n"
> +	printf "1 OK\r\n2 OK\r\n3 OK\r\n"
> +	cat >/dev/null
> +	EOF
> +	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" \
> +		HEAD^ &&
> +	{
> +		echo "From mboxrd@z Thu Jan  1 00:00:00 1970" &&
> +		cat msgtxt1
> +	} >archive.mbox &&
> +	git imap-send --folder=INBOX <archive.mbox 2>err &&
> +	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

Re: [PATCH 2/2] imap-send: accept messages with Subject before Date

[Harald Nordgren]

> RFC 822/2822/5322 specifies these headers as case insensitive, so
> you'd want to use istarts_with() instead, though.  I can see that
> the original did not get it right, but we are making the wrongs in
> the original right, so ...

Thanks!

Would you like to see a test for case insensitive? Personally, I would
skip it since I trust 'istarts_with'.


Harald

[PATCH v2 0/2] Fix imap-send with reordered message headers

[Harald Nordgren via GitGitGadget]

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 v2:

 * Case insensitive header match according to RFC 822/2822/5322.

Harald Nordgren (2):
  imap-send: avoid leaking the tunnel command
  imap-send: accept messages with Subject before Date

 imap-send.c           | 38 ++++++++++++++++++--------------------
 t/t9001-send-email.sh | 35 +++++++++++++++++++++++++++++++++++
 2 files changed, 53 insertions(+), 20 deletions(-)


base-commit: 47fdf2208c5b94c8cc01c106b8f7c85efb91c926
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-2456%2FHaraldNordgren%2Ffix%2Fimap-send-header-order-v2
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-2456/HaraldNordgren/fix/imap-send-header-order-v2
Pull-Request: https://github.com/git/git/pull/2456

Range-diff vs v1:

 1:  0784c16134 = 1:  b170b1f6d3 imap-send: avoid leaking the tunnel command
 2:  94129fb718 ! 2:  c42827a122 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. The copy that git send-email --imap-sent-folder pipes to
     +    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.
     @@ 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 (starts_with(line, "From: "))
     ++		if (istarts_with(line, "From: "))
      +			from = true;
     -+		else if (starts_with(line, "Date: "))
     ++		else if (istarts_with(line, "Date: "))
      +			date = true;
     -+		else if (starts_with(line, "Subject: "))
     ++		else if (istarts_with(line, "Subject: "))
      +			subject = true;
      +	}
      +	return from && date && subject;

-- 
gitgitgadget

[PATCH v2 1/2] imap-send: avoid leaking the tunnel command

[Harald Nordgren via GitGitGadget]

From: Harald Nordgren <haraldnordgren@gmail.com>

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.

Only the pipes to the tunnel are needed once it is running, so release
the rest right after starting it.

Signed-off-by: Harald Nordgren <haraldnordgren@gmail.com>
---
 imap-send.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/imap-send.c b/imap-send.c
index bf1d2cf74d..6f76bade35 100644
--- a/imap-send.c
+++ b/imap-send.c
@@ -1176,6 +1176,7 @@ static struct imap_store *imap_open_store(struct imap_server_conf *srvc, const c
 
 		imap->buf.sock.fd[0] = tunnel.out;
 		imap->buf.sock.fd[1] = tunnel.in;
+		child_process_clear(&tunnel);
 
 		imap_info("OK\n");
 	} else {
-- 
gitgitgadget

[PATCH v2 2/2] imap-send: accept messages with Subject before Date

[Harald Nordgren via GitGitGadget]

From: Harald Nordgren <haraldnordgren@gmail.com>

Patches sent with git send-email cannot be fed back into git imap-send
after fetching them from the mailing list archive, for example with

    b4 mbox --single-message -o - <url> | git imap-send

which fails with "no messages found to send". imap-send only counts a
message when its From, Date and Subject headers come in that order,
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.

Signed-off-by: Harald Nordgren <haraldnordgren@gmail.com>
---
 imap-send.c           | 37 +++++++++++++++++--------------------
 t/t9001-send-email.sh | 35 +++++++++++++++++++++++++++++++++++
 2 files changed, 52 insertions(+), 20 deletions(-)

diff --git a/imap-send.c b/imap-send.c
index 6f76bade35..7ae2eeddc9 100644
--- a/imap-send.c
+++ b/imap-send.c
@@ -1453,32 +1453,29 @@ static void wrap_in_html(struct strbuf *msg)
 	*msg = buf;
 }
 
+static bool has_required_headers(const char *line)
+{
+	bool from = false, date = false, subject = false;
+
+	while ((line = strchr(line, '\n')) && *++line != '\n') {
+		if (istarts_with(line, "From: "))
+			from = true;
+		else if (istarts_with(line, "Date: "))
+			date = true;
+		else if (istarts_with(line, "Subject: "))
+			subject = true;
+	}
+	return from && date && subject;
+}
+
 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++;
-			} else {
-				p = strstr(p+5, "\nFrom: ");
-				if (!p) break;
-				p = strstr(p+7, "\nDate: ");
-				if (!p) break;
-				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;
diff --git a/t/t9001-send-email.sh b/t/t9001-send-email.sh
index d1393ef197..1f77c14315 100755
--- a/t/t9001-send-email.sh
+++ b/t/t9001-send-email.sh
@@ -2863,4 +2863,39 @@ test_expect_success $PREREQ '--compose handles to headers' '
 	test_cmp expect msgtxt2.to
 '
 
+test_expect_success $PREREQ 'imap-send accepts a sent patch from the list archive' '
+	write_script fake-imap-server <<-\EOF &&
+	printf "* PREAUTH [CAPABILITY IMAP4rev1 LITERAL+] ready\r\n"
+	printf "1 OK\r\n2 OK\r\n3 OK\r\n"
+	cat >/dev/null
+	EOF
+	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" \
+		HEAD^ &&
+	{
+		echo "From mboxrd@z Thu Jan  1 00:00:00 1970" &&
+		cat msgtxt1
+	} >archive.mbox &&
+	git imap-send --folder=INBOX <archive.mbox 2>err &&
+	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
-- 
gitgitgadget

Re: [PATCH v2 1/2] imap-send: avoid leaking the tunnel command

[Patrick Steinhardt]

On Wed, Oct 21, 2026 at 07:07:48AM +0000, Harald Nordgren via GitGitGadget wrote:
> diff --git a/imap-send.c b/imap-send.c
> index bf1d2cf74d..6f76bade35 100644
> --- a/imap-send.c
> +++ b/imap-send.c
> @@ -1176,6 +1176,7 @@ static struct imap_store *imap_open_store(struct imap_server_conf *srvc, const c
>  
>  		imap->buf.sock.fd[0] = tunnel.out;
>  		imap->buf.sock.fd[1] = tunnel.in;
> +		child_process_clear(&tunnel);

This change plugs the memory leak, but the bigger question to me is why
we never call `finish_command()` on the child process at all. Is there a
reason for that?

Patrick

Re: [PATCH v2 2/2] imap-send: accept messages with Subject before Date

[Patrick Steinhardt]

On Wed, Oct 21, 2026 at 07:07:49AM +0000, Harald Nordgren via GitGitGadget wrote:
> From: Harald Nordgren <haraldnordgren@gmail.com>
> 
> Patches sent with git send-email cannot be fed back into git imap-send
> after fetching them from the mailing list archive, for example with
> 
>     b4 mbox --single-message -o - <url> | git imap-send
> 
> which fails with "no messages found to send". imap-send only counts a
> message when its From, Date and Subject headers come in that order,
> 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.

Aren't you fixing two different issues at once? I'd argue that this
should be done in separate commits.

> diff --git a/imap-send.c b/imap-send.c
> index 6f76bade35..7ae2eeddc9 100644
> --- a/imap-send.c
> +++ b/imap-send.c
> @@ -1453,32 +1453,29 @@ static void wrap_in_html(struct strbuf *msg)
>  	*msg = buf;
>  }
>  
> +static bool has_required_headers(const char *line)
> +{
> +	bool from = false, date = false, subject = false;
> +
> +	while ((line = strchr(line, '\n')) && *++line != '\n') {
> +		if (istarts_with(line, "From: "))
> +			from = true;
> +		else if (istarts_with(line, "Date: "))
> +			date = true;
> +		else if (istarts_with(line, "Subject: "))
> +			subject = true;
> +	}
> +
> +	return from && date && subject;
> +}
> +
>  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++;
> -			} else {
> -				p = strstr(p+5, "\nFrom: ");
> -				if (!p) break;
> -				p = strstr(p+7, "\nDate: ");
> -				if (!p) break;
> -				p = strstr(p+7, "\nSubject: ");
> -				if (!p) break;
> -				p += 10;
> -				count++;
> -			}
> -		}
> +		if (starts_with(p, "From ") && has_required_headers(p))
> +			count++;

In the preimage we special-cased when the line starts with "From
git-send-email", and in that case we didn't recognize "Subject:" lines.
So you're not only changing that we now accept the headers in any
arbitrary order, but you also make us accept "Subject:" lines when we
saw "From git-send-email". You don't really explain why that change is
okay to do though.

> diff --git a/t/t9001-send-email.sh b/t/t9001-send-email.sh
> index d1393ef197..1f77c14315 100755
> --- a/t/t9001-send-email.sh
> +++ b/t/t9001-send-email.sh
> @@ -2863,4 +2863,39 @@ test_expect_success $PREREQ '--compose handles to headers' '
>  	test_cmp expect msgtxt2.to
>  '
>  
> +test_expect_success $PREREQ 'imap-send accepts a sent patch from the list archive' '

Quite weird that every single test carries this prerequisite. Makes me
wonder why we don't just skip all tests at once.

Patrick

Re: [PATCH v2 2/2] imap-send: accept messages with Subject before Date

[Harald Nordgren]

> > Patches sent with git send-email cannot be fed back into git imap-send
> > after fetching them from the mailing list archive, for example with
> >
> >     b4 mbox --single-message -o - <url> | git imap-send
> >
> > which fails with "no messages found to send". imap-send only counts a
> > message when its From, Date and Subject headers come in that order,
> > 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.
>
> Aren't you fixing two different issues at once? I'd argue that this
> should be done in separate commits.

Good point.

> > @@ -2863,4 +2863,39 @@ test_expect_success $PREREQ '--compose handles to headers' '
> >       test_cmp expect msgtxt2.to
> >  '
> >
> > +test_expect_success $PREREQ 'imap-send accepts a sent patch from the list archive' '
>
> Quite weird that every single test carries this prerequisite. Makes me
> wonder why we don't just skip all tests at once.

What do you mean here, do you want to remove the tests?



Harald

Re: [PATCH v2 1/2] imap-send: avoid leaking the tunnel command

[Harald Nordgren]

> >               imap->buf.sock.fd[0] = tunnel.out;
> >               imap->buf.sock.fd[1] = tunnel.in;
> > +             child_process_clear(&tunnel);
>
> This change plugs the memory leak, but the bigger question to me is why
> we never call `finish_command()` on the child process at all. Is there a
> reason for that?

Yeah, that's curious. I'll take a look at handling this better.


Harald

Re: [PATCH v2 2/2] imap-send: accept messages with Subject before Date

[Patrick Steinhardt]

On Wed, Oct 21, 2026 at 10:45:42AM +0200, Harald Nordgren wrote:
> > > @@ -2863,4 +2863,39 @@ test_expect_success $PREREQ '--compose handles to headers' '
> > >       test_cmp expect msgtxt2.to
> > >  '
> > >
> > > +test_expect_success $PREREQ 'imap-send accepts a sent patch from the list archive' '
> >
> > Quite weird that every single test carries this prerequisite. Makes me
> > wonder why we don't just skip all tests at once.
> 
> What do you mean here, do you want to remove the tests?

No, I rather think that we should have this at the beginning of the test
file:

        if ! test_have_prereq PERL
        then
        	skip_all='skipping send-email tests; Perl not available'
        	test_done
        fi

Then we can drop the $PREREQ from all tests. But I just spotted that we
do some more gymanstics with another prereq that we optionally add
later, so this doesn't quite work as-is.

Overall though this is fine, we don't have to fix it as part of this
patch series.

Patrick

Re: [PATCH v2 2/2] imap-send: accept messages with Subject before Date

[Harald Nordgren]

> > What do you mean here, do you want to remove the tests?
>
> No, I rather think that we should have this at the beginning of the test
> file:
>
>         if ! test_have_prereq PERL
>         then
>                 skip_all='skipping send-email tests; Perl not available'
>                 test_done
>         fi
>
> Then we can drop the $PREREQ from all tests. But I just spotted that we
> do some more gymanstics with another prereq that we optionally add
> later, so this doesn't quite work as-is.
>
> Overall though this is fine, we don't have to fix it as part of this
> patch series.

Alright, maybe better to have this as #leftoverbits ?


Harald

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

[Harald Nordgren via GitGitGadget]

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

-- 
gitgitgadget

[PATCH v3 1/4] imap-send: wait for the tunnel command to finish

[Harald Nordgren via GitGitGadget]

From: Harald Nordgren <haraldnordgren@gmail.com>

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.

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 | 20 ++++++++++++--------
 1 file changed, 12 insertions(+), 8 deletions(-)

diff --git a/imap-send.c b/imap-send.c
index bf1d2cf74d..fa326c4ae2 100644
--- a/imap-send.c
+++ b/imap-send.c
@@ -118,6 +118,7 @@ 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 */
 };
 
@@ -858,6 +859,8 @@ 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);
 }
 
@@ -1163,19 +1166,20 @@ 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;
+		imap->buf.sock.fd[0] = tunnel->out;
+		imap->buf.sock.fd[1] = tunnel->in;
 
 		imap_info("OK\n");
 	} else {
-- 
gitgitgadget

[PATCH v3 2/4] imap-send: accept messages with Subject before Date

[Harald Nordgren via GitGitGadget]

From: Harald Nordgren <haraldnordgren@gmail.com>

Patches sent with git send-email cannot be fed back into git imap-send
after fetching them from the mailing list archive, for example with

    b4 mbox --single-message -o - <url> | git imap-send

which fails with "no messages found to send". imap-send only counts a
message when its From, Date and Subject headers come in that order,
and git send-email writes Subject before Date.

Accept the three headers in any order within the header block of each
message.

Signed-off-by: Harald Nordgren <haraldnordgren@gmail.com>
---
 imap-send.c           | 24 ++++++++++++++++--------
 t/t9001-send-email.sh | 22 ++++++++++++++++++++++
 2 files changed, 38 insertions(+), 8 deletions(-)

diff --git a/imap-send.c b/imap-send.c
index fa326c4ae2..ce2321d42e 100644
--- a/imap-send.c
+++ b/imap-send.c
@@ -1456,6 +1456,21 @@ static void wrap_in_html(struct strbuf *msg)
 	*msg = buf;
 }
 
+static bool has_required_headers(const char *line)
+{
+	bool from = false, date = false, subject = false;
+
+	while ((line = strchr(line, '\n')) && *++line != '\n') {
+		if (starts_with(line, "From: "))
+			from = true;
+		else if (starts_with(line, "Date: "))
+			date = true;
+		else if (starts_with(line, "Subject: "))
+			subject = true;
+	}
+	return from && date && subject;
+}
+
 static int count_messages(struct strbuf *all_msgs)
 {
 	int count = 0;
@@ -1471,14 +1486,7 @@ static int count_messages(struct strbuf *all_msgs)
 				if (!p) break;
 				p += 5;
 				count++;
-			} else {
-				p = strstr(p+5, "\nFrom: ");
-				if (!p) break;
-				p = strstr(p+7, "\nDate: ");
-				if (!p) break;
-				p = strstr(p+7, "\nSubject: ");
-				if (!p) break;
-				p += 10;
+			} else if (has_required_headers(p)) {
 				count++;
 			}
 		}
diff --git a/t/t9001-send-email.sh b/t/t9001-send-email.sh
index d1393ef197..d105fcb41c 100755
--- a/t/t9001-send-email.sh
+++ b/t/t9001-send-email.sh
@@ -2863,4 +2863,26 @@ test_expect_success $PREREQ '--compose handles to headers' '
 	test_cmp expect msgtxt2.to
 '
 
+test_expect_success $PREREQ 'imap-send accepts a sent patch from the list archive' '
+	write_script fake-imap-server <<-\EOF &&
+	printf "* PREAUTH [CAPABILITY IMAP4rev1 LITERAL+] ready\r\n"
+	printf "1 OK\r\n2 OK\r\n3 OK\r\n"
+	cat >/dev/null
+	EOF
+	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" \
+		HEAD^ &&
+	{
+		echo "From mboxrd@z Thu Jan  1 00:00:00 1970" &&
+		cat msgtxt1
+	} >archive.mbox &&
+	git imap-send --folder=INBOX <archive.mbox 2>err &&
+	test_grep "^Sending 1 message to INBOX folder" err &&
+	test_grep "(1/1) done" err
+'
+
 test_done
-- 
gitgitgadget

[PATCH v3 3/4] imap-send: check git send-email copies like other messages

[Harald Nordgren via GitGitGadget]

From: Harald Nordgren <haraldnordgren@gmail.com>

The copy that git send-email --imap-sent-folder pipes to imap-send
starts with a "From git-send-email" line. 04133f5bc4 (send-email: add
ability to send a copy of sent emails to an IMAP folder, 2025-08-12)
made imap-send count such a message when it has From and To headers,
without looking for Date and Subject. Without that special case the
copy would not be counted at all, because git send-email writes
Subject before Date.

Now that the headers are accepted in any order, the special case is no
longer needed. The copy carries the same headers git send-email sends
over SMTP, which always include From, Subject and Date, so it passes
the check for other messages. A To header is no longer required, the
same as for any other message.

Signed-off-by: Harald Nordgren <haraldnordgren@gmail.com>
---
 imap-send.c           | 15 ++-------------
 t/t9001-send-email.sh | 13 +++++++++++++
 2 files changed, 15 insertions(+), 13 deletions(-)

diff --git a/imap-send.c b/imap-send.c
index ce2321d42e..1091825dfb 100644
--- a/imap-send.c
+++ b/imap-send.c
@@ -1477,19 +1477,8 @@ static int count_messages(struct strbuf *all_msgs)
 	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++;
-			} else if (has_required_headers(p)) {
-				count++;
-			}
-		}
+		if (starts_with(p, "From ") && has_required_headers(p))
+			count++;
 		p = strstr(p+5, "\nFrom ");
 		if (!p)
 			break;
diff --git a/t/t9001-send-email.sh b/t/t9001-send-email.sh
index d105fcb41c..1f77c14315 100755
--- a/t/t9001-send-email.sh
+++ b/t/t9001-send-email.sh
@@ -2885,4 +2885,17 @@ test_expect_success $PREREQ 'imap-send accepts a sent patch from the list archiv
 	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
-- 
gitgitgadget

[PATCH v3 4/4] imap-send: match header names case insensitively

[Harald Nordgren via GitGitGadget]

From: Harald Nordgren <haraldnordgren@gmail.com>

imap-send says "no messages found to send" for a message whose headers
are spelled "from:", "date:" or "subject:", although RFC 5322 treats
header field names as case insensitive. Match the three names in any
case.

Signed-off-by: Harald Nordgren <haraldnordgren@gmail.com>
---
 imap-send.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/imap-send.c b/imap-send.c
index 1091825dfb..21e20d7720 100644
--- a/imap-send.c
+++ b/imap-send.c
@@ -1461,11 +1461,11 @@ static bool has_required_headers(const char *line)
 	bool from = false, date = false, subject = false;
 
 	while ((line = strchr(line, '\n')) && *++line != '\n') {
-		if (starts_with(line, "From: "))
+		if (istarts_with(line, "From: "))
 			from = true;
-		else if (starts_with(line, "Date: "))
+		else if (istarts_with(line, "Date: "))
 			date = true;
-		else if (starts_with(line, "Subject: "))
+		else if (istarts_with(line, "Subject: "))
 			subject = true;
 	}
 	return from && date && subject;
-- 
gitgitgadget