Bug 343809: Merge FlagType::validate() with Flag::validate() - Patch by Fr�d�ric Buclin <LpSolit@gmail.com> a=myk

This commit is contained in:
lpsolit%gmail.com 2006-08-24 15:56:39 +00:00
Родитель a67ad71bab
Коммит 5b99b331a5
6 изменённых файлов: 197 добавлений и 283 удалений

Просмотреть файл

@ -729,8 +729,8 @@ sub insert_attachment_for_bug {
$isurl = 0;
}
# The order of these function calls is important, as both Flag::validate
# and FlagType::validate assume User::match_field has ensured that the
# The order of these function calls is important, as Flag::validate
# assumes User::match_field has ensured that the
# values in the requestee fields are legitimate user email addresses.
my $match_status = Bugzilla::User::match_field($cgi, {
'^requestee(_type)?-(\d+)$' => { 'type' => 'multi' },
@ -744,13 +744,11 @@ sub insert_attachment_for_bug {
$$hr_vars->{'message'} = 'user_match_multiple';
}
# FlagType::validate() and Flag::validate() should not detect
# any reference to existing flags when creating a new attachment.
# Setting the third param to -1 will force this function to check this
# point.
# Flag::validate() should not detect any reference to existing flags
# when creating a new attachment. Setting the third param to -1 will
# force this function to check this point.
# XXX needs $throw_error treatment
Bugzilla::Flag::validate($cgi, $bug->bug_id, -1);
Bugzilla::FlagType::validate($cgi, $bug->bug_id, -1);
# Escape characters in strings that will be used in SQL statements.
my $description = $cgi->param('description');

Просмотреть файл

@ -96,6 +96,14 @@ Returns the ID of the flag.
Returns the name of the flagtype the flag belongs to.
=item C<bug_id>
Returns the ID of the bug this flag belongs to.
=item C<attach_id>
Returns the ID of the attachment this flag belongs to, if any.
=item C<status>
Returns the status '+', '-', '?' of the flag.
@ -106,6 +114,8 @@ Returns the status '+', '-', '?' of the flag.
sub id { return $_[0]->{'id'}; }
sub name { return $_[0]->type->name; }
sub bug_id { return $_[0]->{'bug_id'}; }
sub attach_id { return $_[0]->{'attach_id'}; }
sub status { return $_[0]->{'status'}; }
###############################
@ -235,154 +245,205 @@ to -1 to force its check anyway.
sub validate {
my ($cgi, $bug_id, $attach_id) = @_;
my $user = Bugzilla->user;
my $dbh = Bugzilla->dbh;
# 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 : (), $cgi->param());
# whose name looks like "flag_type-nnn" (new flags) or "flag-nnn"
# (existing flags), where "nnn" is the ID, and returning just
# the ID portion of matching field names.
my @flagtype_ids = map(/^flag_type-(\d+)$/ ? $1 : (), $cgi->param());
my @flag_ids = map(/^flag-(\d+)$/ ? $1 : (), $cgi->param());
return unless (scalar(@flagtype_ids) || scalar(@flag_ids));
return unless scalar(@ids);
# No flag reference should exist when changing several bugs at once.
ThrowCodeError("flags_not_available", { type => 'b' }) unless $bug_id;
# No reference to existing flags should exist when creating a new
# attachment.
if ($attach_id && ($attach_id < 0)) {
ThrowCodeError("flags_not_available", { type => 'a' });
# We don't check that these new flags are valid for this bug/attachment,
# because the bug may be moved into another product meanwhile.
# This check will be done later when creating new flags, see FormToNewFlags().
# All new flags must belong to active flag types.
if (scalar(@flagtype_ids)) {
my $inactive_flagtypes =
$dbh->selectrow_array('SELECT 1 FROM flagtypes
WHERE id IN (' . join(',', @flagtype_ids) . ')
AND is_active = 0 ' .
$dbh->sql_limit(1));
ThrowCodeError('flag_type_inactive') if $inactive_flagtypes;
}
# Make sure all flags belong to the bug/attachment they pretend to be.
my $field = ($attach_id) ? "attach_id" : "bug_id";
my $field_id = $attach_id || $bug_id;
my $not = ($attach_id) ? "" : "NOT";
if (scalar(@flag_ids)) {
# No reference to existing flags should exist when creating a new
# attachment.
if ($attach_id && ($attach_id < 0)) {
ThrowCodeError('flags_not_available', { type => 'a' });
}
my $invalid_data =
$dbh->selectrow_array("SELECT 1 FROM flags
WHERE id IN (" . join(',', @ids) . ")
AND ($field != ? OR attach_id IS $not NULL) " .
$dbh->sql_limit(1),
undef, $field_id);
# Make sure all existing flags belong to the bug/attachment
# they pretend to be.
my $field = ($attach_id) ? "attach_id" : "bug_id";
my $field_id = $attach_id || $bug_id;
my $not = ($attach_id) ? "" : "NOT";
if ($invalid_data) {
ThrowCodeError("invalid_flag_association",
{ bug_id => $bug_id,
attach_id => $attach_id });
my $invalid_data =
$dbh->selectrow_array("SELECT 1 FROM flags
WHERE id IN (" . join(',', @flag_ids) . ")
AND ($field != ? OR attach_id IS $not NULL) " .
$dbh->sql_limit(1),
undef, $field_id);
if ($invalid_data) {
ThrowCodeError('invalid_flag_association',
{ bug_id => $bug_id,
attach_id => $attach_id });
}
}
foreach my $id (@ids) {
# Validate new flags.
foreach my $id (@flagtype_ids) {
my $status = $cgi->param("flag_type-$id");
my @requestees = $cgi->param("requestee_type-$id");
my $private_attachment = $cgi->param('isprivate') ? 1 : 0;
# Don't bother validating types the user didn't touch.
next if $status eq 'X';
# Make sure the flag type exists.
my $flag_type = new Bugzilla::FlagType($id);
$flag_type || ThrowCodeError('flag_type_nonexistent', { id => $id });
_validate(undef, $flag_type, $status, \@requestees, $private_attachment,
$bug_id, $attach_id);
}
# Validate existing flags.
foreach my $id (@flag_ids) {
my $status = $cgi->param("flag-$id");
my @requestees = $cgi->param("requestee-$id");
my $private_attachment = $cgi->param('isprivate') ? 1 : 0;
# Make sure the flag exists.
my $flag = new Bugzilla::Flag($id);
$flag || ThrowCodeError("flag_nonexistent", { id => $id });
# Make sure the user chose a valid status.
grep($status eq $_, qw(X + - ?))
|| ThrowCodeError("flag_status_invalid",
{ id => $id, status => $status });
# Make sure the user didn't request the flag unless it's requestable.
# If the flag was requested before it became unrequestable, leave it
# as is.
if ($status eq '?'
&& $flag->status ne '?'
&& !$flag->type->is_requestable)
{
ThrowCodeError("flag_status_invalid",
{ id => $id, status => $status });
}
# Make sure the user didn't specify a requestee unless the flag
# is specifically requestable. If the requestee was set before
# the flag became specifically unrequestable, don't let the user
# change the requestee, but let the user remove it by entering
# an empty string for the requestee.
if ($status eq '?' && !$flag->type->is_requesteeble) {
my $old_requestee = $flag->requestee ? $flag->requestee->login : '';
my $new_requestee = join('', @requestees);
if ($new_requestee && $new_requestee ne $old_requestee) {
ThrowCodeError("flag_requestee_disabled",
{ type => $flag->type });
}
}
# Make sure the user didn't enter multiple requestees for a flag
# that can't be requested from more than one person at a time.
if ($status eq '?'
&& !$flag->type->is_multiplicable
&& scalar(@requestees) > 1)
{
ThrowUserError("flag_not_multiplicable", { type => $flag->type });
}
# Make sure the requestees are authorized to access the bug.
# (and attachment, if this installation is using the "insider group"
# feature and the attachment is marked private).
if ($status eq '?' && $flag->type->is_requesteeble) {
my $old_requestee = $flag->requestee ? $flag->requestee->login : '';
foreach my $login (@requestees) {
next if $login eq $old_requestee;
# We know the requestee exists because we ran
# Bugzilla::User::match_field before getting here.
my $requestee = new Bugzilla::User({ name => $login });
# Throw an error if the user can't see the bug.
# Note that if permissions on this bug are changed,
# can_see_bug() will refer to old settings.
if (!$requestee->can_see_bug($bug_id)) {
ThrowUserError("flag_requestee_unauthorized",
{ flag_type => $flag->type,
requestee => $requestee,
bug_id => $bug_id,
attach_id => $attach_id
});
}
# 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 ($attach_id
&& $cgi->param('isprivate')
&& Bugzilla->params->{"insidergroup"}
&& !$requestee->in_group(Bugzilla->params->{"insidergroup"}))
{
ThrowUserError("flag_requestee_unauthorized_attachment",
{ flag_type => $flag->type,
requestee => $requestee,
bug_id => $bug_id,
attach_id => $attach_id
});
}
}
}
# Make sure the user is authorized to modify flags, see bug 180879
# - The flag is unchanged
next if ($status eq $flag->status);
# - User in the request_group can clear pending requests and set flags
# and can rerequest set flags.
next if (($status eq 'X' || $status eq '?')
&& (!$flag->type->request_group
|| $user->in_group_id($flag->type->request_group->id)));
# - User in the grant_group can set/clear flags, including "+" and "-".
next if (!$flag->type->grant_group
|| $user->in_group_id($flag->type->grant_group->id));
# - Any other flag modification is denied
ThrowUserError("flag_update_denied",
{ name => $flag->type->name,
status => $status,
old_status => $flag->status });
_validate($flag, $flag->type, $status, \@requestees, $private_attachment);
}
}
sub _validate {
my ($flag, $flag_type, $status, $requestees, $private_attachment,
$bug_id, $attach_id) = @_;
my $user = Bugzilla->user;
my $id = $flag ? $flag->id : $flag_type->id; # Used in the error messages below.
$bug_id ||= $flag->bug_id;
$attach_id ||= $flag->attach_id if $flag; # Maybe it's a bug flag.
# Make sure the user chose a valid status.
grep($status eq $_, qw(X + - ?))
|| ThrowCodeError('flag_status_invalid',
{ id => $id, status => $status });
# Make sure the user didn't request the flag unless it's requestable.
# If the flag existed and was requested before it became unrequestable,
# leave it as is.
if ($status eq '?'
&& (!$flag || $flag->status ne '?')
&& !$flag_type->is_requestable)
{
ThrowCodeError('flag_status_invalid',
{ id => $id, status => $status });
}
# Make sure the user didn't specify a requestee unless the flag
# is specifically requestable. For existing flags, if the requestee
# was set before the flag became specifically unrequestable, don't
# let the user change the requestee, but let the user remove it by
# entering an empty string for the requestee.
if ($status eq '?' && !$flag_type->is_requesteeble) {
my $old_requestee = ($flag && $flag->requestee) ?
$flag->requestee->login : '';
my $new_requestee = join('', @$requestees);
if ($new_requestee && $new_requestee ne $old_requestee) {
ThrowCodeError('flag_requestee_disabled',
{ type => $flag_type });
}
}
# Make sure the user didn't enter multiple requestees for a flag
# that can't be requested from more than one person at a time.
if ($status eq '?'
&& !$flag_type->is_multiplicable
&& scalar(@$requestees) > 1)
{
ThrowUserError('flag_not_multiplicable', { type => $flag_type });
}
# Make sure the requestees are authorized to access the bug
# (and attachment, if this installation is using the "insider group"
# feature and the attachment is marked private).
if ($status eq '?' && $flag_type->is_requesteeble) {
my $old_requestee = ($flag && $flag->requestee) ?
$flag->requestee->login : '';
foreach my $login (@$requestees) {
next if $login eq $old_requestee;
# We know the requestee exists because we ran
# Bugzilla::User::match_field before getting here.
my $requestee = new Bugzilla::User({ name => $login });
# Throw an error if the user can't see the bug.
# Note that if permissions on this bug are changed,
# can_see_bug() will refer to old settings.
if (!$requestee->can_see_bug($bug_id)) {
ThrowUserError('flag_requestee_unauthorized',
{ flag_type => $flag_type,
requestee => $requestee,
bug_id => $bug_id,
attach_id => $attach_id });
}
# 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 ($attach_id
&& $private_attachment
&& Bugzilla->params->{'insidergroup'}
&& !$requestee->in_group(Bugzilla->params->{'insidergroup'}))
{
ThrowUserError('flag_requestee_unauthorized_attachment',
{ flag_type => $flag_type,
requestee => $requestee,
bug_id => $bug_id,
attach_id => $attach_id });
}
}
}
# Make sure the user is authorized to modify flags, see bug 180879
# - The flag exists and is unchanged.
return if ($flag && ($status eq $flag->status));
# - User in the request_group can clear pending requests and set flags
# and can rerequest set flags.
return if (($status eq 'X' || $status eq '?')
&& (!$flag_type->request_group
|| $user->in_group_id($flag_type->request_group->id)));
# - User in the grant_group can set/clear flags, including "+" and "-".
return if (!$flag_type->grant_group
|| $user->in_group_id($flag_type->grant_group->id));
# - Any other flag modification is denied
ThrowUserError('flag_update_denied',
{ name => $flag_type->name,
status => $status,
old_status => $flag ? $flag->status : 'X' });
}
sub snapshot {
my ($bug_id, $attach_id) = @_;

Просмотреть файл

@ -359,143 +359,6 @@ sub count {
return $count;
}
=pod
=over
=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
whose name looks like "flag_type-nnn", where "nnn" is the ID,
and returning just the ID portion of matching field names.
If the attachment is new, it has no ID yet and $attach_id is set
to -1 to force its check anyway.
=back
=cut
sub validate {
my ($cgi, $bug_id, $attach_id) = @_;
my $user = Bugzilla->user;
my $dbh = Bugzilla->dbh;
my @ids = map(/^flag_type-(\d+)$/ ? $1 : (), $cgi->param());
return unless scalar(@ids);
# No flag reference should exist when changing several bugs at once.
ThrowCodeError("flags_not_available", { type => 'b' }) unless $bug_id;
# We don't check that these flag types are valid for
# this bug/attachment. This check will be done later when
# processing new flags, see Flag::FormToNewFlags().
# All flag types have to be active
my $inactive_flagtypes =
$dbh->selectrow_array("SELECT 1 FROM flagtypes
WHERE id IN (" . join(',', @ids) . ")
AND is_active = 0 " .
$dbh->sql_limit(1));
ThrowCodeError("flag_type_inactive") if $inactive_flagtypes;
foreach my $id (@ids) {
my $status = $cgi->param("flag_type-$id");
my @requestees = $cgi->param("requestee_type-$id");
# Don't bother validating types the user didn't touch.
next if $status eq "X";
# Make sure the flag type exists.
my $flag_type = new Bugzilla::FlagType($id);
$flag_type
|| ThrowCodeError("flag_type_nonexistent", { id => $id });
# Make sure the value of the field is a valid status.
grep($status eq $_, qw(X + - ?))
|| ThrowCodeError("flag_status_invalid",
{ id => $id , status => $status });
# Make sure the user didn't request the flag unless it's requestable.
if ($status eq '?' && !$flag_type->is_requestable) {
ThrowCodeError("flag_status_invalid",
{ id => $id , status => $status });
}
# Make sure the user didn't specify a requestee unless the flag
# is specifically requestable.
if ($status eq '?'
&& !$flag_type->is_requesteeble
&& scalar(@requestees) > 0)
{
ThrowCodeError("flag_requestee_disabled", { type => $flag_type });
}
# Make sure the user didn't enter multiple requestees for a flag
# that can't be requested from more than one person at a time.
if ($status eq '?'
&& !$flag_type->is_multiplicable
&& scalar(@requestees) > 1)
{
ThrowUserError("flag_not_multiplicable", { type => $flag_type });
}
# Make sure the requestees are authorized to access the bug
# (and attachment, if this installation is using the "insider group"
# feature and the attachment is marked private).
if ($status eq '?' && $flag_type->is_requesteeble) {
foreach my $login (@requestees) {
# We know the requestee exists because we ran
# Bugzilla::User::match_field before getting here.
my $requestee = new Bugzilla::User({ name => $login });
# Throw an error if the user can't see the bug.
if (!$requestee->can_see_bug($bug_id)) {
ThrowUserError("flag_requestee_unauthorized",
{ flag_type => $flag_type,
requestee => $requestee,
bug_id => $bug_id,
attach_id => $attach_id });
}
# 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 ($attach_id
&& Bugzilla->params->{"insidergroup"}
&& $cgi->param('isprivate')
&& !$requestee->in_group(Bugzilla->params->{"insidergroup"}))
{
ThrowUserError("flag_requestee_unauthorized_attachment",
{ flag_type => $flag_type,
requestee => $requestee,
bug_id => $bug_id,
attach_id => $attach_id });
}
}
}
# Make sure the user is authorized to modify flags, see bug 180879
# - User in the grant_group can set flags, including "+" and "-".
next if (!$flag_type->grant_group
|| $user->in_group_id($flag_type->grant_group->id));
# - User in the request_group can request flags.
next if ($status eq '?'
&& (!$flag_type->request_group
|| $user->in_group_id($flag_type->request_group->id)));
# - Any other flag modification is denied
ThrowUserError("flag_update_denied",
{ name => $flag_type->name,
status => $status,
old_status => "X" });
}
}
######################################################################
# Private Functions
######################################################################

Просмотреть файл

@ -643,14 +643,13 @@ sub update
validatePrivate();
my $dbh = Bugzilla->dbh;
# 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.
# The order of these function calls is important, as Flag::validate
# assumes User::match_field has ensured that the values in the
# requestee fields are legitimate user email addresses.
Bugzilla::User::match_field($cgi, {
'^requestee(_type)?-(\d+)$' => { 'type' => 'multi' }
});
Bugzilla::Flag::validate($cgi, $bugid, $attach_id);
Bugzilla::FlagType::validate($cgi, $bugid, $attach_id);
my $bug = new Bugzilla::Bug($bugid);
# Lock database tables in preparation for updating the attachment.

Просмотреть файл

@ -39,6 +39,7 @@ use Bugzilla::Product;
use Bugzilla::Component;
use Bugzilla::Keyword;
use Bugzilla::Token;
use Bugzilla::Flag;
my $user = Bugzilla->login(LOGIN_REQUIRED);
@ -447,11 +448,7 @@ if (defined($cgi->upload('data')) || $cgi->param('attachurl')) {
my $error_mode_cache = Bugzilla->error_mode;
Bugzilla->error_mode(ERROR_MODE_DIE);
eval {
# Make sure no flags have already been set for this bug.
# Impossible? - Well, depends if you hack the URL or not.
# Passing a bug ID of 0 will make it complain if it finds one.
Bugzilla::Flag::validate($cgi, 0);
Bugzilla::FlagType::validate($cgi, $id);
Bugzilla::Flag::validate($cgi, $id);
Bugzilla::Flag::process($bug, undef, $timestamp, $cgi);
};
Bugzilla->error_mode($error_mode_cache);

Просмотреть файл

@ -54,10 +54,7 @@ use Bugzilla::Field;
use Bugzilla::Product;
use Bugzilla::Component;
use Bugzilla::Keyword;
# Use the Flag module to modify flag data if the user set flags.
use Bugzilla::Flag;
use Bugzilla::FlagType;
my $user = Bugzilla->login(LOGIN_REQUIRED);
local our $whoid = $user->id;
@ -214,8 +211,8 @@ foreach my $field ("dependson", "blocked") {
# do a match on the fields if applicable
# The order of these function calls is important, as both Flag::validate
# and FlagType::validate assume User::match_field has ensured that the values
# The order of these function calls is important, as Flag::validate
# assumes User::match_field has ensured that the values
# in the requestee fields are legitimate user email addresses.
&Bugzilla::User::match_field($cgi, {
'qa_contact' => { 'type' => 'single' },
@ -228,7 +225,6 @@ foreach my $field ("dependson", "blocked") {
# Validate flags in all cases. validate() should not detect any
# reference to flags if $cgi->param('id') is undefined.
Bugzilla::Flag::validate($cgi, $cgi->param('id'));
Bugzilla::FlagType::validate($cgi, $cgi->param('id'));
######################################################################
# End Data/Security Validation