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@xxxxxxxxxxx,bar@xxxxxxxxxxx
> error: unable to extract a valid address from: foo@xxxxxxxxxxx,bar@xxxxxxxxxxx
>
> 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@xxxxxxxxxxx,bar@xxxxxxxxxxx') called from file '/home/peff/compile/git/git-send-email' line 1161
> $ = main::extract_valid_address_or_die('foo@xxxxxxxxxxx,bar@xxxxxxxxxxx') called from file '/home/peff/compile/git/git-send-email' line 2087
> @ = main::unique_email_list('foo@xxxxxxxxxxx,bar@xxxxxxxxxxx') 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@xxxxxxxxxxx') called from file '/home/peff/compile/git/git-send-email' line 1166
> @ = main::validate_address('foo@xxxxxxxxxxx') called from file '/home/peff/compile/git/git-send-email' line 1189
> @ = main::validate_address_list('foo@xxxxxxxxxxx', 'bar@xxxxxxxxxxx') called from file '/home/peff/compile/git/git-send-email' line 1348
> @ = main::process_address_list('foo@xxxxxxxxxxx,bar@xxxxxxxxxxx') 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:
signature.asc
Description: PGP signature
