Re: [PATCH] BUG10963: implement a better email verification

[Michael Tremer <michael.tremer@ipfire.org>]

[-- Attachment #1: Type: text/plain, Size: 3151 bytes --]

Hi,

On Thu, 2015-11-12 at 16:27 +0100, Alexander Marx wrote:
> With this patch the new domains with german umlauts are checked. In
> addition we 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.
> 
> Signed-off-by: Alexander Marx <alexander.marx(a)ipfire.org>
> ---
>  config/cfgroot/general-functions.pl | 33 ++++++++++++++++++++++-----
> ------
>  1 file changed, 22 insertions(+), 11 deletions(-)
> 
> diff --git a/config/cfgroot/general-functions.pl
> b/config/cfgroot/general-functions.pl
> index 2b5cd19..55ea5b6 100644
> --- a/config/cfgroot/general-functions.pl
> +++ b/config/cfgroot/general-functions.pl
> @@ -662,13 +662,13 @@ sub validfqdn
>  		if (length ($part) < 1 || length ($part) > 63) {
>  			return 0;}
>  		# Only valid characters are a-z, A-Z, 0-9 and -
> -		if ($part !~ /^[a-zA-Z0-9-]*$/) {
> +		if ($part !~ /^[a-zA-ZöäüÖÄÜ0-9-]*$/) {
>  			return 0;}
>  		# First character can only be a letter or a digit
> -		if (substr ($part, 0, 1) !~ /^[a-zA-Z0-9]*$/) {
> +		if (substr ($part, 0, 1) !~ /^[a-zA-ZöäüÖÄÜ0-9]*$/)
> {
>  			return 0;}
>  		# Last character can only be a letter or a digit
> -		if (substr ($part, -1, 1) !~ /^[a-zA-Z0-9]*$/) {
> +		if (substr ($part, -1, 1) !~ /^[a-zA-ZöäüÖÄÜ0-9]*$/)
> {
>  			return 0;}
>  	}
>  	return 1;

There can't be any of those special characters in the domain name.
These must be escaped by using the IDN standard.

> @@ -747,14 +747,25 @@ sub ipcidr2msk {
>  }
>  
>  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 @positionen = split( /\@/, $address );
> +    my $anz=@positionen;

The variables in this code are not English.

> +
> +    #check if we have one part before and after '@'
> +    return 0 if ( $anz != 2 );
> +
> +    #check if one of the parts starts or ends with a dot
> +    return 0 if ( substr($positionen[0],0,1) eq '.' );
> +    return 0 if ( substr($positionen[0],-1,1) eq '.' );
> +    return 0 if ( substr($positionen[1],0,1) eq '.' );
> +    return 0 if ( substr($positionen[1],-1,1) eq '.' );
> +
> +    #check first addresspart (before '@' sign)
> +    return 0 if  ( $positionen[0] !~ m/^[a-zA-Z0-9\.!\-\+#]+$/ );

In this part may be special characters. I don't think that this is a
good thing though, but some mailboxes allow unicode.

> +
> +    #check second addresspart (after '@' sign)
> +    return 0 if  ( !&validfqdn( $positionen[1] ) );
> +

You could write this easier as:

  return &validfqdn(...);

If the validfqdn() method finds an invalid domain name, the return code
would be false. Otherwise it would be true.

>      return 1;
>  }
>  

-Michael

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]