Warning when GMime is parsing broken email addresses

Warning when GMime is parsing broken email addresses

[Matthieu Lemerre]

Hi,

I just realized that GMime is not robust to some broken email addresses
sent by broken email clients. I received a mail, with the to field being

xxxx.yyyy <uuuu-vvvv@gmail.com>

The email client (Exchange) had forgotten to put a "" around
"xxxx.yyyy", which is necessary because `.' is a special character, and
this confused GMime. This resulted in notmuch storing xxxx.yyyy as the
email address, instead of using uuuu-vvvv@gmail.com. Because of that I
had trouble finding one of my mail using a to: in the search terms.

I have filed a bug report for GMime
(https://bugzilla.gnome.org/show_bug.cgi?id=545333), but I just wanted
to make people using notmuch aware of this problem, because I guess that
such problems might be extremely common.

Maybe it would also be interesting to add a warning/assertion to check
that all email adresses added to the database are correct email
addresses? I.e. check that the `addr' variable in _index_address_mailbox
always has a @. This check is in fact already done using the function
strchr, but a bad value is explicitly ignored...

Matthieu

Re: Warning when GMime is parsing broken email addresses

[Matthieu Lemerre]

The problem is solved in version 2.4.18 (current is
2.4.20). Unfortunately the last version packaged for Debian is 2.4.14...

Matthieu

Re: Warning when GMime is parsing broken email addresses

[Daniel Kahn Gillmor]

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

Re: Warning when GMime is parsing broken email addresses:

On 11/18/2010 01:37 PM, Matthieu Lemerre wrote:
> The problem is solved in version 2.4.18 (current is
> 2.4.20). Unfortunately the last version packaged for Debian is 2.4.14...

is the patch that fixes the problem it backportable to 2.4.14 ?  I'm
sure a report to the debian bts with a patch would be appreciated.  even
if it doesn't make it into the release because of the freeze, a
narrowly-targeted patch might be acceptable for a future point release.

	--dkg (IANARM)


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 900 bytes --]

Re: Warning when GMime is parsing broken email addresses

[Matthieu Lemerre]

On Thu, 18 Nov 2010 13:51:59 -0500, Daniel Kahn Gillmor <dkg@fifthhorseman.net> wrote:
> Re: Warning when GMime is parsing broken email addresses:
> 
> On 11/18/2010 01:37 PM, Matthieu Lemerre wrote:
> > The problem is solved in version 2.4.18 (current is
> > 2.4.20). Unfortunately the last version packaged for Debian is 2.4.14...
> 
> is the patch that fixes the problem it backportable to 2.4.14 ?  I'm
> sure a report to the debian bts with a patch would be appreciated.  even
> if it doesn't make it into the release because of the freeze, a
> narrowly-targeted patch might be acceptable for a future point release.

Don't know. The main problem is that the package seems to have been more
or less abandoned by its maintainer though... 

Re: Warning when GMime is parsing broken email addresses

[Daniel Kahn Gillmor]

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

On 11/20/2010 06:23 AM, Matthieu Lemerre wrote:
> On Thu, 18 Nov 2010 13:51:59 -0500, Daniel Kahn Gillmor <dkg@fifthhorseman.net> wrote:
>> Re: Warning when GMime is parsing broken email addresses:
>>
>> On 11/18/2010 01:37 PM, Matthieu Lemerre wrote:
>>> The problem is solved in version 2.4.18 (current is
>>> 2.4.20). Unfortunately the last version packaged for Debian is 2.4.14...
>>
>> is the patch that fixes the problem it backportable to 2.4.14 ?  I'm
>> sure a report to the debian bts with a patch would be appreciated.  even
>> if it doesn't make it into the release because of the freeze, a
>> narrowly-targeted patch might be acceptable for a future point release.
> 
> Don't know. The main problem is that the package seems to have been more
> or less abandoned by its maintainer though... 

That's certainly not the case for upstream, where i recently had
basically the ideal interaction:

  https://bugzilla.gnome.org/show_bug.cgi?id=635152

If you're saying the debian package has been abandoned, that's even more
of a reason to file a debian bug -- i don't see any listed outstanding
issues that might indicate abandonment on either the library package or
its source:

 http://bugs.debian.org/cgi-bin/pkgreport.cgi?package=libgmime-2.4-2
 http://bugs.debian.org/cgi-bin/pkgreport.cgi?src=gmime2.4

Filing a bug would help to start documenting abandonment, if that is
indeed the case.  Once it's documented, we can try to do something about it.

Regards,

	--dkg


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 900 bytes --]

Re: Warning when GMime is parsing broken email addresses

[Carl Worth]

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

