On Mon, Mar 19, 2012 at 11:14 PM, Lukas Fleischer <[email protected]>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 <[email protected]> > > --- > > 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; } > 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 >
