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

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

[Uwe Kleine-König]

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

Hello,

	$ git send-email --to 'A B <a@b.org>, C D <c@d.org>' lala.patch
	Use of uninitialized value $address in sprintf at /usr/lib/git-core/git-send-email line 1172.
	error: unable to extract a valid address from:

This happens for me with git 2.42.0 and also on master (59167d7d09fd, "The seventeenth batch").

Bisection points at

	a8022c5f7b67 ("send-email: expose header information to git-send-email's sendemail-validate hook")

I didn't try to understand that change and fix the problem.

Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | https://www.pengutronix.de/ |

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

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

[Kristoffer Haugsbakk]

On Fri, Oct 13, 2023, at 16:14, Uwe Kleine-König wrote:
> 	$ git send-email --to 'A B <a@b.org>, C D <c@d.org>' lala.patch
> 	Use of uninitialized value $address in sprintf at 

This looks the same problem as https://lore.kernel.org/git/ZQ1eGzqfyoeeTBUq@debian.me/

-- 
Kristoffer Haugsbakk

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

[Uwe Kleine-König]

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

hello,

On Fri, Oct 13, 2023 at 04:14:37PM +0200, Uwe Kleine-König wrote:
> Hello,
> 
> 	$ git send-email --to 'A B <a@b.org>, C D <c@d.org>' lala.patch
> 	Use of uninitialized value $address in sprintf at /usr/lib/git-core/git-send-email line 1172.
> 	error: unable to extract a valid address from:
> 
> This happens for me with git 2.42.0 and also on master (59167d7d09fd, "The seventeenth batch").
> 
> Bisection points at
> 
> 	a8022c5f7b67 ("send-email: expose header information to git-send-email's sendemail-validate hook")
> 
> I didn't try to understand that change and fix the problem.

Another (similar?) problem with non-ascii-chars:

	$ git send-email --to 'Will Deacon <will@kernel.org>' --to 'Krzysztof Wilczyński <kw@linux.com>' --to 'Lorenzo Pieralisi <lpieralisi@kernel.org>' --cc 'Rob Herring <robh@kernel.org>' --to 'Bjorn Helgaas <bhelgaas@google.com>' --cc 'linux-pci@vger.kernel.org' --cc kernel@pengutronix.de -1 --base=@~
	Use of uninitialized value $address in sprintf at /home/uwe/gsrc/git/git-send-email line 1162.
	error: unable to extract a valid address from:

Bisection points to the same commit, when dropping ń in Krzysztof's
name, it works fine.

Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | https://www.pengutronix.de/ |

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

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

[Michael Strawbridge]

On 10/20/23 06:04, Uwe Kleine-König wrote:
> hello,
> 
> On Fri, Oct 13, 2023 at 04:14:37PM +0200, Uwe Kleine-König wrote:
>> Hello,
>>
>> 	$ git send-email --to 'A B <a@b.org>, C D <c@d.org>' lala.patch
>> 	Use of uninitialized value $address in sprintf at /usr/lib/git-core/git-send-email line 1172.
>> 	error: unable to extract a valid address from:
>>
>> This happens for me with git 2.42.0 and also on master (59167d7d09fd, "The seventeenth batch").
>>
>> Bisection points at
>>
>> 	a8022c5f7b67 ("send-email: expose header information to git-send-email's sendemail-validate hook")
>>
>> I didn't try to understand that change and fix the problem.
> 
> Another (similar?) problem with non-ascii-chars:
> 
> 	$ git send-email --to 'Will Deacon <will@kernel.org>' --to 'Krzysztof Wilczyński <kw@linux.com>' --to 'Lorenzo Pieralisi <lpieralisi@kernel.org>' --cc 'Rob Herring <robh@kernel.org>' --to 'Bjorn Helgaas <bhelgaas@google.com>' --cc 'linux-pci@vger.kernel.org' --cc kernel@pengutronix.de -1 --base=@~
> 	Use of uninitialized value $address in sprintf at /home/uwe/gsrc/git/git-send-email line 1162.
> 	error: unable to extract a valid address from:
> 
> Bisection points to the same commit, when dropping ń in Krzysztof's
> name, it works fine.
> 
This is interesting.  Thanks for reporting it.  If you are able, could you please try the patches found in the below threads:
- https://public-inbox.org/git/20230918212004.GC2163162@coredump.intra.peff.net/T/#mae64003cbb72f015bf5c0c04216524fcb6bb8d09
- https://public-inbox.org/git/f5c6a72b-f888-4d43-8be8-3ce2c878c669@gmail.com/T/#mca12dc95ccfd3ce2b94e7752ebaae9891201084f

