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

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

Hi,

[Sorry if this isn't the right list for posting bugs/enhancements...]

I have a fresh installation of Geeklog and all seems well. I'm running
into a problem where I can't register with an address of the form
username+tag at example.com. Postfix and other MTAs can be configured to
deliver mail directed at username+tag to the account username. This
technique is useful for sorting mail and determining where an email
address was harvested from when it later gets spammed.

The upshot is, COM_isemail in lib-common.php chokes on addresses
containing the literal '+'.

Original regex:

    if( eregi( "^([-_0-9a-z])+([-._0-9a-z])*@[0-9a-z]([-.]?[0-9a-z])*.[a-z]{2,3}$", $email, $check ))

Suggested change:

    if( eregi( "^([-_0-9a-z])+([-._0-9a-z])*([+])?([-._0-9a-z])*@[0-9a-z]([-.]?[0-9a-z])*.[a-z]{2,3}$", $email, $check ))

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/) and the final '.' before the TLD is
unquoted, so it's treated as a metacharater rather than as a literal.

Finally, just for code cleanliness, I think you can lose some parens
since you're not using the results of $check.

Suggested change (incorporating everything above):

    if( eregi( "^[-_0-9a-z]+[-._0-9a-z]*\\+?[-._0-9a-z]*@[0-9a-z]([-.]?[0-9a-z])*\\.[a-z]{2,6}$", $email, $check ))

I've tested this and it works, correctly accepting
username+something at example.museum and username+ at example.museum and
correctly rejecting username+ at exampleZmuseum and
username+s+omething at example.museum. I'm not sure about the last case but
I've never seen an address of this form before; I'm not sure it's legal
and I'm too lazy to test it. :) Regardless, if it's legal syntax, the
regex is easy enough to modify to accept addresses of that form.

I know, someone could easily spend a month trying to make COM_isemail
identify all legal email addresses; that's not my intent. I just wanted
to point out that the routine chokes on a common address variant and
suggest a few fixes. I try not to point out problems in code without at
least suggesting a fix.

One more thing - I wasn't aware that PHP ignored case when considering
subroutine names. COM_isemail is often called as COM_isEmail (see
below). At some point you might want to clean that up...

hth,

-- Bob

$ find geeklog-1.3.8-1 -type f -print | xargs egrep -i isemail
geeklog-1.3.8-1/public_html/lib-common.php:function COM_isemail( $email )
geeklog-1.3.8-1/public_html/profiles.php:        if (COM_isemail($authoremail)) {
geeklog-1.3.8-1/public_html/admin/user.php:            if (COM_isEmail($email)) {
geeklog-1.3.8-1/public_html/admin/user.php:            } // end if COM_isEmail($email)
geeklog-1.3.8-1/public_html/usersettings.php:    if (!COM_isEmail ($A['email'])) {
geeklog-1.3.8-1/public_html/users.php:    if (COM_isEmail($email)) {