aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
-rw-r--r--Bugzilla/FlagType.pm14
-rw-r--r--Bugzilla/User.pm3
-rw-r--r--Bugzilla/Util.pm26
-rw-r--r--docs/en/xml/administration.xml10
-rw-r--r--t/007util.t12
-rw-r--r--template/en/default/admin/params/auth.html.tmpl11
-rw-r--r--template/en/default/global/code-error.html.tmpl3
-rw-r--r--template/en/default/global/user-error.html.tmpl5
-rwxr-xr-xtoken.cgi4
-rwxr-xr-xuserprefs.cgi3
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>\ ( ) &amp; &lt; &gt; , ; : &quot; [ ]</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>\ ( ) &amp; &lt; &gt; , ; : &quot; [ ]</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
diff --git a/token.cgi b/token.cgi
index 7c6b4e8d1..5f647edb3 100755
--- a/token.cgi
+++ b/token.cgi
@@ -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});