Thanks,
Michael

> Best regards
> Uwe
> 

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

[Uwe Kleine-König]

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

Hello,

On Fri, Oct 20, 2023 at 05:06:36PM -0400, Michael Strawbridge wrote:
> On 10/20/23 06:04, Uwe Kleine-König wrote:
> > hello,
> > 
> > On Fri, Oct 13, 2023 at 04:14:37PM +0200, Uwe Kleine-König wrote:
> >> Hello,
> >>
> >> 	$ git send-email --to 'A B <a@b.org>, C D <c@d.org>' lala.patch
> >> 	Use of uninitialized value $address in sprintf at /usr/lib/git-core/git-send-email line 1172.
> >> 	error: unable to extract a valid address from:
> >>
> >> This happens for me with git 2.42.0 and also on master (59167d7d09fd, "The seventeenth batch").
> >>
> >> Bisection points at
> >>
> >> 	a8022c5f7b67 ("send-email: expose header information to git-send-email's sendemail-validate hook")
> >>
> >> I didn't try to understand that change and fix the problem.
> > 
> > Another (similar?) problem with non-ascii-chars:
> > 
> > 	$ git send-email --to 'Will Deacon <will@kernel.org>' --to 'Krzysztof Wilczyński <kw@linux.com>' --to 'Lorenzo Pieralisi <lpieralisi@kernel.org>' --cc 'Rob Herring <robh@kernel.org>' --to 'Bjorn Helgaas <bhelgaas@google.com>' --cc 'linux-pci@vger.kernel.org' --cc kernel@pengutronix.de -1 --base=@~
> > 	Use of uninitialized value $address in sprintf at /home/uwe/gsrc/git/git-send-email line 1162.
> > 	error: unable to extract a valid address from:
> > 
> > Bisection points to the same commit, when dropping ń in Krzysztof's
> > name, it works fine.
> > 
> This is interesting.  Thanks for reporting it.  If you are able, could you please try the patches found in the below threads:
> - https://public-inbox.org/git/20230918212004.GC2163162@coredump.intra.peff.net/T/#mae64003cbb72f015bf5c0c04216524fcb6bb8d09

On main (2e8e77cbac8a) this one is already applied, with that the error
message reduces to:

	$ git send-email --to 'Uwe Kleine-König <u.kleine-koenig@pengutronix.de>' -1
	error: unable to extract a valid address from: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>

> - https://public-inbox.org/git/f5c6a72b-f888-4d43-8be8-3ce2c878c669@gmail.com/T/#mca12dc95ccfd3ce2b94e7752ebaae9891201084f

This one doesn't help either. With it applied on top of main I get the
sams result as on vanilla main.

Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | https://www.pengutronix.de/ |

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

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

[Michael Strawbridge]

