threads / patch / 18664

patch, 4 partssend-email: correct various issues

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

## tl;dr

7 messages between Mar 31, 2009 and Apr 1, 2009. Diffs are folded; open one to read it.

replies: 6people: 3as markdown or json

Jay Soffian· Mar 31, 2009, 16:22 UTC · lore

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(-)
Jay Soffian· Mar 31, 2009, 16:22 UTC · re: Jay Soffian · lore

[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 <jaysoffian@gmail.com>
---
 git-send-email.perl |    3 +++
 1 files changed, 3 insertions(+), 0 deletions(-)
Show changes to git-send-email.perl +3 −0
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
Jay Soffian· Mar 31, 2009, 16:22 UTC · re: Jay Soffian · lore

[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 <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(-)
Show changes to 2 files +4 −3

git-send-email.perl, t/t9001-send-email.sh

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
Jay Soffian· Mar 31, 2009, 16:22 UTC · re: Jay Soffian · lore

[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 <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(-)
Show changes to t/t9001-send-email.sh +5 −3
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
Jay Soffian· Mar 31, 2009, 16:22 UTC · re: Jay Soffian · lore

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

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(-)
Show changes to 2 files +15 −1

git-send-email.perl, t/t9001-send-email.sh

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
Jeff King· Mar 31, 2009, 19:46 UTC · re: Jay Soffian · lore

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

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
Matthieu Moy· Apr 1, 2009, 08:33 UTC · re: Jay Soffian · lore

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

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

← back to recent threads