diff options
| -rw-r--r-- | Bugzilla/FlagType.pm | 14 | ||||
| -rw-r--r-- | Bugzilla/User.pm | 3 | ||||
| -rw-r--r-- | Bugzilla/Util.pm | 26 | ||||
| -rw-r--r-- | docs/en/xml/administration.xml | 10 | ||||
| -rw-r--r-- | t/007util.t | 12 | ||||
| -rw-r--r-- | template/en/default/admin/params/auth.html.tmpl | 11 | ||||
| -rw-r--r-- | template/en/default/global/code-error.html.tmpl | 3 | ||||
| -rw-r--r-- | template/en/default/global/user-error.html.tmpl | 5 | ||||
| -rwxr-xr-x | token.cgi | 4 | ||||
| -rwxr-xr-x | userprefs.cgi | 3 |
10 files changed, 59 insertions, 32 deletions
diff --git a/Bugzilla/FlagType.pm b/Bugzilla/FlagType.pm index ddaa6eb62..b4709212e 100644 --- a/Bugzilla/FlagType.pm +++ b/Bugzilla/FlagType.pm @@ -38,6 +38,8 @@ use Bugzilla::Error; use Bugzilla::Util; use Bugzilla::Group; +use Email::Address; + use base qw(Bugzilla::Object); ############################### @@ -287,15 +289,11 @@ sub _check_cc_list { || ThrowUserError('flag_type_cc_list_invalid', { cc_list => $cc_list }); my @addresses = split(/[,\s]+/, $cc_list); - # We do not call Util::validate_email_syntax because these - # addresses do not require to match 'emailregexp' and do not - # depend on 'emailsuffix'. So we limit ourselves to a simple - # sanity check: - # - match the syntax of a fully qualified email address; - # - do not contain any illegal character. + my $addr_spec = $Email::Address::addr_spec; + # We do not call check_email_syntax() because these addresses do not + # require to match 'emailregexp' and do not depend on 'emailsuffix'. foreach my $address (@addresses) { - ($address =~ /^[\w\.\+\-=]+@[\w\.\-]+\.[\w\-]+$/ - && $address !~ /[\\\(\)<>&,;:"\[\] \t\r\n]/) + ($address !~ /\P{ASCII}/ && $address =~ /^$addr_spec$/) || ThrowUserError('illegal_email_address', {addr => $address, default => 1}); } diff --git a/Bugzilla/User.pm b/Bugzilla/User.pm index 373fc1ef3..23b08c63a 100644 --- a/Bugzilla/User.pm +++ b/Bugzilla/User.pm @@ -195,8 +195,7 @@ sub check_login_name_for_creation { my ($invocant, $name) = @_; $name = trim($name); $name || ThrowUserError('user_login_required'); - validate_email_syntax($name) - || ThrowUserError('illegal_email_address', { addr => $name }); + check_email_syntax($name); # Check the name if it's a new user, or if we're changing the name. if (!ref($invocant) || $invocant->login ne $name) { diff --git a/Bugzilla/Util.pm b/Bugzilla/Util.pm index a04095647..bf8569839 100644 --- a/Bugzilla/Util.pm +++ b/Bugzilla/Util.pm @@ -20,7 +20,7 @@ use base qw(Exporter); format_time validate_date validate_time datetime_from file_mod_time is_7bit_clean bz_crypt generate_random_password - validate_email_syntax clean_text + validate_email_syntax check_email_syntax clean_text get_text template_var disable_utf8 detect_encoding); @@ -552,7 +552,13 @@ sub generate_random_password { sub validate_email_syntax { my ($addr) = @_; my $match = Bugzilla->params->{'emailregexp'}; - my $ret = ($addr =~ /$match/ && $addr !~ /[\\\(\)<>&,;:"\[\] \t\r\n]/); + my $email = $addr . Bugzilla->params->{'emailsuffix'}; + # This regexp follows RFC 2822 section 3.4.1. + my $addr_spec = $Email::Address::addr_spec; + # RFC 2822 section 2.1 specifies that email addresses must + # be made of US-ASCII characters only. + # Email::Address::addr_spec doesn't enforce this. + my $ret = ($addr =~ /$match/ && $email !~ /\P{ASCII}/ && $email =~ /^$addr_spec$/); if ($ret) { # We assume these checks to suffice to consider the address untainted. trick_taint($_[0]); @@ -560,6 +566,15 @@ sub validate_email_syntax { return $ret ? 1 : 0; } +sub check_email_syntax { + my ($addr) = @_; + + unless (validate_email_syntax(@_)) { + my $email = $addr . Bugzilla->params->{'emailsuffix'}; + ThrowUserError('illegal_email_address', { addr => $email }); + } +} + sub validate_date { my ($date) = @_; my $date2; @@ -763,6 +778,7 @@ Bugzilla::Util - Generic utility functions for bugzilla # Validation Functions validate_email_syntax($email); + check_email_syntax($email); validate_date($date); # DB-related functions @@ -1069,6 +1085,12 @@ Do a syntax checking for a legal email address and returns 1 if the check is successful, else returns 0. Untaints C<$email> if successful. +=item C<check_email_syntax($email)> + +Do a syntax checking for a legal email address and throws an error +if the check fails. +Untaints C<$email> if successful. + =item C<validate_date($date)> Make sure the date has the correct format and returns 1 if diff --git a/docs/en/xml/administration.xml b/docs/en/xml/administration.xml index bdcdaeefb..0ececabd2 100644 --- a/docs/en/xml/administration.xml +++ b/docs/en/xml/administration.xml @@ -288,11 +288,11 @@ <para> Defines the regular expression used to validate email addresses used for login names. The default attempts to match fully - qualified email addresses (i.e. 'user@example.com'). Some - Bugzilla installations allow only local user names (i.e 'user' - instead of 'user@example.com'). In that case, the - <command>emailsuffix</command> parameter should be used to define - the email domain. + qualified email addresses (i.e. 'user@example.com') in a slightly + more restrictive way than what is allowed in RFC 2822. + Some Bugzilla installations allow only local user names (i.e 'user' + instead of 'user@example.com'). In that case, this parameter + should be used to define the email domain. </para> </listitem> </varlistentry> diff --git a/t/007util.t b/t/007util.t index e21ec28f8..a73f4dfa7 100644 --- a/t/007util.t +++ b/t/007util.t @@ -11,7 +11,7 @@ use lib 't'; use Support::Files; -use Test::More tests => 15; +use Test::More tests => 17; BEGIN { use_ok(Bugzilla); @@ -58,6 +58,16 @@ foreach my $input (keys %email_strings) { "email_filter('$input')"); } +# validate_email_syntax. We need to override some parameters. +my $params = Bugzilla->params; +$params->{emailregexp} = '.*'; +$params->{emailsuffix} = ''; +my $ascii_email = 'admin@company.com'; +# U+0430 returns the Cyrillic "а", which looks similar to the ASCII "a". +my $utf8_email = "\N{U+0430}dmin\@company.com"; +ok(validate_email_syntax($ascii_email), 'correctly formatted ASCII-only email address is valid'); +ok(!validate_email_syntax($utf8_email), 'correctly formatted email address with non-ASCII characters is rejected'); + # diff_arrays(): my @old_array = qw(alpha beta alpha gamma gamma beta alpha delta epsilon gamma); my @new_array = qw(alpha alpha beta gamma epsilon delta beta delta); diff --git a/template/en/default/admin/params/auth.html.tmpl b/template/en/default/admin/params/auth.html.tmpl index ceb85c984..96aba3c1d 100644 --- a/template/en/default/admin/params/auth.html.tmpl +++ b/template/en/default/admin/params/auth.html.tmpl @@ -93,10 +93,13 @@ "front page will require a login. No anonymous users will " _ "be permitted.", - emailregexp => "This defines the regexp to use for legal email addresses. The " _ - "default tries to match fully qualified email addresses. Another " _ - "popular value to put here is <tt>^[^@]+$</tt>, which means " _ - "'local usernames, no @ allowed.'", + emailregexp => + "This defines the regular expression to use for legal email addresses. " _ + "The default tries to match fully qualified email addresses. " _ + "Use <tt>.*</tt> to accept any email address following the " _ + "<a href='http://tools.ietf.org/html/rfc2822#section-3.4.1'>RFC 2822</a> " _ + "specification. Another popular value to put here is <tt>^[^@]+$</tt>, " _ + "which means 'local usernames, no @ allowed.'", emailregexpdesc => "This describes in English words what kinds of legal addresses " _ "are allowed by the <tt>emailregexp</tt> param.", diff --git a/template/en/default/global/code-error.html.tmpl b/template/en/default/global/code-error.html.tmpl index 506dca582..24d0392ca 100644 --- a/template/en/default/global/code-error.html.tmpl +++ b/template/en/default/global/code-error.html.tmpl @@ -36,8 +36,7 @@ [% ELSE %] [%+ Param('emailregexpdesc') FILTER html_light %] [% END %] - It must also not contain any of these special characters: - <tt>\ ( ) & < > , ; : " [ ]</tt>, or any whitespace. + It also must not contain any illegal characters. [% ELSIF error == "authres_unhandled" %] The result value of [% value FILTER html %] was not handled by diff --git a/template/en/default/global/user-error.html.tmpl b/template/en/default/global/user-error.html.tmpl index 1585003ec..8dbaa4ad4 100644 --- a/template/en/default/global/user-error.html.tmpl +++ b/template/en/default/global/user-error.html.tmpl @@ -841,9 +841,8 @@ [% ELSE %] [%+ Param('emailregexpdesc') FILTER html_light %] [% END %] - It must also not contain any of these special characters: - <tt>\ ( ) & < > , ; : " [ ]</tt>, or any whitespace. - + It also must not contain any illegal characters. + [% ELSIF error == "illegal_frequency" %] [% title = "Too Frequent" %] Unless you are an administrator, you may not create series which are @@ -117,9 +117,7 @@ sub requestChangePassword { my $login_name = $cgi->param('loginname') or ThrowUserError("login_needed_for_password_change"); - validate_email_syntax($login_name) - || ThrowUserError('illegal_email_address', {addr => $login_name}); - + check_email_syntax($login_name); my $user = Bugzilla::User->check($login_name); # Make sure the user account is active. diff --git a/userprefs.cgi b/userprefs.cgi index ac323c65e..e527b1489 100755 --- a/userprefs.cgi +++ b/userprefs.cgi @@ -111,8 +111,7 @@ sub SaveAccount { } # Before changing an email address, confirm one does not exist. - validate_email_syntax($new_login_name) - || ThrowUserError('illegal_email_address', {addr => $new_login_name}); + check_email_syntax($new_login_name); is_available_username($new_login_name) || ThrowUserError("account_exists", {email => $new_login_name}); |