Subject: Re: Regression: git send-email fails with "Use of uninitialized value $address" + "unable to extract a valid address"

From: Todd Zullinger <[email-masked]>
Date: Thu, 26 Oct 2023 10:46:30 -0400

Michael Strawbridge wrote:
> On 10/26/23 08:41, Junio C Hamano wrote:
>> Jeff King <[email-masked]> writes:
>> 
>>> Note that the bug will only trigger if Email::Valid is installed.
>> 
>> I recall we chased a different bug that depends on the use/non-use
>> of this package a few years ago.  Is the difference significant
>> enough that we may want to install on one but not in another CI
>> environment, like we have a separate CI jobs with exotic settings, I
>> wonder.
> 
> That would make sense to me.  We have had 3 regressions threads
> recently for git send email where Email::Valid was important.
> 
> - [REGRESSION] uninitialized value $address in git send-email when given multiple recipients separated by commas - (this thread)
> - [REGRESSION] uninitialized value $address in git send-email - https://public-inbox.org/git/[email-masked]/T/#m9e0211a8ad387adbbadf31dcfcd7982d4046633d
> - Regression: git send-email fails with "Use of uninitialized value $address" + "unable to extract a valid address" - https://public-inbox.org/git/[email-masked]/T/#m1411c155e11ad9c5d913d22d1d11180ed56eabc7

Alternately, perhaps having Email::Valid as an optional
dependency is worth reconsidering. If it's truly important
to validation, make it a requirement.  If it's not, then
drop it to simplify the code and avoid these sort of issues.

As a (former) distribution packager, having these optional
dependencies which change the behavior is always a tough
position to be in.

If I make the git package require it to ensure consistent
behavior then some folks will -quite rightly- complain that
it should not be a requirement.  If I keep it an optional
dependency, then debugging becomes more difficult for the
reasons we've seen in these recent (and not-so-recent)
threads.

I'd lean toward dropping the dependency entirely and leave
the more basic validation of git-send-email in place.  That
may not catch every type of address error, but I would argue
that what we do without Email::Valid is perfectly reasonable
for checking basic email address syntax sanity.

Further validation will happen along the path of mail
transfer agents and failures should be reported to the
sender in the same way as any other invalid email address.

On a related note, one issue¹ we had reported in Fedora
after making Email::Valid a requirement was that it rejected
messages where the local part was too long, per the relevant
RFC's.  But these were generated addresses from GitLab.  The
addresses worked in practice.  While Email::Valid was
technically correct in rejecting such addresses, it didn't
improve the experience of git send-email users.

¹ https://bugzilla.redhat.com/2046203

-- 
Todd