Not sure. On Mon, Mar 19, 2012 at 11:41 PM, Lukas Fleischer <archli...@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 > > <archli...@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.devol...@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 > > > >