On 10/24/23 09:00, Uwe Kleine-König wrote:
> Hello,
> 
> On Fri, Oct 20, 2023 at 05:06:36PM -0400, Michael Strawbridge wrote:
>> On 10/20/23 06:04, Uwe Kleine-König wrote:
>>> hello,
>>>
>>> On Fri, Oct 13, 2023 at 04:14:37PM +0200, Uwe Kleine-König wrote:
>>>> Hello,
>>>>
>>>> 	$ git send-email --to 'A B <a@b.org>, C D <c@d.org>' lala.patch
>>>> 	Use of uninitialized value $address in sprintf at /usr/lib/git-core/git-send-email line 1172.
>>>> 	error: unable to extract a valid address from:
>>>>
>>>> This happens for me with git 2.42.0 and also on master (59167d7d09fd, "The seventeenth batch").
Hm.  I tried reproing with master (59167d7d09fd, "The seventeenth batch") but I don't seem to see an error:
```
$ git send-email --to 'Uwe Kleine-König <u.kleine-koenig@pengutronix.de>' -1 --smtp-server="$(pwd)/fake.sendmail"

/tmp/FEGzLrm_4U/0001-test-test.patch
1
(mbox) Adding cc: Your Name <you@example.com> from line 'From: Your Name <you@example.com>'
(body) Adding cc: Your Name <you@example.com> from line 'Signed-off-by: Your Name <you@example.com>'

From: Your Name <you@example.com>
To: =?UTF-8?q?Uwe=20Kleine-K=C3=B6nig?= <u.kleine-koenig@pengutronix.de>
Cc: Your Name <you@example.com>
Subject: [PATCH] test test
Date: Tue, 24 Oct 2023 18:33:51 +0000
Message-ID: <20231024183350.10-2-you@example.com>
X-Mailer: git-send-email 2.40.0.rc1.1708.gbffa95ed23.dirty
MIME-Version: 1.0
Content-Transfer-Encoding: 8bit

    The Cc list above has been expanded by additional
    addresses found in the patch commit message. By default
    send-email prompts before sending whenever this occurs.
    This behavior is controlled by the sendemail.confirm
    configuration setting.

    For additional information, run 'git send-email --help'.
    To retain the current behavior, but squelch this message,
    run 'git config --global sendemail.confirm auto'.

Send this email? ([y]es|[n]o|[e]dit|[q]uit|[a]ll): y
-i
OK. Log says:
Sendmail: /git/fake.sendmail -i u.kleine-koenig@pengutronix.de you@example.com
From: Your Name <you@example.com>
To: =?UTF-8?q?Uwe=20Kleine-K=C3=B6nig?= <u.kleine-koenig@pengutronix.de>
Cc: Your Name <you@example.com>
Subject: [PATCH] test test
Date: Tue, 24 Oct 2023 18:33:51 +0000
Message-ID: <20231024183350.10-2-you@example.com>
X-Mailer: git-send-email 2.40.0.rc1.1708.gbffa95ed23.dirty
MIME-Version: 1.0
Content-Transfer-Encoding: 8bit

Result: OK
```

I think my terminal may be mangling the UTF-8 encoding but git send-email doesn't seem to complain.

Only when I go to an older git (version 2.42.0 for example) do I then get:

```
Use of uninitialized value $address in sprintf at /usr/lib/git-core/git-send-email line 1172.
error: unable to extract a valid address from: 
```

This would be expected because https://public-inbox.org/git/20230918212004.GC2163162@coredump.intra.peff.net/T/#mae64003cbb72f015bf5c0c04216524fcb6bb8d09
is meant to fix it.

Just in case it was the "from" address, I tried using 'Uwe Kleine-König <u.kleine-koenig@pengutronix.de>'
as the from address.  This still worked without issue for (59167d7d09fd, "The seventeenth batch").


>>>>
>>>> Bisection points at
>>>>
>>>> 	a8022c5f7b67 ("send-email: expose header information to git-send-email's sendemail-validate hook")
>>>>
>>>> I didn't try to understand that change and fix the problem.
>>>
>>> Another (similar?) problem with non-ascii-chars:
>>>
>>> 	$ git send-email --to 'Will Deacon <will@kernel.org>' --to 'Krzysztof Wilczyński <kw@linux.com>' --to 'Lorenzo Pieralisi <lpieralisi@kernel.org>' --cc 'Rob Herring <robh@kernel.org>' --to 'Bjorn Helgaas <bhelgaas@google.com>' --cc 'linux-pci@vger.kernel.org' --cc kernel@pengutronix.de -1 --base=@~
>>> 	Use of uninitialized value $address in sprintf at /home/uwe/gsrc/git/git-send-email line 1162.
>>> 	error: unable to extract a valid address from:
>>>
>>> Bisection points to the same commit, when dropping ń in Krzysztof's
>>> name, it works fine.

