Re: [REGRESSION] uninitialized value $address in git send-email when given multiple recipients separated by commas

[Bagas Sanjaya <bagasdotme@gmail.com>]

[-- Attachment #1: Type: text/plain, Size: 3282 bytes --]

On Mon, Sep 25, 2023 at 12:17:48PM -0400, Jeff King wrote:
> On Mon, Sep 25, 2023 at 10:48:29AM -0400, Todd Zullinger wrote:
> 
> > From the peanut gallery... could the presence or lack of the
> > Email::Valid perl module be a factor?
> 
> Ah, thanks! The thought of differing modules even occurred to me, since
> I know we have a few optimistic dependencies, but when I looked I didn't
> manage to find that one (somehow I thought Mail::Address was the
> interesting one here; I think I might be getting senile).
> 
> With Email::Valid installed, I can reproduce with just (in git.git, but
> I think it would work in any repo):
> 
>   $ echo "exit 0" >.git/hooks/sendemail-validate
>   $ chmod +x .git/hooks/sendemail-validate
>   $ git send-email --dry-run -1 --to=foo@example.com,bar@example.com
>   error: unable to extract a valid address from: foo@example.com,bar@example.com
> 
> Disabling the hook with "chmod -x" makes the problem go away (and this
> is with current "master", hence the more readable error message).
> 
> I think the issue is that a8022c5f7b ends up in extract_valid_address()
> via this call stack:
> 
>   $ = main::extract_valid_address('foo@example.com,bar@example.com') called from file '/home/peff/compile/git/git-send-email' line 1161
>   $ = main::extract_valid_address_or_die('foo@example.com,bar@example.com') called from file '/home/peff/compile/git/git-send-email' line 2087
>   @ = main::unique_email_list('foo@example.com,bar@example.com') called from file '/home/peff/compile/git/git-send-email' line 1507
>   @ = main::gen_header() called from file '/home/peff/compile/git/git-send-email' line 2113
>   . = main::validate_patch('/tmp/WfoPQSKCUa/0001-The-twelfth-batch.patch', 'auto') called from file '/home/peff/compile/git/git-send-email' line 815
> 
> whereas prior to that commit, we hit it later:
> 
>   $ = main::extract_valid_address('foo@example.com') called from file '/home/peff/compile/git/git-send-email' line 1166
>   @ = main::validate_address('foo@example.com') called from file '/home/peff/compile/git/git-send-email' line 1189
>   @ = main::validate_address_list('foo@example.com', 'bar@example.com') called from file '/home/peff/compile/git/git-send-email' line 1348
>   @ = main::process_address_list('foo@example.com,bar@example.com') called from file '/home/peff/compile/git/git-send-email' line 1091
> 
> So the issue is the call to gen_header() added in validate_patch(). We
> won't yet have processed the address lists by that point. We can move
> those calls up, but it requires moving a bit of extra code, too (like
> the parts prompting for the "to" list if it isn't filled in).
> 
> Possibly the validation checks need to be moved down, if they want to
> see a more complete view of the emails. But now we're doing more work
> (like asking the user to write the cover letter!) before we do
> validation, which is probably bad.
> 
> So I dunno. Maybe gen_header() should be lazily doing this
> process_address_list() stuff? I'm not very familiar with the send-email
> code, so I'm not sure what secondary effects that could have.
> 

Michael, did you look into this since you authored the culprit?

-- 
An old man doll... just what I always wanted! - Clara

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]