send-email: move validation code below process_address_list
Move validation logic below processing of email address lists so that email validation gets the proper email addresses. As a side effect, some initialization needed to be moved down. In order for validation and the actual email sending to have the same initial state, the initialized variables that get modified by pre_process_file are encapsulated in a new function. This fixes email address validation errors when the optional perl module Email::Valid is installed and multiple addresses are passed in on a single to/cc argument like --to=foo@example.com,bar@example.com. A new test was added to t9001 to expose failures with this case in the future. Reported-by: Bagas Sanjaya <bagasdotme@gmail.com> Signed-off-by: Michael Strawbridge <michael.strawbridge@amd.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Michael Strawbridge committed
Oct 25, 2023 at 14:51 UTC
0d8647034e4c1647d850cb4a3bb1ea56fd4c3f32
2 files changed
+51
-28
git-send-email.perl
+32
-28
@@ -811,30 +811,6 @@ $sender = sanitize_address($sender);
811
812
$time = time - scalar $#files;
813
814
-if ($validate) {
815
- # FIFOs can only be read once, exclude them from validation.
816
- my @real_files = ();
817
- foreach my $f (@files) {
818
- unless (-p $f) {
819
- push(@real_files, $f);
820
- }
821
- }
822
-
823
- # Run the loop once again to avoid gaps in the counter due to FIFO
824
- # arguments provided by the user.
825
- my $num = 1;
826
- my $num_files = scalar @real_files;
827
- $ENV{GIT_SENDEMAIL_FILE_TOTAL} = "$num_files";
828
- foreach my $r (@real_files) {
829
- $ENV{GIT_SENDEMAIL_FILE_COUNTER} = "$num";
830
- pre_process_file($r, 1);
831
- validate_patch($r, $target_xfer_encoding);
832
- $num += 1;
833
- }
834
- delete $ENV{GIT_SENDEMAIL_FILE_COUNTER};
835
- delete $ENV{GIT_SENDEMAIL_FILE_TOTAL};
836
-}
837
-
814
@files = handle_backup_files(@files);
815
816
if (@files) {
@@ -1764,10 +1740,6 @@ EOF
1740
return 1;
1741
}
1742
1767
-$in_reply_to = $initial_in_reply_to;
1768
-$references = $initial_in_reply_to || '';
1769
-$message_num = 0;
1770
-
1743
sub pre_process_file {
1744
my ($t, $quiet) = @_;
1745
@@ -2033,6 +2005,38 @@ sub process_file {
2005
return 1;
2006
}
2007
2008
+sub initialize_modified_loop_vars {
2009
+ $in_reply_to = $initial_in_reply_to;
2010
+ $references = $initial_in_reply_to || '';
2011
+ $message_num = 0;
2012
+}
2013
+
2014
+if ($validate) {
2015
+ # FIFOs can only be read once, exclude them from validation.
2016
+ my @real_files = ();
2017
+ foreach my $f (@files) {
2018
+ unless (-p $f) {
2019
+ push(@real_files, $f);
2020
+ }
2021
+ }
2022
+
2023
+ # Run the loop once again to avoid gaps in the counter due to FIFO
2024
+ # arguments provided by the user.
2025
+ my $num = 1;
2026
+ my $num_files = scalar @real_files;
2027
+ $ENV{GIT_SENDEMAIL_FILE_TOTAL} = "$num_files";
2028
+ initialize_modified_loop_vars();
2029
+ foreach my $r (@real_files) {
2030
+ $ENV{GIT_SENDEMAIL_FILE_COUNTER} = "$num";
2031
+ pre_process_file($r, 1);
2032
+ validate_patch($r, $target_xfer_encoding);
2033
+ $num += 1;
2034
+ }
2035
+ delete $ENV{GIT_SENDEMAIL_FILE_COUNTER};
2036
+ delete $ENV{GIT_SENDEMAIL_FILE_TOTAL};
2037
+}
2038
+
2039
+initialize_modified_loop_vars();
2040
foreach my $t (@files) {
2041
while (!process_file($t)) {
2042
# user edited the file
t/t9001-send-email.sh
+19
@@ -632,6 +632,25 @@ test_expect_success $PREREQ "--validate respects absolute core.hooksPath path" '
632
test_cmp expect actual
633
'
634
635
+test_expect_success $PREREQ "--validate hook supports multiple addresses in arguments" '
636
+ hooks_path="$(pwd)/my-hooks" &&
637
+ test_config core.hooksPath "$hooks_path" &&
638
+ test_when_finished "rm my-hooks.ran" &&
639
+ test_must_fail git send-email \
640
+ --from="Example <nobody@example.com>" \
641
+ --to=nobody@example.com,abc@example.com \
642
+ --smtp-server="$(pwd)/fake.sendmail" \
643
+ --validate \
644
+ longline.patch 2>actual &&
645
+ test_path_is_file my-hooks.ran &&
646
+ cat >expect <<-EOF &&
647
+ fatal: longline.patch: rejected by sendemail-validate hook
648
+ fatal: command '"'"'git hook run --ignore-missing sendemail-validate -- <patch> <header>'"'"' died with exit code 1
649
+ warning: no patches were sent
650
+ EOF
651
+ test_cmp expect actual
652
+'
653
+
654
test_expect_success $PREREQ "--validate hook supports header argument" '
655
write_script my-hooks/sendemail-validate <<-\EOF &&
656
if test "$#" -ge 2