I tried this as well with seventeen and it succeeds:
```
/tmp/GfTxbwKSx_/0001-test-test.patch
1
(mbox) Adding cc: =?UTF-8?q?Uwe=20Kleine-K=C3=B6nig?= <u.kleine-koenig@pengutronix.de> from line 'From: =?UTF-8?q?Uwe=20Kleine-K=C3=B6nig?= <u.kleine-koenig@pengutronix.de>'
(body) Adding cc: Uwe Kleine-König <u.kleine-koenig@pengutronix.de> from line 'Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>'

From: =?UTF-8?q?Uwe=20Kleine-K=C3=B6nig?= <u.kleine-koenig@pengutronix.de>
To: Will Deacon <will@kernel.org>,
	=?UTF-8?q?Krzysztof=20Wilczy=C5=84ski?= <kw@linux.com>,
	Lorenzo Pieralisi <lpieralisi@kernel.org>,
	Bjorn Helgaas <bhelgaas@google.com>
Cc: Rob Herring <robh@kernel.org>,
	linux-pci@vger.kernel.org,
	kernel@pengutronix.de,
	=?UTF-8?q?Uwe=20Kleine-K=C3=B6nig?= <u.kleine-koenig@pengutronix.de>
Subject: [PATCH] test test
Date: Tue, 24 Oct 2023 18:57:16 +0000
Message-ID: <20231024185715.10-2-u.kleine-koenig@pengutronix.de>
X-Mailer: git-send-email 2.40.0.rc1.1708.gbffa95ed23.dirty
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

    The Cc list above has been expanded by additional
    addresses found in the patch commit message. By default
    send-email prompts before sending whenever this occurs.
    This behavior is controlled by the sendemail.confirm
    configuration setting.

    For additional information, run 'git send-email --help'.
    To retain the current behavior, but squelch this message,
    run 'git config --global sendemail.confirm auto'.

Send this email? ([y]es|[n]o|[e]dit|[q]uit|[a]ll): y
-i
OK. Log says:
Sendmail: /git/fake.sendmail -i will@kernel.org kw@linux.com lpieralisi@kernel.org bhelgaas@google.com robh@kernel.org linux-pci@vger.kernel.org kernel@pengutronix.de u.kleine-koenig@pengutronix.de
From: =?UTF-8?q?Uwe=20Kleine-K=C3=B6nig?= <u.kleine-koenig@pengutronix.de>
To: Will Deacon <will@kernel.org>,
	=?UTF-8?q?Krzysztof=20Wilczy=C5=84ski?= <kw@linux.com>,
	Lorenzo Pieralisi <lpieralisi@kernel.org>,
	Bjorn Helgaas <bhelgaas@google.com>
Cc: Rob Herring <robh@kernel.org>,
	linux-pci@vger.kernel.org,
	kernel@pengutronix.de,
	=?UTF-8?q?Uwe=20Kleine-K=C3=B6nig?= <u.kleine-koenig@pengutronix.de>
Subject: [PATCH] test test
Date: Tue, 24 Oct 2023 18:57:16 +0000
Message-ID: <20231024185715.10-2-u.kleine-koenig@pengutronix.de>
X-Mailer: git-send-email 2.40.0.rc1.1708.gbffa95ed23.dirty
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Result: OK
```

Perhaps there is another variable involved?

>>>
>> This is interesting.  Thanks for reporting it.  If you are able, could you please try the patches found in the below threads:
>> - https://public-inbox.org/git/20230918212004.GC2163162@coredump.intra.peff.net/T/#mae64003cbb72f015bf5c0c04216524fcb6bb8d09
> 
> On main (2e8e77cbac8a) this one is already applied, with that the error
> message reduces to:
> 
> 	$ git send-email --to 'Uwe Kleine-König <u.kleine-koenig@pengutronix.de>' -1
> 	error: unable to extract a valid address from: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
> 
>> - https://public-inbox.org/git/f5c6a72b-f888-4d43-8be8-3ce2c878c669@gmail.com/T/#mca12dc95ccfd3ce2b94e7752ebaae9891201084f
> 
> This one doesn't help either. With it applied on top of main I get the
> sams result as on vanilla main.
> 
> Best regards
> Uwe
> 

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

