[geeklog-users] Suggested changes to lib-common.php/COM_isemail()

[geeklog-users-admin at lists.geeklog.net]

Hi,

On Tue, 2 Sep 2003 10:20:21 +0200 geeklog-users-admin at lists.geeklog.net wrote:

> Bob wrote:
> 
> >[Sorry if this isn't the right list for posting bugs/enhancements...]
> 
> Didn't see the "Report a bug" link on geeklog.net? But it's okay to
> discuss problems here or on the forums before actually sending bug reports.

I'd rather have confirmation and discussion; I'd hate to have my first
contact with developers be a bug report that's either wrong, well-known
but as yet unfixed, or otherwise answered in the docs or a hundred
identical forum or mailing list posts.

And yes, I completely missed the "Report a bug" link. Oops... :/

> >The upshot is, COM_isemail in lib-common.php chokes on addresses
> >containing the literal '+'.
> 
> As the comment above that function suggests, it is known not to be
> strictly RFC822 compliant.

It's been a while since I've looked through RFC822; I usually refer to
the example case in Jeffrey Friedl's "Mastering Regular Expressions"
where he beats email address validation to death (he ends up with a 6.5K
regex that still doesn't cover all the edge cases.)

Also, I believe you'll find '+' is acceptable in the local part of the
address according to RFC2822 and also RFC822 (superceded by 2822), at
least in my brief reading of it. Prgamatically, Postfix tends to be
pretty RFC-compliant; I can't imagine it promoting non-compliant
behavior, at least without a lot of argument and some very solid
reasoning.

But this may not be important - see below.

> Btw, I just love it when people report problems with things that I just
> thought about changing a few hours ago :-)

Great minds think alike? :)

> I'm currently in the process
> of changing Geeklog's email functions so that they will use PEAR::Mail.
> That package includes a class to check for RFC822 compliant email
> addresses and I was already thinking about rewriting COM_ismail() to use
> that class ...

It looks like Mail_RFC822::isValidInetAddress has the same problem with
not recognizing '+' as valid, though I agree, using PEAR::Mail is a
better solution, if only so you have less code to maintain. Reduce,
reuse, recycle.

The problem is that Mail_RFC822::parseAddressList parses well but
Mail_RFC822::isValidInetAddress is too simplistic. For example, it won't
match user at example.museum because it expects TLDs to match [a-z]{2,4}.

Mail_RFC822 is useful for parsing address lists, removing comments in
addresses, breaking addresses into mailbox and host parts, and for
coarse validation, but IMO you'll still want to write your own regexes
for address validation.

> >When digging into this issue, I found two more problems - some TLDs are
> >longer than three characters (.info, .name, .coop, .aero, .museum - see
> >http://www.icann.org/tlds/)
> 
> Hmm, my .info addresses seem to work just fine, i.e. I can sign up with them.

That's probably because of the unquoted '.' before the [a-z]{2,3}. It
works, but for the wrong reason. :/

Anyway, thanks again for the quick response (and all your time and
effort developing Geeklog.) If you want, I'll send you my code for
testing various address schemes. I'm much more of a perl programmer than
a PHP hacker but I manage.

-- Bob