[aur-dev] [PATCH] valid_email :: add dns check

[Slavi Pantaleev]

Not sure.

On Mon, Mar 19, 2012 at 11:41 PM, Lukas Fleischer
<archlinux at cryptocrack.de>wrote:

> On Mon, Mar 19, 2012 at 11:33:29PM +0200, Slavi Pantaleev wrote:
> > On Mon, Mar 19, 2012 at 11:14 PM, Lukas Fleischer
> > <archlinux at cryptocrack.de>wrote:
> >
> > > On Mon, Mar 19, 2012 at 09:48:36PM +0100, BlackEagle wrote:
> > > > When an email address is of the correct format check if there is an
> mx
> > > > record for the domain part. This forces people wanting to trash
> > > > something in aur to use a domain part with an mx record.
> > > >
> > > > This will not stop invalid email, it only adds an extra check and a
> > > > possible help for typos.
> > > >
> > > > Signed-off-by: BlackEagle <ike.devolder at gmail.com>
> > > > ---
> > > >  web/lib/aur.inc.php |    9 ++++++++-
> > > >  1 file changed, 8 insertions(+), 1 deletion(-)
> > > >
> > > > diff --git a/web/lib/aur.inc.php b/web/lib/aur.inc.php
> > > > index c662b80..7e9d7de 100644
> > > > --- a/web/lib/aur.inc.php
> > > > +++ b/web/lib/aur.inc.php
> > > > @@ -80,7 +80,14 @@ function check_sid($dbh=NULL) {
> > > >  # verify that an email address looks like it is legitimate
> > > >  #
> > > >  function valid_email($addy) {
> > > > -     return (filter_var($addy, FILTER_VALIDATE_EMAIL) !== false);
> > > > +     if (filter_var($addy, FILTER_VALIDATE_EMAIL) === false)
> > > > +             return false;
> > >
> > > This isn't noted down for the AUR yet but we always use opening and
> > > closing braces in other Arch projects, such as pacman (even if it's
> just
> > > a one-line block). It probably makes sense to add such a rule to our
> > > coding guidelines.
> > >
> > > Could you please change that when resubmitting the patch? I'll take
> care
> > > of adding that to our guidelines later...
> > >
> > > > +
> > > > +     $domain = substr($addy, strrpos($addy, '@')+1);
> > >
> > > Please add spaces before and after the plus sign (it should look like
> > > "strrpos($addy, '@') + 1").
> > >
> > > > +     if (!checkdnsrr($domain, 'MX') || checkdnsrr($domain, 'A'))
> > >
> > > That check doesn't look correct to me. Why would we reject mail
> > > addresses if there's an A record for the domain part? You probably
> > > forgot parentheses and/or a negation ("!") here.
> > >
> > > Also, I'm not sure why you're checking for A records at all? Could you
> > > please elaborate on this?
> > >
> >
> > Email delivery falls back to where the A record points if an MX record
> does
> > not exist.
> > https://en.wikipedia.org/wiki/MX_record#History_of_fallback_to_A
> >
> > I think the correct check should be:
> >
> > if (! (checkdnsrr($domain, 'MX') || checkdnsrr($domain, 'A'))) {
> >     return false;
> > }
>
> What about AAAA records? Shouldn't we check for these as well?
>

It doesn't seem like the Wikipedia article says anything about those.
It probably doesn't matter much at this point. I don't think there are many
mail servers now that have no MX/A records, but have IPv6 connectivity.


>
> >
> >
> > > Oh, and don't forget to add braces here as well :)
> > >
> > > > +             return false;
> > > > +
> > > > +     return true;
> > > >  }
> > > >
> > > >  # a new seed value for mt_srand()
> > > > --
> > > > 1.7.9.4
> > >
>