[PATCH 0/4] send-email: correct various issues

[PATCH 0/4] send-email: correct various issues

[Jay Soffian]

Junio, these are technically all independent bug fixes, but they were
minor, so I've lumped them together. I figured you'd just add them to
the js/send-email topic anyway. Also, I apologize for causing trouble in
master. :-(

Björn, I've cc'd you for 3/4, which caused me to notice the problem that
led to 4/4. Uwe, I've cc'd you for 4/4.

Jay Soffian (4):
  send-email: don't attempt to prompt if tty is closed
  send-email: ask_default should apply to all emails, not just the
    first
  send-email: correct two tests which were going interactive
  send-email: ensure quoted addresses are rfc2047 encoded

 git-send-email.perl   |    9 ++++++---
 t/t9001-send-email.sh |   25 +++++++++++++++++++++----
 2 files changed, 27 insertions(+), 7 deletions(-)

[PATCH 1/4] send-email: don't attempt to prompt if tty is closed

[Jay Soffian]

Attempting to prompt when the tty is closed (typically when running from
cron) is pointless and emits a warning. This patch causes ask() to
return early, squelching the warning.

Signed-off-by: Jay Soffian <jaysoffian@gmail.com>
---
 git-send-email.perl |    3 +++
 1 files changed, 3 insertions(+), 0 deletions(-)

diff --git a/git-send-email.perl b/git-send-email.perl
index 5916c86..d790660 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -612,6 +612,9 @@ sub ask {
 	my $default = $arg{default};
 	my $resp;
 	my $i = 0;
+	return defined $default ? $default : undef
+		unless defined $term->IN and defined fileno($term->IN) and
+		       defined $term->OUT and defined fileno($term->OUT);
 	while ($i++ < 10) {
 		$resp = $term->readline($prompt);
 		if (!defined $resp) { # EOF
-- 
1.6.2.1.427.g15408

[PATCH 2/4] send-email: ask_default should apply to all emails, not just the first

[Jay Soffian]

Commit 6e18251 made the "Send this email?" prompt assume yes if confirm
= "inform" when it was unable to get a valid response. However, the
"yes" assumption only worked correctly for the first email. This commit
fixes the issue and confirms the fix my modifying the existing test for
the prompt to send multiple emails.

Reported by Matthieu Moy <Matthieu.Moy@imag.fr>

Signed-off-by: Jay Soffian <jaysoffian@gmail.com>
---
 git-send-email.perl   |    3 +--
 t/t9001-send-email.sh |    4 +++-
 2 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/git-send-email.perl b/git-send-email.perl
index d790660..fc153f9 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -687,7 +687,7 @@ if ($compose && $compose > 0) {
 
 # Variables we set as part of the loop over files
 our ($message_id, %mail, $subject, $reply_to, $references, $message,
-	$needs_confirm, $message_num);
+	$needs_confirm, $message_num, $ask_default);
 
 sub extract_valid_address {
 	my $address = shift;
@@ -845,7 +845,6 @@ X-Mailer: git-send-email $gitversion
 
 	if ($needs_confirm && !$dry_run) {
 		print "\n$header\n";
-		my $ask_default;
 		if ($needs_confirm eq "inform") {
 			$confirm_unconfigured = 0; # squelch this message for the rest of this run
 			$ask_default = "y"; # assume yes on EOF since user hasn't explicitly asked for confirmation
diff --git a/t/t9001-send-email.sh b/t/t9001-send-email.sh
index b4de98c..195ff8b 100755
--- a/t/t9001-send-email.sh
+++ b/t/t9001-send-email.sh
@@ -462,12 +462,14 @@ test_expect_success 'confirm by default (due to --compose)' '
 test_expect_success 'confirm detects EOF (inform assumes y)' '
 	CONFIRM=$(git config --get sendemail.confirm) &&
 	git config --unset sendemail.confirm &&
+	rm -fr outdir &&
+	git format-patch -2 -o outdir &&
 	GIT_SEND_EMAIL_NOTTY=1 \
 		git send-email \
 			--from="Example <nobody@example.com>" \
 			--to=nobody@example.com \
 			--smtp-server="$(pwd)/fake.sendmail" \
-			$patches < /dev/null
+			outdir/*.patch < /dev/null
 	ret="$?"
 	git config sendemail.confirm ${CONFIRM:-never}
 	test $ret = "0"
-- 
1.6.2.1.427.g15408

[PATCH 3/4] send-email: correct two tests which were going interactive

[Jay Soffian]

Commit 67f1fe5 added two tests which went interactive under the
dash shell. This commit corrects the issue. Reported by
Björn Steinbrink <B.Steinbrink@gmx.de>

Signed-off-by: Jay Soffian <jaysoffian@gmail.com>
---
 t/t9001-send-email.sh |    8 +++++---
 1 files changed, 5 insertions(+), 3 deletions(-)

diff --git a/t/t9001-send-email.sh b/t/t9001-send-email.sh
index 195ff8b..84238f7 100755
--- a/t/t9001-send-email.sh
+++ b/t/t9001-send-email.sh
@@ -478,7 +478,8 @@ test_expect_success 'confirm detects EOF (inform assumes y)' '
 test_expect_success 'confirm detects EOF (auto causes failure)' '
 	CONFIRM=$(git config --get sendemail.confirm) &&
 	git config sendemail.confirm auto &&
-	GIT_SEND_EMAIL_NOTTY=1 \
+	GIT_SEND_EMAIL_NOTTY=1 &&
+	export GIT_SEND_EMAIL_NOTTY &&
 		test_must_fail git send-email \
 			--from="Example <nobody@example.com>" \
 			--to=nobody@example.com \
@@ -492,8 +493,9 @@ test_expect_success 'confirm detects EOF (auto causes failure)' '
 test_expect_success 'confirm doesnt loop forever' '
 	CONFIRM=$(git config --get sendemail.confirm) &&
 	git config sendemail.confirm auto &&
-	yes "bogus" | GIT_SEND_EMAIL_NOTTY=1 \
-		test_must_fail git send-email \
+	GIT_SEND_EMAIL_NOTTY=1 &&
+	export GIT_SEND_EMAIL_NOTTY &&
+		yes "bogus" | test_must_fail git send-email \
 			--from="Example <nobody@example.com>" \
 			--to=nobody@example.com \
 			--smtp-server="$(pwd)/fake.sendmail" \
-- 
1.6.2.1.427.g15408

[PATCH 4/4] send-email: ensure quoted addresses are rfc2047 encoded

[Jay Soffian]

sanitize_address assumes that quoted addresses (e.g., "first last"
<first.last@example.com) do not need rfc2047 encoding, but this is
not always the case.

For example, various places in send-email extract addresses using
parse_address_line. parse_address_line returns the addresses already
quoted (e.g., "first last" <first.last@example.com), but not rfc2047
encoded.

This patch makes sanitize_address stricter about what needs rfc2047
encoding and adds a test demonstrating where I noticed the problem.

Signed-off-by: Jay Soffian <jaysoffian@gmail.com>
---
 git-send-email.perl   |    3 ++-
 t/t9001-send-email.sh |   13 +++++++++++++
 2 files changed, 15 insertions(+), 1 deletions(-)

diff --git a/git-send-email.perl b/git-send-email.perl
index fc153f9..6bbdfec 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -776,12 +776,13 @@ sub sanitize_address
 	}
 
 	# if recipient_name is already quoted, do nothing
-	if ($recipient_name =~ /^(".*"|=\?utf-8\?q\?.*\?=)$/) {
+	if ($recipient_name =~ /^("[[:ascii:]]*"|=\?utf-8\?q\?.*\?=)$/) {
 		return $recipient;
 	}
 
 	# rfc2047 is needed if a non-ascii char is included
 	if ($recipient_name =~ /[^[:ascii:]]/) {
+		$recipient_name =~ s/^"(.*)"$/$1/;
 		$recipient_name = quote_rfc2047($recipient_name);
 	}
 
diff --git a/t/t9001-send-email.sh b/t/t9001-send-email.sh
index 84238f7..192b97b 100755
--- a/t/t9001-send-email.sh
+++ b/t/t9001-send-email.sh
@@ -505,6 +505,19 @@ test_expect_success 'confirm doesnt loop forever' '
 	test $ret = "0"
 '
 
+test_expect_success 'utf8 Cc is rfc2047 encoded' '
+	clean_fake_sendmail &&
+	rm -fr outdir &&
+	git format-patch -1 -o outdir --cc="àéìöú <utf8@example.com>" &&
+	git send-email \
+	--from="Example <nobody@example.com>" \
+	--to=nobody@example.com \
+	--smtp-server="$(pwd)/fake.sendmail" \
+	outdir/*.patch &&
+	grep "^Cc:" msgtxt1 |
+	grep "=?utf-8?q?=C3=A0=C3=A9=C3=AC=C3=B6=C3=BA?= <utf8@example.com>"
+'
+
 test_expect_success '--compose adds MIME for utf8 body' '
 	clean_fake_sendmail &&
 	(echo "#!$SHELL_PATH" &&
-- 
1.6.2.1.427.g15408

Re: [PATCH 3/4] send-email: correct two tests which were going interactive

[Jeff King]

On Tue, Mar 31, 2009 at 12:22:13PM -0400, Jay Soffian wrote:

> Commit 67f1fe5 added two tests which went interactive under the
> dash shell. This commit corrects the issue. Reported by
> Björn Steinbrink <B.Steinbrink@gmx.de>

Thanks. I was seeing the same issue as Björn, and this fixes it for me.

-Peff

Re: [PATCH 0/4] send-email: correct various issues

[Matthieu Moy]

Jay Soffian <jaysoffian@gmail.com> writes:

> Junio, these are technically all independent bug fixes, but they were
> minor, so I've lumped them together. I figured you'd just add them to
> the js/send-email topic anyway. Also, I apologize for causing trouble in
> master. :-(

But I shall thank you for fixing them ;-).

> Jay Soffian (4):
>   send-email: don't attempt to prompt if tty is closed

Works, I don't have garbage anymore in my logs.

>   send-email: ask_default should apply to all emails, not just the
>     first

Works, I get the informational message in my logs once, and all the
mails are sent successfully.

>   send-email: correct two tests which were going interactive
>   send-email: ensure quoted addresses are rfc2047 encoded

Didn't test these.

-- 
Matthieu