git/list[1] front-page[2] threads[3] people[4] search[5] about
 

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

From
Bagas Sanjaya <bagasdotme@gmail.com>
Date
Oct 11, 2023, 13:41 UTC
Message-ID
<ZSal-mQIZAUBaq6g@debian.me>
In-Reply-To
<20230925161748.GA2149383@coredump.intra.peff.net>
On Mon, Sep 25, 2023 at 12:17:48PM -0400, Jeff King wrote:
Show 51 quoted lines
> 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
Previous: Jeff KingNext: Michael Strawbridge
Message 7 of 47 in “[REGRESSION] uninitialized value $address in git send-email when given multiple recipients separated by commas”
  1. Bagas SanjayaSep 22, 2023
  2. Jeff KingSep 24, 2023
  3. Bagas SanjayaSep 25, 2023
  4. Jeff KingSep 25, 2023
  5. Todd ZullingerSep 25, 2023
  6. Jeff KingSep 25, 2023
  7. Bagas SanjayaOct 11, 2023
  8. Michael StrawbridgeOct 11, 2023
  9. send-email: move process_address_list earlier to avoid, uninitialized address errorMichael Strawbridge, Oct 11, 2023
  10. Michael StrawbridgeOct 11, 2023
  11. Junio C HamanoOct 11, 2023
  12. Jeff KingOct 11, 2023
  13. Junio C HamanoOct 11, 2023
  14. Jeff KingOct 11, 2023
  15. Michael StrawbridgeOct 13, 2023
  16. Jeff KingOct 20, 2023
  17. Jeff KingOct 20, 2023
  18. 0/3 some send-email --compose fixesJeff King, Oct 20, 2023
  19. 1/3 doc/send-email: mention handling of "reply-to" with --composeJeff King, Oct 20, 2023
  20. 2/3 Revert "send-email: extract email-parsing code into a subroutine"Jeff King, Oct 20, 2023
  21. Oswald BuddenhagenOct 20, 2023
  22. Jeff KingOct 23, 2023
  23. Oswald BuddenhagenOct 23, 2023
  24. Jeff KingOct 25, 2023
  25. Oswald BuddenhagenOct 25, 2023
  26. Junio C HamanoOct 27, 2023
  27. Jeff KingOct 30, 2023
  28. Junio C HamanoOct 20, 2023
  29. Jeff KingOct 23, 2023
  30. 3/3 send-email: handle to/cc/bcc from --compose messageJeff King, Oct 20, 2023
  31. Eric SunshineOct 20, 2023
  32. Junio C HamanoOct 20, 2023
  33. Jeff KingOct 23, 2023
  34. Michael StrawbridgeOct 24, 2023
  35. send-email: move validation code below process_address_listMichael Strawbridge, Oct 24, 2023
  36. Junio C HamanoOct 24, 2023
  37. Junio C HamanoOct 24, 2023
  38. Michael StrawbridgeOct 25, 2023
  39. send-email: move validation code below process_address_listMichael Strawbridge, Oct 25, 2023
  40. Junio C HamanoOct 26, 2023
  41. Michael StrawbridgeOct 26, 2023
  42. Jeff KingOct 25, 2023
  43. Michael StrawbridgeOct 25, 2023
  44. Uwe Kleine-KönigOct 25, 2023
  45. Junio C HamanoOct 27, 2023
  46. Bagas SanjayaOct 20, 2023
  47. Bagas SanjayaSep 26, 2023

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.