diff options
| -rw-r--r-- | Bugzilla/Flag.pm | 50 | ||||
| -rw-r--r-- | Bugzilla/FlagType.pm | 14 | ||||
| -rw-r--r-- | Bugzilla/User.pm | 58 | ||||
| -rwxr-xr-x | attachment.cgi | 22 | ||||
| -rwxr-xr-x | editwhines.cgi | 2 | ||||
| -rwxr-xr-x | post_bug.cgi | 2 | ||||
| -rwxr-xr-x | process_bug.cgi | 9 | ||||
| -rw-r--r-- | template/en/default/global/confirm-user-match.html.tmpl | 2 | ||||
| -rw-r--r-- | template/en/default/global/hidden-fields.html.tmpl | 23 |
9 files changed, 93 insertions, 89 deletions
diff --git a/Bugzilla/Flag.pm b/Bugzilla/Flag.pm index 961957278..0aca49c87 100644 --- a/Bugzilla/Flag.pm +++ b/Bugzilla/Flag.pm @@ -228,7 +228,7 @@ sub count { =over -=item C<validate($data, $bug_id)> +=item C<validate($cgi, $bug_id)> Validates fields containing flag modifications. @@ -238,17 +238,17 @@ Validates fields containing flag modifications. sub validate { my $user = Bugzilla->user; - my ($data, $bug_id) = @_; + my ($cgi, $bug_id) = @_; # Get a list of flags to validate. Uses the "map" function # to extract flag IDs from form field names by matching fields # whose name looks like "flag-nnn", where "nnn" is the ID, # and returning just the ID portion of matching field names. - my @ids = map(/^flag-(\d+)$/ ? $1 : (), keys %$data); + my @ids = map(/^flag-(\d+)$/ ? $1 : (), $cgi->param()); foreach my $id (@ids) { - my $status = $data->{"flag-$id"}; + my $status = $cgi->param("flag-$id"); # Make sure the flag exists. my $flag = get($id); @@ -277,9 +277,9 @@ sub validate { # feature and the attachment is marked private). if ($status eq '?' && $flag->{type}->{is_requesteeble} - && trim($data->{"requestee-$id"})) + && trim($cgi->param("requestee-$id"))) { - my $requestee_email = trim($data->{"requestee-$id"}); + my $requestee_email = trim($cgi->param("requestee-$id")); if ($requestee_email ne $flag->{'requestee'}->{'email'}) { # We know the requestee exists because we ran # Bugzilla::User::match_field before getting here. @@ -299,7 +299,7 @@ sub validate { # Throw an error if the target is a private attachment and # the requestee isn't in the group of insiders who can see it. if ($flag->{target}->{attachment}->{exists} - && $data->{'isprivate'} + && $cgi->param('isprivate') && Param("insidergroup") && !$requestee->in_group(Param("insidergroup"))) { @@ -353,21 +353,21 @@ sub snapshot { =over -=item C<process($target, $timestamp, $data)> +=item C<process($target, $timestamp, $cgi)> Processes changes to flags. The target is the bug or attachment this flag is about, the timestamp is the date/time the bug was last touched (so that changes to the flag -can be stamped with the same date/time), the data is the form data -with flag fields that the user submitted. +can be stamped with the same date/time), the cgi is the CGI object +used to obtain the flag fields that the user submitted. =back =cut sub process { - my ($target, $timestamp, $data) = @_; + my ($target, $timestamp, $cgi) = @_; my $dbh = Bugzilla->dbh; my $bug_id = $target->{'bug'}->{'id'}; @@ -381,14 +381,14 @@ sub process { my @old_summaries = snapshot($bug_id, $attach_id); # Cancel pending requests if we are obsoleting an attachment. - if ($attach_id && $data->{'isobsolete'}) { + if ($attach_id && $cgi->param('isobsolete')) { CancelRequests($bug_id, $attach_id); } # Create new flags and update existing flags. - my $new_flags = FormToNewFlags($target, $data); + my $new_flags = FormToNewFlags($target, $cgi); foreach my $flag (@$new_flags) { create($flag, $timestamp) } - modify($data, $timestamp); + modify($cgi, $timestamp); # In case the bug's product/component has changed, clear flags that are # no longer valid. @@ -521,7 +521,7 @@ sub migrate { =over -=item C<modify($data, $timestamp)> +=item C<modify($cgi, $timestamp)> Modifies flags in the database when a user changes them. Note that modified flags are always set active (is_active = 1) - @@ -533,14 +533,14 @@ attachment.cgi midairs. See bug 223878 for details. =cut sub modify { - my ($data, $timestamp) = @_; + my ($cgi, $timestamp) = @_; # Use the date/time we were given if possible (allowing calling code # to synchronize the comment's timestamp with those of other records). $timestamp = ($timestamp ? &::SqlQuote($timestamp) : "NOW()"); # Extract a list of flags from the form data. - my @ids = map(/^flag-(\d+)$/ ? $1 : (), keys %$data); + my @ids = map(/^flag-(\d+)$/ ? $1 : (), $cgi->param()); # Loop over flags and update their record in the database if necessary. # Two kinds of changes can happen to a flag: it can be set to a different @@ -550,8 +550,8 @@ sub modify { foreach my $id (@ids) { my $flag = get($id); - my $status = $data->{"flag-$id"}; - my $requestee_email = trim($data->{"requestee-$id"}); + my $status = $cgi->param("flag-$id"); + my $requestee_email = trim($cgi->param("requestee-$id")); # Ignore flags the user didn't change. There are two components here: @@ -672,7 +672,7 @@ sub clear { =over -=item C<FormToNewFlags($target, $data) +=item C<FormToNewFlags($target, $cgi) Someone pleasedocument this function. @@ -681,20 +681,20 @@ Someone pleasedocument this function. =cut sub FormToNewFlags { - my ($target, $data) = @_; + my ($target, $cgi) = @_; # Get information about the setter to add to each flag. # Uses a conditional to suppress Perl's "used only once" warnings. my $setter = new Bugzilla::User($::userid); # Extract a list of flag type IDs from field names. - my @type_ids = map(/^flag_type-(\d+)$/ ? $1 : (), keys %$data); - @type_ids = grep($data->{"flag_type-$_"} ne 'X', @type_ids); + my @type_ids = map(/^flag_type-(\d+)$/ ? $1 : (), $cgi->param()); + @type_ids = grep($cgi->param("flag_type-$_") ne 'X', @type_ids); # Process the form data and create an array of flag objects. my @flags; foreach my $type_id (@type_ids) { - my $status = $data->{"flag_type-$type_id"}; + my $status = $cgi->param("flag_type-$type_id"); &::trick_taint($status); # Create the flag record and populate it with data from the form. @@ -706,7 +706,7 @@ sub FormToNewFlags { }; if ($status eq "?") { - my $requestee = $data->{"requestee_type-$type_id"}; + my $requestee = $cgi->param("requestee_type-$type_id"); if ($requestee) { my $requestee_id = login_to_id($requestee); $flag->{'requestee'} = new Bugzilla::User($requestee_id); diff --git a/Bugzilla/FlagType.pm b/Bugzilla/FlagType.pm index 084777b29..da0c43281 100644 --- a/Bugzilla/FlagType.pm +++ b/Bugzilla/FlagType.pm @@ -303,7 +303,7 @@ sub count { =over -=item C<validate($data, $bug_id, $attach_id)> +=item C<validate($cgi, $bug_id, $attach_id)> Get a list of flag types to validate. Uses the "map" function to extract flag type IDs from form field names by matching columns @@ -316,13 +316,13 @@ and returning just the ID portion of matching field names. sub validate { my $user = Bugzilla->user; - my ($data, $bug_id, $attach_id) = @_; + my ($cgi, $bug_id, $attach_id) = @_; - my @ids = map(/^flag_type-(\d+)$/ ? $1 : (), keys %$data); + my @ids = map(/^flag_type-(\d+)$/ ? $1 : (), $cgi->param()); foreach my $id (@ids) { - my $status = $data->{"flag_type-$id"}; + my $status = $cgi->param("flag_type-$id"); # Don't bother validating types the user didn't touch. next if $status eq "X"; @@ -348,9 +348,9 @@ sub validate { # feature and the attachment is marked private). if ($status eq '?' && $flag_type->{is_requesteeble} - && trim($data->{"requestee_type-$id"})) + && trim($cgi->param("requestee_type-$id"))) { - my $requestee_email = trim($data->{"requestee_type-$id"}); + my $requestee_email = trim($cgi->param("requestee_type-$id")); # We know the requestee exists because we ran # Bugzilla::User::match_field before getting here. @@ -370,7 +370,7 @@ sub validate { # the requestee isn't in the group of insiders who can see it. if ($attach_id && Param("insidergroup") - && $data->{'isprivate'} + && $cgi->param('isprivate') && !$requestee->in_group(Param("insidergroup"))) { ThrowUserError("flag_requestee_unauthorized_attachment", diff --git a/Bugzilla/User.pm b/Bugzilla/User.pm index 2096db407..8cb4d5f99 100644 --- a/Bugzilla/User.pm +++ b/Bugzilla/User.pm @@ -687,7 +687,8 @@ sub match { # Here's what it does: # # 1. Accepts a list of fields along with whether they may take multiple values -# 2. Takes the values of those fields from $::FORM and passes them to match() +# 2. Takes the values of those fields from the first parameter, a $cgi object +# and passes them to match() # 3. Checks the results of the match and displays confirmation or failure # messages as appropriate. # @@ -710,7 +711,7 @@ sub match { # How to call it: # -# Bugzilla::User::match_field ({ +# Bugzilla::User::match_field($cgi, { # 'field_name' => { 'type' => fieldtype }, # 'field_name2' => { 'type' => fieldtype }, # [...] @@ -720,8 +721,8 @@ sub match { # sub match_field { - - my $fields = shift; # arguments as a hash + my $cgi = shift; # CGI object to look up fields in + my $fields = shift; # arguments as a hash my $matches = {}; # the values sent to the template my $matchsuccess = 1; # did the match fail? my $need_confirm = 0; # whether to display confirmation screen @@ -729,11 +730,9 @@ sub match_field { # prepare default form values my $vars = $::vars; - $vars->{'form'} = \%::FORM; - $vars->{'mform'} = \%::MFORM; # What does a "--do_not_change--" field look like (if any)? - my $dontchange = $vars->{'form'}->{'dontchange'}; + my $dontchange = $cgi->param('dontchange'); # Fields can be regular expressions matching multiple form fields # (f.e. "requestee-(\d+)"), so expand each non-literal field @@ -747,7 +746,7 @@ sub match_field { $expanded_fields->{$field_pattern} = $fields->{$field_pattern}; } else { - my @field_names = grep(/$field_pattern/, keys %{$vars->{'form'}}); + my @field_names = grep(/$field_pattern/, $cgi->param()); foreach my $field_name (@field_names) { $expanded_fields->{$field_name} = { type => $fields->{$field_pattern}->{'type'} }; @@ -774,23 +773,27 @@ sub match_field { # Tolerate fields that do not exist. # # This is so that fields like qa_contact can be specified in the code - # and it won't break if $::MFORM does not define them. + # and it won't break if the CGI object does not know about them. # # It has the side-effect that if a bad field name is passed it will be # quietly ignored rather than raising a code error. - next if !defined($vars->{'mform'}->{$field}); + next if !defined $cgi->param($field); # Skip it if this is a --do_not_change-- field - next if $dontchange && $dontchange eq $vars->{'form'}->{$field}; + next if $dontchange && $dontchange eq $cgi->param($field); # We need to move the query to $raw_field, where it will be split up, - # modified by the search, and put back into $::FORM and $::MFORM + # modified by the search, and put back into the CGI environment # incrementally. - my $raw_field = join(" ", @{$vars->{'mform'}->{$field}}); - $vars->{'form'}->{$field} = ''; - $vars->{'mform'}->{$field} = []; + my $raw_field = join(" ", $cgi->param($field)); + + # When we add back in values later, it matters that we delete + # the param here, and not set it to '', so that we will add + # things to an empty list, and not to a list containing one + # empty string + $cgi->delete($field); my @queries = (); @@ -835,12 +838,12 @@ sub match_field { if ((scalar(@{$users}) == 1) && (@{$users}[0]->{'login'} eq $query)) { - # delimit with spaces if necessary - if ($vars->{'form'}->{$field}) { - $vars->{'form'}->{$field} .= " "; - } - $vars->{'form'}->{$field} .= @{$users}[0]->{'login'}; - push @{$vars->{'mform'}->{$field}}, @{$users}[0]->{'login'}; + $cgi->append(-name=>$field, + -values=>[@{$users}[0]->{'login'}]); + + # XXX FORM compatilibity code, will be removed in bug 225818 + $::FORM{$field} = join(" ", $cgi->param($field)); + next; } @@ -850,12 +853,13 @@ sub match_field { # here is where it checks for multiple matches if (scalar(@{$users}) == 1) { # exactly one match - # delimit with spaces if necessary - if ($vars->{'form'}->{$field}) { - $vars->{'form'}->{$field} .= " "; - } - $vars->{'form'}->{$field} .= @{$users}[0]->{'login'}; - push @{$vars->{'mform'}->{$field}}, @{$users}[0]->{'login'}; + + $cgi->append(-name=>$field, + -values=>[@{$users}[0]->{'login'}]); + + # XXX FORM compatilibity code, will be removed in bug 225818 + $::FORM{$field} = join(" ", $cgi->param($field)); + $need_confirm = 1 if &::Param('confirmuniqueusermatch'); } diff --git a/attachment.cgi b/attachment.cgi index df25b768d..9f4b603dc 100755 --- a/attachment.cgi +++ b/attachment.cgi @@ -126,10 +126,11 @@ elsif ($action eq "insert") # The order of these function calls is important, as both Flag::validate # and FlagType::validate assume User::match_field has ensured that the values # in the requestee fields are legitimate user email addresses. - Bugzilla::User::match_field({ '^requestee(_type)?-(\d+)$' => - { 'type' => 'single' } }); - Bugzilla::Flag::validate(\%::FORM, $bugid); - Bugzilla::FlagType::validate(\%::FORM, $bugid, $::FORM{'id'}); + Bugzilla::User::match_field($cgi, { + '^requestee(_type)?-(\d+)$' => { 'type' => 'single' } + }); + Bugzilla::Flag::validate($cgi, $bugid); + Bugzilla::FlagType::validate($cgi, $bugid, $::FORM{'id'}); insert($data); } @@ -155,10 +156,11 @@ elsif ($action eq "update") # The order of these function calls is important, as both Flag::validate # and FlagType::validate assume User::match_field has ensured that the values # in the requestee fields are legitimate user email addresses. - Bugzilla::User::match_field({ '^requestee(_type)?-(\d+)$' => - { 'type' => 'single' } }); - Bugzilla::Flag::validate(\%::FORM, $bugid); - Bugzilla::FlagType::validate(\%::FORM, $bugid, $::FORM{'id'}); + Bugzilla::User::match_field($cgi, { + '^requestee(_type)?-(\d+)$' => { 'type' => 'single' } + }); + Bugzilla::Flag::validate($cgi, $bugid); + Bugzilla::FlagType::validate($cgi, $bugid, $::FORM{'id'}); update(); } @@ -1033,7 +1035,7 @@ sub insert # Create flags. my $target = Bugzilla::Flag::GetTarget(undef, $attachid); - Bugzilla::Flag::process($target, $timestamp, \%::FORM); + Bugzilla::Flag::process($target, $timestamp, $cgi); # Define the variables and functions that will be passed to the UI template. $vars->{'mailrecipients'} = { 'changer' => Bugzilla->user->login, @@ -1168,7 +1170,7 @@ sub update # is obsoleting this attachment without deleting any requests # the user submits at the same time. my $target = Bugzilla::Flag::GetTarget(undef, $::FORM{'id'}); - Bugzilla::Flag::process($target, $timestamp, \%::FORM); + Bugzilla::Flag::process($target, $timestamp, $cgi); # Update the attachment record in the database. SendSQL("UPDATE attachments diff --git a/editwhines.cgi b/editwhines.cgi index 013ee0df4..e65a585d0 100755 --- a/editwhines.cgi +++ b/editwhines.cgi @@ -198,7 +198,7 @@ if ($cgi->param('update')) { } } if (scalar %{$arglist}) { - &Bugzilla::User::match_field($arglist); + &Bugzilla::User::match_field($cgi, $arglist); } for my $sid (@scheduleids) { diff --git a/post_bug.cgi b/post_bug.cgi index 7f05dc7bc..84a9fd9df 100755 --- a/post_bug.cgi +++ b/post_bug.cgi @@ -60,7 +60,7 @@ my $dbh = Bugzilla->dbh; # do a match on the fields if applicable -&Bugzilla::User::match_field ({ +&Bugzilla::User::match_field ($cgi, { 'cc' => { 'type' => 'multi' }, 'assigned_to' => { 'type' => 'single' }, 'qa_contact' => { 'type' => 'single' }, diff --git a/process_bug.cgi b/process_bug.cgi index 94d86c936..aff3698bd 100755 --- a/process_bug.cgi +++ b/process_bug.cgi @@ -44,6 +44,7 @@ use Bugzilla::Util; # Use the Flag module to modify flag data if the user set flags. use Bugzilla::Flag; +use Bugzilla::FlagType; # Shut up misguided -w warnings about "used only once": @@ -138,7 +139,7 @@ foreach my $field ("dependson", "blocked") { # The order of these function calls is important, as both Flag::validate # and FlagType::validate assume User::match_field has ensured that the values # in the requestee fields are legitimate user email addresses. -&Bugzilla::User::match_field({ +&Bugzilla::User::match_field($cgi, { 'qa_contact' => { 'type' => 'single' }, 'newcc' => { 'type' => 'multi' }, 'masscc' => { 'type' => 'multi' }, @@ -148,8 +149,8 @@ foreach my $field ("dependson", "blocked") { # Validate flags, but only if the user is changing a single bug, # since the multi-change form doesn't include flag changes. if (defined $::FORM{'id'}) { - Bugzilla::Flag::validate(\%::FORM, $::FORM{'id'}); - Bugzilla::FlagType::validate(\%::FORM, $::FORM{'id'}); + Bugzilla::Flag::validate($cgi, $::FORM{'id'}); + Bugzilla::FlagType::validate($cgi, $::FORM{'id'}); } ###################################################################### @@ -1814,7 +1815,7 @@ foreach my $id (@idlist) { # Set and update flags. if ($UserInEditGroupSet) { my $target = Bugzilla::Flag::GetTarget($id); - Bugzilla::Flag::process($target, $timestamp, \%::FORM); + Bugzilla::Flag::process($target, $timestamp, $cgi); } if ($bug_changed) { SendSQL("UPDATE bugs SET delta_ts = $sql_timestamp WHERE bug_id = $id"); diff --git a/template/en/default/global/confirm-user-match.html.tmpl b/template/en/default/global/confirm-user-match.html.tmpl index 456af1fc2..fd09f89a6 100644 --- a/template/en/default/global/confirm-user-match.html.tmpl +++ b/template/en/default/global/confirm-user-match.html.tmpl @@ -21,8 +21,6 @@ #%] [%# INTERFACE: - # form: hash; the form values submitted to the script - # mform: hash; the form multi-values submitted to the script # fields: hash/record; the fields being matched, each of which has: # type: single|multi: whether or not the user can select multiple matches # flag_type: for flag requestee fields, the type of flag being requested diff --git a/template/en/default/global/hidden-fields.html.tmpl b/template/en/default/global/hidden-fields.html.tmpl index 2fa577b43..7327e43cf 100644 --- a/template/en/default/global/hidden-fields.html.tmpl +++ b/template/en/default/global/hidden-fields.html.tmpl @@ -20,23 +20,22 @@ #%] [%# INTERFACE: - # form: hash; the form fields/values for which to generate hidden fields. - # mform: hash; the form fields/values with multiple values for which to - # generate hidden fields. # exclude: string; a regular expression matching fields to exclude # from the list of hidden fields generated by this template #%] +[%# The global Bugzilla->cgi object is used to obtain form variable values. %] +[% USE Bugzilla %] +[% cgi = Bugzilla.cgi %] + [%# Generate hidden form fields for non-excluded fields. %] -[% FOREACH field = form %] - [% NEXT IF exclude && field.key.search(exclude) %] - [% IF mform.${field.key}.size > 1 %] - [% FOREACH mvalue = mform.${field.key} %] - <input type="hidden" name="[% field.key FILTER html %]" +[% FOREACH field = cgi.param() %] + [% NEXT IF exclude && field.search(exclude) %] + [%# The '.slice(0)' bit is here to force the 'param(field)' to be evaluated + in a list context, so we can avoid extra code checking for single valued or + empty fields %] + [% FOREACH mvalue = cgi.param(field).slice(0) %] + <input type="hidden" name="[% field FILTER html %]" value="[% mvalue FILTER html FILTER html_linebreak %]"> [% END %] - [% ELSE %] - <input type="hidden" name="[% field.key FILTER html %]" - value="[% field.value FILTER html FILTER html_linebreak %]"> - [% END %] [% END %] |