Skip to contentSkip to conversation

send-email: correct various issues

PATCH In review Jay Soffian 3 participants 7 messages Jun 15, 2016

lore
Share & tools
lore

Jay Soffian

author
#0 lore raw

cover send-email: correct various issues

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(-)
author
#1 lore raw

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

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 <[email protected]>
---
 git-send-email.perl |    3 +++
 1 files changed, 3 insertions(+), 0 deletions(-)
git-send-email.perl +3 -0
index 5916c86..d790660 100755
@@ -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
Signature · 1 line
1.6.2.1.427.g15408
author
#2 lore raw

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

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 <[email protected]>

Signed-off-by Jay Soffian <[email protected]>
---
 git-send-email.perl   |    3 +--
 t/t9001-send-email.sh |    4 +++-
 2 files changed, 4 insertions(+), 3 deletions(-)
git-send-email.perl +1 -2
index d790660..fc153f9 100755
@@ -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
t/t9001-send-email.sh +3 -1
index b4de98c..195ff8b 100755
@@ -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 <[email protected]>" \
 			[email protected] \
 			--smtp-server="$(pwd)/fake.sendmail" \
-			$patches < /dev/null
+			outdir/*.patch < /dev/null
 	ret="$?"
 	git config sendemail.confirm ${CONFIRM:-never}
 	test $ret = "0"
Signature · 1 line
1.6.2.1.427.g15408
author
#3 lore raw

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

Commit 67f1fe5 added two tests which went interactive under the dash shell. This commit corrects the issue. Reported by Björn Steinbrink <[email protected]>

Signed-off-by Jay Soffian <[email protected]>
---
 t/t9001-send-email.sh |    8 +++++---
 1 files changed, 5 insertions(+), 3 deletions(-)
t/t9001-send-email.sh +5 -3
index 195ff8b..84238f7 100755
@@ -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 <[email protected]>" \
 			[email protected] \

@@ -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 <[email protected]>" \
 			[email protected] \
 			--smtp-server="$(pwd)/fake.sendmail" \
Signature · 1 line
1.6.2.1.427.g15408
author
#4 lore raw

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

sanitize_address assumes that quoted addresses (e.g., "first last" <[email protected]) 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" <[email protected]), 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 <[email protected]>
---
 git-send-email.perl   |    3 ++-
 t/t9001-send-email.sh |   13 +++++++++++++
 2 files changed, 15 insertions(+), 1 deletions(-)
git-send-email.perl +2 -1
index fc153f9..6bbdfec 100755
@@ -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);
 	}
t/t9001-send-email.sh +13 -0
index 84238f7..192b97b 100755
@@ -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="àéìöú <[email protected]>" &&
+	git send-email \
+	--from="Example <[email protected]>" \
+	[email protected] \
+	--smtp-server="$(pwd)/fake.sendmail" \
+	outdir/*.patch &&
+	grep "^Cc:" msgtxt1 |
+	grep "=?utf-8?q?=C3=A0=C3=A9=C3=AC=C3=B6=C3=BA?= <[email protected]>"
+'
+
 test_expect_success '--compose adds MIME for utf8 body' '
 	clean_fake_sendmail &&
 	(echo "#!$SHELL_PATH" &&
Signature · 1 line
1.6.2.1.427.g15408
#5 lore raw

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

In reply to Jay Soffian
Quoted 3 lines · Jay Soffian

Commit 67f1fe5 added two tests which went interactive under the dash shell. This commit corrects the issue. Reported by Björn Steinbrink <[email protected]>

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

-Peff

#6 lore raw
In reply to Jay Soffian
Quoted 4 lines · 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. :-(

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

Quoted 2 lines

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

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

Quoted 2 lines

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.

Quoted 2 lines

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

Didn't test these.

Signature · 1 line
Matthieu