merged with bugzilla/4.2

13 files changed, 76 insertions, 64 deletions

diff --git a/Bugzilla/Attachment.pm b/Bugzilla/Attachment.pm
index b80228e78..aa7eee2a7 100644
--- a/Bugzilla/Attachment.pm
+++ b/Bugzilla/Attachment.pm
@@ -771,11 +771,8 @@ sub validate_obsolete {
         $attachment->validate_can_edit($bug->product_id)
           || ThrowUserError('illegal_attachment_edit', { attach_id => $attachment->id });
 
-        $vars->{'description'} = $attachment->description;
-
         if ($attachment->bug_id != $bug->bug_id) {
             $vars->{'my_bug_id'} = $bug->bug_id;
-            $vars->{'attach_bug_id'} = $attachment->bug_id;
             ThrowCodeError('mismatched_bug_ids_on_obsolete', $vars);
         }
 
diff --git a/Bugzilla/Bug.pm b/Bugzilla/Bug.pm
index 68ba98549..f8566be4a 100644
--- a/Bugzilla/Bug.pm
+++ b/Bugzilla/Bug.pm
@@ -523,17 +523,14 @@ sub possible_duplicates {
     if ($dbh->FULLTEXT_OR) {
         my $joined_terms = join($dbh->FULLTEXT_OR, @words);
         ($where_sql, $relevance_sql) = 
-            $dbh->sql_fulltext_search('bugs_fulltext.short_desc', 
-                                      $joined_terms, 1);
+            $dbh->sql_fulltext_search('bugs_fulltext.short_desc', $joined_terms);
         $relevance_sql ||= $where_sql;
     }
     else {
         my (@where, @relevance);
-        my $count = 0;
         foreach my $word (@words) {
-            $count++;
             my ($term, $rel_term) = $dbh->sql_fulltext_search(
-                'bugs_fulltext.short_desc', $word, $count);
+                'bugs_fulltext.short_desc', $word);
             push(@where, $term);
             push(@relevance, $rel_term || $term);
         }
diff --git a/Bugzilla/Constants.pm b/Bugzilla/Constants.pm
index aba988c18..0658244a1 100644
--- a/Bugzilla/Constants.pm
+++ b/Bugzilla/Constants.pm
@@ -202,7 +202,7 @@ use Memoize;
 # CONSTANTS
 #
 # Bugzilla version
-use constant BUGZILLA_VERSION => "4.2.3+";
+use constant BUGZILLA_VERSION => "4.2.4+";
 
 # Location of the remote and local XML files to track new releases.
 use constant REMOTE_FILE => 'http://updates.bugzilla.org/bugzilla-update.xml';
diff --git a/Bugzilla/DB/Oracle.pm b/Bugzilla/DB/Oracle.pm
index da263e084..ebf59533f 100644
--- a/Bugzilla/DB/Oracle.pm
+++ b/Bugzilla/DB/Oracle.pm
@@ -56,6 +56,8 @@ use constant BLOB_TYPE => { ora_type => ORA_BLOB };
 use constant MIN_LONG_READ_LEN => 32 * 1024;
 use constant FULLTEXT_OR => ' OR ';
 
+our $fulltext_label = 0;
+
 sub new {
     my ($class, $params) = @_;
     my ($user, $pass, $host, $dbname, $port) = 
@@ -124,7 +126,8 @@ sub bz_explain {
 sub sql_group_concat {
     my ($self, $text, $separator) = @_;
     $separator = $self->quote(', ') if !defined $separator;
-    return "group_concat(T_CLOB_DELIM($text, $separator))";
+    my ($distinct, $rest) = $text =~/^(\s*DISTINCT\s|)(.+)$/i;
+    return "group_concat($distinct T_CLOB_DELIM(NVL($rest, ' '), $separator))";
 }
 
 sub sql_regexp {
@@ -170,11 +173,13 @@ sub sql_from_days{
 
     return " TO_DATE($date,'J') ";
 }
+
 sub sql_fulltext_search {
-    my ($self, $column, $text, $label) = @_;
+    my ($self, $column, $text) = @_;
     $text = $self->quote($text);
     trick_taint($text);
-    return "CONTAINS($column,$text,$label) > 0", "SCORE($label)";
+    $fulltext_label++;
+    return "CONTAINS($column,$text,$fulltext_label) > 0", "SCORE($fulltext_label)";
 }
 
 sub sql_date_format {
@@ -545,14 +550,17 @@ sub bz_setup_database {
               . " RETURN NUMBER IS BEGIN RETURN LENGTH(COLUMN_NAME); END;");
     
     # Create types for group_concat
-    my $t_clob_delim = $self->selectcol_arrayref("
-        SELECT TYPE_NAME FROM USER_TYPES WHERE TYPE_NAME=?",
-        undef, 'T_CLOB_DELIM'); 
-
-    if ( !@$t_clob_delim ) {
-        $self->do("CREATE OR REPLACE TYPE T_CLOB_DELIM AS OBJECT "
-              . "( p_CONTENT CLOB, p_DELIMITER VARCHAR2(256));");
-    }
+    $self->do("DROP TYPE T_GROUP_CONCAT");
+    $self->do("CREATE OR REPLACE TYPE T_CLOB_DELIM AS OBJECT "
+          . "( p_CONTENT CLOB, p_DELIMITER VARCHAR2(256)"
+          . ", MAP MEMBER FUNCTION T_CLOB_DELIM_ToVarchar return VARCHAR2"
+          . ");");
+    $self->do("CREATE OR REPLACE TYPE BODY T_CLOB_DELIM IS
+                  MAP MEMBER FUNCTION T_CLOB_DELIM_ToVarchar return VARCHAR2 is
+                  BEGIN
+                      RETURN p_CONTENT;
+                  END;
+              END;");
 
     $self->do("CREATE OR REPLACE TYPE T_GROUP_CONCAT AS OBJECT 
                (  CLOB_CONTENT CLOB,
diff --git a/Bugzilla/DB/Pg.pm b/Bugzilla/DB/Pg.pm
index b6be64011..4f818932b 100644
--- a/Bugzilla/DB/Pg.pm
+++ b/Bugzilla/DB/Pg.pm
@@ -215,11 +215,12 @@ sub bz_check_server_version {
     my $self = shift;
     my ($db) = @_;
     my $server_version = $self->SUPER::bz_check_server_version(@_);
-    my ($major_version) = $server_version =~ /^(\d+)/;
-    # Pg 9 requires DBD::Pg 2.17.2 in order to properly read bytea values.
+    my ($major_version, $minor_version) = $server_version =~ /^0*(\d+)\.0*(\d+)/;
+    # Pg 9.0 requires DBD::Pg 2.17.2 in order to properly read bytea values.
+    # Pg 9.2 requires DBD::Pg 2.19.3 as spclocation no longer exists.
     if ($major_version >= 9) {
-        local $db->{dbd}->{version} = '2.17.2';
-        local $db->{name} = $db->{name} . ' 9+';
+        local $db->{dbd}->{version} = ($minor_version >= 2) ? '2.19.3' : '2.17.2';
+        local $db->{name} = $db->{name} . " ${major_version}.$minor_version";
         Bugzilla::DB::_bz_check_dbd(@_);
     }
 }
diff --git a/Bugzilla/Field.pm b/Bugzilla/Field.pm
index 5442c6401..8ebf08672 100644
--- a/Bugzilla/Field.pm
+++ b/Bugzilla/Field.pm
@@ -1025,11 +1025,6 @@ sub create {
     my ($params) = @_;
     my $dbh = Bugzilla->dbh;
 
-    # This makes sure the "sortkey" validator runs, even if
-    # the parameter isn't sent to create().
-    $params->{sortkey} = undef if !exists $params->{sortkey};
-    $params->{type} ||= 0;
-
     # BMO: disable bug updates during field creation
     # using an eval as try/finally
     my $field;
@@ -1039,32 +1034,27 @@ sub create {
             write_params();
         }
 
-        # Purpose: if the field is active in the fields list before all of the 
-        # data structures are created, anything accessing Bug.pm will crash. So
-        # stash a copy of the intended obsolete value for later and force it
-        # to be obsolete on initial creation.
-        # Upstreaming: https://bugzilla.mozilla.org/show_bug.cgi?id=531243
-        my $original_obsolete;
-        if ($params->{'custom'}) {
-            $original_obsolete = $params->{'obsolete'};
-            $params->{'obsolete'} = 1;
-        }
+        # This makes sure the "sortkey" validator runs, even if
+        # the parameter isn't sent to create().
+        $params->{sortkey} = undef if !exists $params->{sortkey};
+        $params->{type} ||= 0;
+        # We mark the custom field as obsolete till it has been fully created,
+        # to avoid race conditions when viewing bugs at the same time.
+        my $is_obsolete = $params->{obsolete};
+        $params->{obsolete} = 1 if $params->{custom};
 
         $dbh->bz_start_transaction();
         $class->check_required_create_fields(@_);
         my $field_values      = $class->run_create_validators($params);
         my $visibility_values = delete $field_values->{visibility_values};
         my $field             = $class->insert_create_data($field_values);
-    
+
         $field->set_visibility_values($visibility_values);
         $field->_update_visibility_values();
 
         $dbh->bz_commit_transaction();
 
         if ($field->custom) {
-            # Restore the obsolete value that got stashed earlier (in memory)
-            $field->set_obsolete($original_obsolete);
-
             my $name = $field->name;
             my $type = $field->type;
             if (SQL_DEFINITIONS->{$type}) {
@@ -1082,8 +1072,9 @@ sub create {
                 $dbh->do("INSERT INTO $name (value) VALUES ('---')");
             }
 
-            # Safe to write the original 'obsolete' value to the database now
-            $field->update;
+            # Restore the original obsolete state of the custom field.
+            $dbh->do('UPDATE fielddefs SET obsolete = 0 WHERE id = ?', undef, $field->id)
+              unless $is_obsolete;
         }
     };
 
diff --git a/Bugzilla/FlagType.pm b/Bugzilla/FlagType.pm
index 5fc00e137..617ea54b7 100644
--- a/Bugzilla/FlagType.pm
+++ b/Bugzilla/FlagType.pm
@@ -95,7 +95,7 @@ use constant VALIDATORS => {
     description      => \&_check_description,
     cc_list          => \&_check_cc_list,
     target_type      => \&_check_target_type,
-    sortkey          => \&_check_sortey,
+    sortkey          => \&_check_sortkey,
     is_active        => \&Bugzilla::Object::check_boolean,
     is_requestable   => \&Bugzilla::Object::check_boolean,
     is_requesteeble  => \&Bugzilla::Object::check_boolean,
@@ -325,7 +325,7 @@ sub _check_target_type {
     return $target_type;
 }
 
-sub _check_sortey {
+sub _check_sortkey {
     my ($invocant, $sortkey) = @_;
 
     (detaint_natural($sortkey) && $sortkey <= MAX_SMALLINT)
diff --git a/Bugzilla/Object.pm b/Bugzilla/Object.pm
index 4db37f72f..8089c6ccc 100644
--- a/Bugzilla/Object.pm
+++ b/Bugzilla/Object.pm
@@ -809,7 +809,7 @@ your own C<DB_COLUMNS> subroutine in a subclass.)
 The name of the column that should be considered to be the unique
 "name" of this object. The 'name' is a B<string> that uniquely identifies
 this Object in the database. Defaults to 'name'. When you specify 
-C<{name => $name}> to C<new()>, this is the column that will be 
+C<< {name => $name} >> to C<new()>, this is the column that will be 
 matched against in the DB.
 
 =item C<ID_FIELD>
@@ -972,7 +972,7 @@ for each placeholder in C<condition>, in order.
 
 This is to allow subclasses to have complex parameters, and then to
 translate those parameters into C<condition> and C<values> when they
-call C<$self->SUPER::new> (which is this function, usually).
+call C<< $self->SUPER::new >> (which is this function, usually).
 
 If you try to call C<new> outside of a subclass with the C<condition>
 and C<values> parameters, Bugzilla will throw an error. These parameters
@@ -1097,8 +1097,9 @@ Notes:       In order for this function to work in your subclass,
              your subclass's L</ID_FIELD> must be of C<SERIAL>
              type in the database.
 
-             Subclass Implementors: This function basically just
-             calls L</check_required_create_fields>, then
+Subclass Implementors:
+             This function basically just calls 
+             L</check_required_create_fields>, then
              L</run_create_validators>, and then finally
              L</insert_create_data>. So if you have a complex system that
              you need to implement, you can do it by calling these
@@ -1291,9 +1292,9 @@ C<0> otherwise.
 
  Returns:     A list of objects, or an empty list if there are none.
 
- Notes:       Note that you must call this as C<$class->get_all>. For 
-              example, C<Bugzilla::Keyword->get_all>. 
-              C<Bugzilla::Keyword::get_all> will not work.
+ Notes:       Note that you must call this as $class->get_all. For 
+              example, Bugzilla::Keyword->get_all. 
+              Bugzilla::Keyword::get_all will not work.
 
 =back
 
diff --git a/Bugzilla/Search.pm b/Bugzilla/Search.pm
index c7c5038be..542b01045 100644
--- a/Bugzilla/Search.pm
+++ b/Bugzilla/Search.pm
@@ -2348,9 +2348,9 @@ sub _content_matches {
     
     # Create search terms to add to the SELECT and WHERE clauses.
     my ($term1, $rterm1) =
-        $dbh->sql_fulltext_search("$table.$comments_col", $value, 1);
+        $dbh->sql_fulltext_search("$table.$comments_col", $value);
     my ($term2, $rterm2) =
-        $dbh->sql_fulltext_search("$table.short_desc", $value, 2);
+        $dbh->sql_fulltext_search("$table.short_desc", $value);
     $rterm1 = $term1 if !$rterm1;
     $rterm2 = $term2 if !$rterm2;
 
diff --git a/Bugzilla/WebService/Constants.pm b/Bugzilla/WebService/Constants.pm
index 59aab9b55..6274c3a78 100644
--- a/Bugzilla/WebService/Constants.pm
+++ b/Bugzilla/WebService/Constants.pm
@@ -166,6 +166,7 @@ use constant WS_ERROR_CODE => {
     group_exists => 801,
     empty_group_description => 802,
     invalid_regexp => 803,
+    invalid_group_name => 804,
 
     # Errors thrown by the WebService itself. The ones that are negative 
     # conform to http://xmlrpc-epi.sourceforge.net/specs/rfc.fault_codes.php
diff --git a/Bugzilla/WebService/Server.pm b/Bugzilla/WebService/Server.pm
index feb80e9d0..206f0c657 100644
--- a/Bugzilla/WebService/Server.pm
+++ b/Bugzilla/WebService/Server.pm
@@ -25,7 +25,9 @@ use Scalar::Util qw(blessed);
 
 sub handle_login {
     my ($self, $class, $method, $full_method) = @_;
-    ThrowCodeError('unknown_method', {method => $full_method}) if !$class;
+    # Throw error if the supplied class does not exist or the method is private
+    ThrowCodeError('unknown_method', {method => $full_method}) if (!$class or $method =~ /^_/);
+
     eval "require $class";
     ThrowCodeError('unknown_method', {method => $full_method}) if $@;
     return if ($class->login_exempt($method) 
diff --git a/Bugzilla/WebService/User.pm b/Bugzilla/WebService/User.pm
index d9fc890f7..758c69aa8 100644
--- a/Bugzilla/WebService/User.pm
+++ b/Bugzilla/WebService/User.pm
@@ -242,12 +242,18 @@ sub _filter_users_by_group {
     # If no groups are specified, we return all users.
     return $users if (!$group_ids and !$group_names);
 
+    my $user = Bugzilla->user;
+
     my @groups = map { Bugzilla::Group->check({ id => $_ }) } 
                      @{ $group_ids || [] };
-    my @name_groups = map { Bugzilla::Group->check($_) } 
-                          @{ $group_names || [] };
-    push(@groups, @name_groups);
-    
+
+    if ($group_names) {
+        foreach my $name (@$group_names) {
+            my $group = Bugzilla::Group->check({ name => $name, _error => 'invalid_group_name' });
+            $user->in_group($group) || ThrowUserError('invalid_group_name', { name => $name });
+            push(@groups, $group);
+        }
+    }
 
     my @in_group = grep { $self->_user_in_any_group($_, \@groups) }
                         @$users;
@@ -679,10 +685,10 @@ based on your permission to bless each group.
 
 =over
 
-=item 51 (Bad Login Name or Group Name)
+=item 51 (Bad Login Name or Group ID)
 
 You passed an invalid login name in the "names" array or a bad
-group name/id in the C<groups>/C<group_ids> arguments.
+group ID in the C<group_ids> argument.
 
 =item 304 (Authorization Required)
 
@@ -694,6 +700,11 @@ wanted to get information about by user id.
 Logged-out users cannot use the "ids" or "match" arguments to this 
 function.
 
+=item 804 (Invalid Group Name)
+
+You passed a group name in the C<groups> argument which either does not
+exist or you do not belong to it.
+
 =back
 
 =item B<History>
@@ -711,6 +722,9 @@ for C<match> has changed to only returning enabled accounts.
 
 =item C<saved_searches> Added in Bugzilla B<4.4>.
 
+=item Error 804 has been added in Bugzilla 4.0.9 and 4.2.4. It's now
+illegal to pass a group name you don't belong to.
+
 =back
 
 =back
diff --git a/Bugzilla/WebService/Util.pm b/Bugzilla/WebService/Util.pm
index 6d3a37767..feefd47af 100644
--- a/Bugzilla/WebService/Util.pm
+++ b/Bugzilla/WebService/Util.pm
@@ -153,7 +153,7 @@ a hash to L</filter>, C<0> otherwise.
 
 =head2 validate
 
-This helps in the validation of parameters passed into the WebSerice
+This helps in the validation of parameters passed into the WebService
 methods. Currently it converts listed parameters into an array reference
 if the client only passed a single scalar value. It modifies the parameters
 hash in place so other parameters should be unaltered.