[Uwe Kleine-König]

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

Hello Michael,

On Tue, Oct 24, 2023 at 03:00:38PM -0400, Michael Strawbridge wrote:
> On 10/24/23 09:00, Uwe Kleine-König wrote:
> > On Fri, Oct 20, 2023 at 05:06:36PM -0400, Michael Strawbridge wrote:
> >> On 10/20/23 06:04, Uwe Kleine-König wrote:
> >>> On Fri, Oct 13, 2023 at 04:14:37PM +0200, Uwe Kleine-König wrote:
> >>>> 	$ git send-email --to 'A B <a@b.org>, C D <c@d.org>' lala.patch
> >>>> 	Use of uninitialized value $address in sprintf at /usr/lib/git-core/git-send-email line 1172.
> >>>> 	error: unable to extract a valid address from:
> >>>>
> >>>> This happens for me with git 2.42.0 and also on master (59167d7d09fd, "The seventeenth batch").
> Hm.  I tried reproing with master (59167d7d09fd, "The seventeenth batch") but I don't seem to see an error:
> ```
> $ git send-email --to 'Uwe Kleine-König <u.kleine-koenig@pengutronix.de>' -1 --smtp-server="$(pwd)/fake.sendmail"
> [...]

I debugged a bit and if I do

	mv .git/hooks/sendemail-validate .git/hooks/sendemail-validate.bak

git send-email --to 'Uwe Kleine-König <u.kleine-koenig@pengutronix.de>'
starts to work for me, too.

I'd guess the content of my sendemail-validate script doesn't matter
much, but for the record, it's:

	#!/bin/sh
	# installed by patatt install-hook
	patatt sign --hook "${1}"

Does the problem reproduce on your end with a sendemail-validate script?

Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | https://www.pengutronix.de/ |

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

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

[Jeff King]

On Tue, Oct 24, 2023 at 10:43:18PM +0200, Uwe Kleine-König wrote:

> I debugged a bit and if I do
> 
> 	mv .git/hooks/sendemail-validate .git/hooks/sendemail-validate.bak
> 
> git send-email --to 'Uwe Kleine-König <u.kleine-koenig@pengutronix.de>'
> starts to work for me, too.
> 
> I'd guess the content of my sendemail-validate script doesn't matter
> much, but for the record, it's:
> 
> 	#!/bin/sh
> 	# installed by patatt install-hook
> 	patatt sign --hook "${1}"
> 
> Does the problem reproduce on your end with a sendemail-validate script?

I can reproduce with:

  git init
  echo foo >file && git add file && git commit -m foo
  echo 'exit 0' >.git/hooks/sendemail-validate
  chmod +x .git/hooks/sendemail-validate
  git send-email --dry-run --to='pëff <peff@peff.net>' -1

Note that the bug will only trigger if Email::Valid is installed. I
think this is the same issue being discussed elsewhere. The call to
process_address_list() sanitizes it to use rfc2047 encoding, which is
necessary for it to be syntactically valid.

So the patch to move the validation later in the process here:

  https://lore.kernel.org/git/ee56c4df-e030-45f9-86a9-94fb3540db60@amd.com/

fixes it.

-Peff

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

[Uwe Kleine-König]

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

Hello,

On Wed, Oct 25, 2023 at 03:21:04AM -0400, Jeff King wrote:
> On Tue, Oct 24, 2023 at 10:43:18PM +0200, Uwe Kleine-König wrote:
> 
> > I debugged a bit and if I do
> > 
> > 	mv .git/hooks/sendemail-validate .git/hooks/sendemail-validate.bak
> > 
> > git send-email --to 'Uwe Kleine-König <u.kleine-koenig@pengutronix.de>'
> > starts to work for me, too.
> > 
> > I'd guess the content of my sendemail-validate script doesn't matter
> > much, but for the record, it's:
> > 
> > 	#!/bin/sh
> > 	# installed by patatt install-hook
> > 	patatt sign --hook "${1}"
> > 
> > Does the problem reproduce on your end with a sendemail-validate script?
> 
> I can reproduce with:
> 
>   git init
>   echo foo >file && git add file && git commit -m foo
>   echo 'exit 0' >.git/hooks/sendemail-validate
>   chmod +x .git/hooks/sendemail-validate
>   git send-email --dry-run --to='pëff <peff@peff.net>' -1
> 
> Note that the bug will only trigger if Email::Valid is installed.

I can confirm I have this package installed (via Debian's
libemail-valid-perl).

> I think this is the same issue being discussed elsewhere. The call to
> process_address_list() sanitizes it to use rfc2047 encoding, which is
> necessary for it to be syntactically valid.
> 
> So the patch to move the validation later in the process here:
> 
>   https://lore.kernel.org/git/ee56c4df-e030-45f9-86a9-94fb3540db60@amd.com/
> 
> fixes it.

Tested and indeed that fixes my usecase. With that patch also the
original regression is fixed and I can do

	git send-email -1 --to 'Uwe Kleine-König <u.kleine-koenig@pengutronix.de>, pëff <peff@peff.net>'

again. \o/

Thanks
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | https://www.pengutronix.de/ |

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

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

[Junio C Hamano]

Jeff King <peff@peff.net> 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.

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

[Michael Strawbridge]

On 10/26/23 08:41, Junio C Hamano wrote:
> Jeff King <peff@peff.net> 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/20230918212004.GC2163162@coredump.intra.peff.net/T/#m9e0211a8ad387adbbadf31dcfcd7982d4046633d
- Regression: git send-email fails with "Use of uninitialized value $address" + "unable to extract a valid address" - https://public-inbox.org/git/68d7e5c3-6b4a-4d0d-9885-f3d4e2199f26@amd.com/T/#m1411c155e11ad9c5d913d22d1d11180ed56eabc7

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

[Todd Zullinger]

Michael Strawbridge wrote:
> On 10/26/23 08:41, Junio C Hamano wrote:
>> Jeff King <peff@peff.net> 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/20230918212004.GC2163162@coredump.intra.peff.net/T/#m9e0211a8ad387adbbadf31dcfcd7982d4046633d
> - Regression: git send-email fails with "Use of uninitialized value $address" + "unable to extract a valid address" - https://public-inbox.org/git/68d7e5c3-6b4a-4d0d-9885-f3d4e2199f26@amd.com/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

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

[Junio C Hamano]

Todd Zullinger <tmz@pobox.com> writes:

> 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.

Reducing the possible "valid" configurations we support is a very
tempting proposition.

> 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.

Very true.

> 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.

Yes, it is very tempting, and given that we have to keep our
fallback codepath working for those without Email::Valid ANYWAY,
as long as the dependency is merely optional, I very much agree
with your argument here.

> 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.

I am of two minds here.  I can sympathize with both positions.

 - Trying to be strict to what we send out to the world by using
   Email::Valid that tries to be more RFC kosher matches "be strict
   in what you send out, be lenient in what you receive" mantra

 - Rejecting what works in practice and in real world tend to help
   users.

If we require Email::Valid, then sriking the balance between the
above two will entirely become the responsibility of them; any
end-user who complains "the validation is overly strict" will get
"talk to authors of Email::Valid".

If we ditch Email::Valid, it will become _our_ responsibility, which
means a bit of extra maintenance burden to this project.  But perhaps
it is worth it?  I dunno.

Having Email::Valid as an optional dependency does not place us in a
position better than either of these two, so from that point of view,
too, I like your "we should either make it required or unused, not
an optional dependency" very much.

Thanks.

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

[Jeff King]

On Thu, Oct 26, 2023 at 10:46:30AM -0400, Todd Zullinger wrote:

> 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.

I am somewhat tempted to say the same, but in this case didn't it help
us find a real bug? True, the bug was that we were feeding garbage to
Email::Valid because we were calling it to early, and I _think_ the
ultimate emails we sent would have been OK. But I think we were possibly
feeding that garbage to the user-visible validation hook.

That might just mean we need to beef up our homegrown validation a bit
(though as you indicate here, I am generally of the opinion that the
best validation is trying to actually send things).

-Peff