BUG10963: implement a better email verification
Message ID | 1447671201-9157-1-git-send-email-alexander.marx@ipfire.org |
---|---|
State | Superseded |
Headers |
Return-Path: <development-bounces@lists.ipfire.org> Received: from mail01.ipfire.org (mail01.tremer.info [172.28.1.200]) by septima.ipfire.org (Postfix) with ESMTP id 3D33760FB1 for <patchwork@ipfire.org>; Mon, 16 Nov 2015 11:53:30 +0100 (CET) Received: from hedwig.ipfire.org (localhost [IPv6:::1]) by mail01.ipfire.org (Postfix) with ESMTP id DFC48446C; Mon, 16 Nov 2015 11:53:29 +0100 (CET) Received: from nbk-edv.kappeln2011.lan (ip1f11b49c.dynamic.kabel-deutschland.de [31.17.180.156]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-SHA256 (128/128 bits)) (No client certificate requested) by mail01.ipfire.org (Postfix) with ESMTPSA id A6E934448; Mon, 16 Nov 2015 11:53:27 +0100 (CET) From: Alexander Marx <alexander.marx@ipfire.org> To: development@lists.ipfire.org Subject: [PATCH] BUG10963: implement a better email verification Date: Mon, 16 Nov 2015 11:53:21 +0100 Message-Id: <1447671201-9157-1-git-send-email-alexander.marx@ipfire.org> X-Mailer: git-send-email 1.9.1 X-BeenThere: development@lists.ipfire.org X-Mailman-Version: 2.1.20 Precedence: list List-Id: IPFire development talk <development.lists.ipfire.org> List-Unsubscribe: <http://lists.ipfire.org/mailman/options/development>, <mailto:development-request@lists.ipfire.org?subject=unsubscribe> List-Archive: <http://lists.ipfire.org/pipermail/development/> List-Post: <mailto:development@lists.ipfire.org> List-Help: <mailto:development-request@lists.ipfire.org?subject=help> List-Subscribe: <http://lists.ipfire.org/mailman/listinfo/development>, <mailto:development-request@lists.ipfire.org?subject=subscribe> Errors-To: development-bounces@lists.ipfire.org Sender: "Development" <development-bounces@lists.ipfire.org> |
Message
Alexander Marx
Nov. 16, 2015, 9:53 p.m. UTC
We now check all allowed chars in the address before the @ sign.
To check the fqdn of an email the function validfqdn has been adapted as
well. Here a valid domain part is for example: user@ipfire or user@localhost.localdomain
Signed-off-by: Alexander Marx <alexander.marx@ipfire.org>
---
config/cfgroot/general-functions.pl | 30 +++++++++++++++++++-----------
1 file changed, 19 insertions(+), 11 deletions(-)
Comments
Hi, okay, this looks better, but I still would like to request a change: On Mon, 2015-11-16 at 11:53 +0100, Alexander Marx wrote: > We now check all allowed chars in the address before the @ sign. > > To check the fqdn of an email the function validfqdn has been adapted > as > well. Here a valid domain part is for example: user@ipfire or > user@localhost.localdomain > > Signed-off-by: Alexander Marx <alexander.marx@ipfire.org> > --- > config/cfgroot/general-functions.pl | 30 +++++++++++++++++++-------- > --- > 1 file changed, 19 insertions(+), 11 deletions(-) > > diff --git a/config/cfgroot/general-functions.pl > b/config/cfgroot/general-functions.pl > index 2b5cd19..564a904 100644 > --- a/config/cfgroot/general-functions.pl > +++ b/config/cfgroot/general-functions.pl > @@ -653,9 +653,6 @@ sub validfqdn > # Checks a fully qualified domain name against RFC1035 > my $fqdn = $_[0]; > my @parts = split (/\./, $fqdn); # Split hostname at > the '.' > - if (scalar(@parts) < 2) { # At least two > parts should > - return 0;} # exist in a FQDN > - # (i.e. > hostname.domain) > foreach $part (@parts) { > # Each part should be at least one character in > length > # but no more than 63 characters > @@ -747,14 +744,25 @@ sub ipcidr2msk { > } This function above is called validfqdn and is supposed (according to the comment) to check for a FQDN as defined in RFC1035. You change changes that. Therefore I think this function should be left as it is so other things that use this function don't break. > sub validemail { > - my $mail = shift; > - return 0 if ( $mail !~ /^[0-9a-zA-Z\.\-\_]+\@[0-9a-zA-Z\.\-]+$/ > ); > - return 0 if ( $mail =~ /^[^0-9a-zA-Z]|[^0-9a-zA-Z]$/); > - return 0 if ( $mail !~ /([0-9a-zA-Z]{1})\@./ ); > - return 0 if ( $mail !~ /.\@([0-9a-zA-Z]{1})/ ); > - return 0 if ( $mail =~ /.\.\-.|.\-\..|.\.\..|.\-\-./g ); > - return 0 if ( $mail =~ /.\.\_.|.\-\_.|.\_\..|.\_\-.|.\_\_./g ); > - return 0 if ( $mail !~ /\.([a-zA-Z]{2,4})$/ ); > + my $address = shift; > + my @parts = split( /\@/, $address ); > + my $count=@parts; > + > + #check if we have one part before and after '@' > + return 0 if ( $count != 2 ); > + > + #check if one of the parts starts or ends with a dot > + return 0 if ( substr($parts[0],0,1) eq '.' ); > + return 0 if ( substr($parts[0],-1,1) eq '.' ); > + return 0 if ( substr($parts[1],0,1) eq '.' ); > + return 0 if ( substr($parts[1],-1,1) eq '.' ); > + > + #check first addresspart (before '@' sign) > + return 0 if ( $parts[0] !~ m/^[a-zA-Z0-9\.!\-\+#]+$/ ); > + > + #check second addresspart (after '@' sign) > + return 0 if ( !&validfqdn( $parts[1] ) ); If the FQDN function is not amended, the domain name should just be checked for invalid characters. > + > return 1; > } > -Michael
Well, according to your last mail regarding this patch, you stated that we *should* use the validfqdn function to check the domainpart... (The reason was, that i just checked for valid chars in the domainpart already)?! As we earlier discussed, a valid domain could be "user@ipfire", if we own a top-level domain... I am a little bit confused now... Am 16.11.2015 um 15:12 schrieb Michael Tremer: > Hi, > > okay, this looks better, but I still would like to request a change: > > On Mon, 2015-11-16 at 11:53 +0100, Alexander Marx wrote: >> We now check all allowed chars in the address before the @ sign. >> >> To check the fqdn of an email the function validfqdn has been adapted >> as >> well. Here a valid domain part is for example: user@ipfire or >> user@localhost.localdomain >> >> Signed-off-by: Alexander Marx <alexander.marx@ipfire.org> >> --- >> config/cfgroot/general-functions.pl | 30 +++++++++++++++++++-------- >> --- >> 1 file changed, 19 insertions(+), 11 deletions(-) >> >> diff --git a/config/cfgroot/general-functions.pl >> b/config/cfgroot/general-functions.pl >> index 2b5cd19..564a904 100644 >> --- a/config/cfgroot/general-functions.pl >> +++ b/config/cfgroot/general-functions.pl >> @@ -653,9 +653,6 @@ sub validfqdn >> # Checks a fully qualified domain name against RFC1035 >> my $fqdn = $_[0]; >> my @parts = split (/\./, $fqdn); # Split hostname at >> the '.' >> - if (scalar(@parts) < 2) { # At least two >> parts should >> - return 0;} # exist in a FQDN >> - # (i.e. >> hostname.domain) >> foreach $part (@parts) { >> # Each part should be at least one character in >> length >> # but no more than 63 characters >> @@ -747,14 +744,25 @@ sub ipcidr2msk { >> } > This function above is called validfqdn and is supposed (according to > the comment) to check for a FQDN as defined in RFC1035. You change > changes that. Therefore I think this function should be left as it is > so other things that use this function don't break. > >> sub validemail { >> - my $mail = shift; >> - return 0 if ( $mail !~ /^[0-9a-zA-Z\.\-\_]+\@[0-9a-zA-Z\.\-]+$/ >> ); >> - return 0 if ( $mail =~ /^[^0-9a-zA-Z]|[^0-9a-zA-Z]$/); >> - return 0 if ( $mail !~ /([0-9a-zA-Z]{1})\@./ ); >> - return 0 if ( $mail !~ /.\@([0-9a-zA-Z]{1})/ ); >> - return 0 if ( $mail =~ /.\.\-.|.\-\..|.\.\..|.\-\-./g ); >> - return 0 if ( $mail =~ /.\.\_.|.\-\_.|.\_\..|.\_\-.|.\_\_./g ); >> - return 0 if ( $mail !~ /\.([a-zA-Z]{2,4})$/ ); >> + my $address = shift; >> + my @parts = split( /\@/, $address ); >> + my $count=@parts; >> + >> + #check if we have one part before and after '@' >> + return 0 if ( $count != 2 ); >> + >> + #check if one of the parts starts or ends with a dot >> + return 0 if ( substr($parts[0],0,1) eq '.' ); >> + return 0 if ( substr($parts[0],-1,1) eq '.' ); >> + return 0 if ( substr($parts[1],0,1) eq '.' ); >> + return 0 if ( substr($parts[1],-1,1) eq '.' ); >> + >> + #check first addresspart (before '@' sign) >> + return 0 if ( $parts[0] !~ m/^[a-zA-Z0-9\.!\-\+#]+$/ ); >> + >> + #check second addresspart (after '@' sign) >> + return 0 if ( !&validfqdn( $parts[1] ) ); > If the FQDN function is not amended, the domain name should just be > checked for invalid characters. > >> + >> return 1; >> } >> > -Michael
-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1 Sorry to jump in here since I really don't contribute much to this except testing. But, has anyone simply considered one regex to validate the e-mail? I remembered reading Jeffrey Friedl's book "Mastering Regular Expressions" a long, long time ago, and then Jan Goyvaert/Steve Levithans "Regular Expressions Cookbook" so I went and looked at Goyvaerts web site, http://www.regular-expressions.info/email.html. Goyvaert's "99% matching" e-mail regex is simple: \b[A-Z0-9._%+-]+@[A-Z0-9.-]+\.[A-Z]{2,}\b If anyone wants to look at it, it is discussed at http://www.regular-expressions.info/email.html. He has much more inclusive regex's that will even match some edge cases the above simple one won't, but really, if you just warn when an invalid address is entered (instead of erroring), that might be the simplest solution, and will really simplify the checking. Rod On 11/16/2015 08:12 AM, Michael Tremer wrote: > Hi, > > okay, this looks better, but I still would like to request a > change: > > On Mon, 2015-11-16 at 11:53 +0100, Alexander Marx wrote: >> We now check all allowed chars in the address before the @ sign. >> >> To check the fqdn of an email the function validfqdn has been >> adapted as well. Here a valid domain part is for example: >> user@ipfire or user@localhost.localdomain >> >> Signed-off-by: Alexander Marx <alexander.marx@ipfire.org> --- >> config/cfgroot/general-functions.pl | 30 >> +++++++++++++++++++-------- --- 1 file changed, 19 >> insertions(+), 11 deletions(-) >> >> diff --git a/config/cfgroot/general-functions.pl >> b/config/cfgroot/general-functions.pl index 2b5cd19..564a904 >> 100644 --- a/config/cfgroot/general-functions.pl +++ >> b/config/cfgroot/general-functions.pl @@ -653,9 +653,6 @@ sub >> validfqdn # Checks a fully qualified domain name against RFC1035 >> my $fqdn = $_[0]; my @parts = split (/\./, $fqdn); # Split >> hostname at the '.' - if (scalar(@parts) < 2) { # At least two >> parts should - return 0;} # exist in a FQDN - # (i.e. >> hostname.domain) foreach $part (@parts) { # Each part should be >> at least one character in length # but no more than 63 characters >> @@ -747,14 +744,25 @@ sub ipcidr2msk { } > > This function above is called validfqdn and is supposed (according > to the comment) to check for a FQDN as defined in RFC1035. You > change changes that. Therefore I think this function should be > left as it is so other things that use this function don't break. > >> sub validemail { - my $mail = shift; - return 0 if ( $mail >> !~ /^[0-9a-zA-Z\.\-\_]+\@[0-9a-zA-Z\.\-]+$/ ); - return 0 if >> ( $mail =~ /^[^0-9a-zA-Z]|[^0-9a-zA-Z]$/); - return 0 if ( >> $mail !~ /([0-9a-zA-Z]{1})\@./ ); - return 0 if ( $mail !~ >> /.\@([0-9a-zA-Z]{1})/ ); - return 0 if ( $mail =~ >> /.\.\-.|.\-\..|.\.\..|.\-\-./g ); - return 0 if ( $mail =~ >> /.\.\_.|.\-\_.|.\_\..|.\_\-.|.\_\_./g ); - return 0 if ( >> $mail !~ /\.([a-zA-Z]{2,4})$/ ); + my $address = shift; + >> my @parts = split( /\@/, $address ); + my $count=@parts; + + >> #check if we have one part before and after '@' + return 0 if >> ( $count != 2 ); + + #check if one of the parts starts or >> ends with a dot + return 0 if ( substr($parts[0],0,1) eq '.' >> ); + return 0 if ( substr($parts[0],-1,1) eq '.' ); + return 0 >> if ( substr($parts[1],0,1) eq '.' ); + return 0 if ( >> substr($parts[1],-1,1) eq '.' ); + + #check first addresspart >> (before '@' sign) + return 0 if ( $parts[0] !~ >> m/^[a-zA-Z0-9\.!\-\+#]+$/ ); + + #check second addresspart >> (after '@' sign) + return 0 if ( !&validfqdn( $parts[1] ) ); > > If the FQDN function is not amended, the domain name should just be > checked for invalid characters. > >> + return 1; } >> > > -Michael > - -- Rod Rodolico Daily Data, Inc. POB 140465 Dallas TX 75214-0465 214.827.2170 http://www.dailydata.net -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.4.12 (GNU/Linux) iEYEARECAAYFAlZKoC0ACgkQuVY3UpYMlTSUUwCaAgQ3r7Yj/vguzrIi+EMYbugf BqAAoIIFz7NfAwUhNKVHJKWKpXsjZaQh =KFEn -----END PGP SIGNATURE-----