On Wed, 17 Nov 2010 23:20:26 +0100, Matthieu Lemerre <racin@free.fr> wrote:
> Maybe it would also be interesting to add a warning/assertion to check
> that all email adresses added to the database are correct email
> addresses? I.e. check that the `addr' variable in _index_address_mailbox
> always has a @. This check is in fact already done using the function
> strchr, but a bad value is explicitly ignored...

Hmm... we could add a warning like that. But I don't know how useful it
would actually be. (I, for one, am currently not seeing any warnings
From "notmuch new"---maybe I should fix that...).

Since GMime is fixed upstream (as of version 2.4.18) another option
would be to simply make the notmuch build system require a sufficiently
new version of GMime in order to build.

What do you think?

-Carl

-- 
carl.d.worth@intel.com

[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]

Re: Warning when GMime is parsing broken email addresses

[Xavier Maillard]

On Mon, 25 Apr 2011 15:23:41 -0700, Carl Worth <cworth@cworth.org> wrote:
> On Wed, 17 Nov 2010 23:20:26 +0100, Matthieu Lemerre <racin@free.fr> wrote:
> > Maybe it would also be interesting to add a warning/assertion to check
> > that all email adresses added to the database are correct email
> > addresses? I.e. check that the `addr' variable in _index_address_mailbox
> > always has a @. This check is in fact already done using the function
> > strchr, but a bad value is explicitly ignored...
> 
> Hmm... we could add a warning like that. But I don't know how useful it
> would actually be. (I, for one, am currently not seeing any warnings
> From "notmuch new"---maybe I should fix that...).
> 
> Since GMime is fixed upstream (as of version 2.4.18) another option
> would be to simply make the notmuch build system require a sufficiently
> new version of GMime in order to build.
> 
> What do you think?

My vote goes to the latter. Cleaner and really simple solution.

/Xavier

Re: Warning when GMime is parsing broken email addresses

[Pieter Praet]

On Wed, 27 Apr 2011 18:30:09 +0200, Xavier Maillard <xavier@maillard.im> wrote:
> On Mon, 25 Apr 2011 15:23:41 -0700, Carl Worth <cworth@cworth.org> wrote:
> > On Wed, 17 Nov 2010 23:20:26 +0100, Matthieu Lemerre <racin@free.fr> wrote:
> > > Maybe it would also be interesting to add a warning/assertion to check
> > > that all email adresses added to the database are correct email
> > > addresses? I.e. check that the `addr' variable in _index_address_mailbox
> > > always has a @. This check is in fact already done using the function
> > > strchr, but a bad value is explicitly ignored...
> > 
> > Hmm... we could add a warning like that. But I don't know how useful it
> > would actually be. (I, for one, am currently not seeing any warnings
> > From "notmuch new"---maybe I should fix that...).
> > 
> > Since GMime is fixed upstream (as of version 2.4.18) another option
> > would be to simply make the notmuch build system require a sufficiently
> > new version of GMime in order to build.
> > 
> > What do you think?
> 
> My vote goes to the latter. Cleaner and really simple solution.
> 
> /Xavier
> _______________________________________________
> notmuch mailing list
> notmuch@notmuchmail.org
> http://notmuchmail.org/mailman/listinfo/notmuch

I'd say both.

Unfortunately, regressions are not uncommon, and regardless, it'd be
nice to be notified when what we stuff in the db is not sane.

It would however be a good idea IMHO to check email address more
thorougly [1] than simply verifying whether an "@" is present.


Peace

-Pieter


[1] http://www.regular-expressions.info/email.html

Re: Warning when GMime is parsing broken email addresses

[Matthieu Lemerre]

On Wed, 27 Apr 2011 21:59:09 +0200, Pieter Praet <pieter@praet.org> wrote:
> On Wed, 27 Apr 2011 18:30:09 +0200, Xavier Maillard <xavier@maillard.im> wrote:
> > On Mon, 25 Apr 2011 15:23:41 -0700, Carl Worth <cworth@cworth.org> wrote:
> > > On Wed, 17 Nov 2010 23:20:26 +0100, Matthieu Lemerre <racin@free.fr> wrote:
> > > > Maybe it would also be interesting to add a warning/assertion to check
> > > > that all email adresses added to the database are correct email
> > > > addresses? I.e. check that the `addr' variable in _index_address_mailbox
> > > > always has a @. This check is in fact already done using the function
> > > > strchr, but a bad value is explicitly ignored...
> > > 
> > > Since GMime is fixed upstream (as of version 2.4.18) another option
> > > would be to simply make the notmuch build system require a sufficiently
> > > new version of GMime in order to build.
> > > 
> > > What do you think?
> 
> I'd say both.
> 
> Unfortunately, regressions are not uncommon, and regardless, it'd be
> nice to be notified when what we stuff in the db is not sane.
> 
> It would however be a good idea IMHO to check email address more
> thorougly [1] than simply verifying whether an "@" is present.

I agree with that, unless it is undesirable to add a dependency to a
regex library to notmuch? If so, the @ check could still be done (and is
_already_ present).

I don't see the warnings either, but maybe with proper differentiation
between stdout and stderr, I could arrange to print stderr and see the
warning messages in emacs when I explicitely launch my mail
synchronization script.

Matthieu