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@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@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?
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