summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
-rwxr-xr-xattachment.cgi546
1 files changed, 287 insertions, 259 deletions
diff --git a/attachment.cgi b/attachment.cgi
index 9f4b603dc..2b119e7ff 100755
--- a/attachment.cgi
+++ b/attachment.cgi
@@ -50,20 +50,8 @@ use Bugzilla::User;
use Bugzilla::Util;
use Bugzilla::Bug;
-# Check whether or not the user is logged in and, if so, set the $::userid
Bugzilla->login();
-# The ID of the bug to which the attachment is attached. Gets set
-# by validateID() (which validates the attachment ID, not the bug ID, but has
-# to check if the user is authorized to access this attachment) and is used
-# by Flag:: and FlagType::validate() to ensure the requestee (if any) for a
-# requested flag is authorized to see the bug in question. Note: This should
-# really be defined just above validateID() itself, but it's used in the main
-# body of the script before that function is defined, so we define it up here
-# instead. We should move the validation into each function and then move this
-# to just above validateID().
-my $bugid;
-
my $cgi = Bugzilla->cgi;
################################################################################
@@ -76,93 +64,42 @@ my $cgi = Bugzilla->cgi;
# supplied, we default to 'view'.
# Determine whether to use the action specified by the user or the default.
-my $action = $::FORM{'action'} || 'view';
+my $action = $cgi->param('action') || 'view';
if ($action eq "view")
{
- validateID();
- view();
+ view();
}
elsif ($action eq "interdiff")
{
- validateID('oldid');
- validateID('newid');
- validateFormat("html", "raw");
- validateContext();
- interdiff();
+ interdiff();
}
elsif ($action eq "diff")
{
- validateID();
- validateFormat("html", "raw");
- validateContext();
- diff();
+ diff();
}
elsif ($action eq "viewall")
{
- ValidateBugID($::FORM{'bugid'});
- viewall();
+ viewall();
}
elsif ($action eq "enter")
{
- Bugzilla->login(LOGIN_REQUIRED);
- ValidateBugID($::FORM{'bugid'});
- validateCanChangeBug($::FORM{'bugid'});
- enter();
+ Bugzilla->login(LOGIN_REQUIRED);
+ enter();
}
elsif ($action eq "insert")
{
- Bugzilla->login(LOGIN_REQUIRED);
- ValidateBugID($::FORM{'bugid'});
- validateCanChangeBug($::FORM{'bugid'});
- ValidateComment($::FORM{'comment'});
- validateFilename();
- validateIsPatch();
- my $data = validateData();
- validateDescription();
- validateContentType() unless $::FORM{'ispatch'};
- validateObsolete() if $::FORM{'obsolete'};
-
- # 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($cgi, {
- '^requestee(_type)?-(\d+)$' => { 'type' => 'single' }
- });
- Bugzilla::Flag::validate($cgi, $bugid);
- Bugzilla::FlagType::validate($cgi, $bugid, $::FORM{'id'});
-
- insert($data);
+ Bugzilla->login(LOGIN_REQUIRED);
+ insert();
}
elsif ($action eq "edit")
{
- validateID();
- validateCanEdit($::FORM{'id'});
- edit();
+ edit();
}
elsif ($action eq "update")
{
- Bugzilla->login(LOGIN_REQUIRED);
- ValidateComment($::FORM{'comment'});
- validateID();
- validateCanEdit($::FORM{'id'});
- validateCanChangeAttachment($::FORM{'id'});
- validateDescription();
- validateIsPatch();
- validateContentType() unless $::FORM{'ispatch'};
- validateIsObsolete();
- validatePrivate();
-
- # 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($cgi, {
- '^requestee(_type)?-(\d+)$' => { 'type' => 'single' }
- });
- Bugzilla::Flag::validate($cgi, $bugid);
- Bugzilla::FlagType::validate($cgi, $bugid, $::FORM{'id'});
-
- update();
+ Bugzilla->login(LOGIN_REQUIRED);
+ update();
}
else
{
@@ -175,6 +112,18 @@ exit;
# Data Validation / Security Authorization
################################################################################
+# Validates an attachment ID. Optionally takes a parameter of a form
+# variable name that contains the ID to be validated. If not specified,
+# uses 'id'.
+#
+# Will throw an error if 1) attachment ID is not a valid number,
+# 2) attachment does not exist, or 3) user isn't allowed to access the
+# attachment.
+#
+# Returns a list, where the first item is the validated, detainted
+# attachment id, and the 2nd item is the bug id corresponding to the
+# attachment.
+#
sub validateID
{
my $param = @_ ? $_[0] : 'id';
@@ -204,7 +153,8 @@ sub validateID
|| ThrowUserError("invalid_attach_id", { attach_id => $attach_id });
# Make sure the user is authorized to access this attachment's bug.
- ($bugid, my $isprivate) = FetchSQLData();
+ (my $bugid, my $isprivate) = FetchSQLData();
+
ValidateBugID($bugid);
if ($isprivate && Param("insidergroup")) {
UserInGroup(Param("insidergroup"))
@@ -212,10 +162,12 @@ sub validateID
object => "attachment"});
}
- # XXX shim code, kill $::FORM
- $::FORM{$param} = $attach_id;
+ return ($attach_id,$bugid);
}
+# Validates format of a diff/interdiff. Takes a list as an parameter, which
+# defines the valid format values. Will throw an error if the format is not
+# in the list. Returns either the user selected or default format.
sub validateFormat
{
# receives a list of legal formats; first item is a default
@@ -225,10 +177,11 @@ sub validateFormat
ThrowUserError("invalid_format", { format => $format, formats => \@_ });
}
- # XXX shim code, kill $::FORM
- $::FORM{'format'} = $format;
+ return $format;
}
+# Validates context of a diff/interdiff. Will throw an error if the context
+# is not number, "file" or "patch". Returns the validated, detainted context.
sub validateContext
{
my $context = $cgi->param('context') || "patch";
@@ -236,8 +189,8 @@ sub validateContext
detaint_natural($context)
|| ThrowUserError("invalid_context", { context => $cgi->param('context') });
}
- # XXX shim code, kill $::FORM
- $::FORM{'context'} = $context;
+
+ return $context;
}
sub validateCanEdit
@@ -249,14 +202,14 @@ sub validateCanEdit
# People not logged in can't actually commit changes, because that code
# calls Bugzilla->login with LOGIN_REQUIRED, not with LOGIN_NORMAL,
# before calling this sub
- return if $::userid == 0;
+ return unless Bugzilla->user;
# People in editbugs can edit all attachments
return if UserInGroup("editbugs");
# Bug 97729 - the submitter can edit their attachments
SendSQL("SELECT attach_id FROM attachments WHERE " .
- "attach_id = $attach_id AND submitter_id = $::userid");
+ "attach_id = $attach_id AND submitter_id = " . Bugzilla->user->id);
FetchSQLData()
|| ThrowUserError("illegal_attachment_edit",
@@ -291,28 +244,28 @@ sub validateCanChangeBug
sub validateDescription
{
- $::FORM{'description'}
- || ThrowUserError("missing_attachment_description");
+ $cgi->param('description')
+ || ThrowUserError("missing_attachment_description");
}
sub validateIsPatch
{
- # Set the ispatch flag to zero if it is undefined, since the UI uses
- # an HTML checkbox to represent this flag, and unchecked HTML checkboxes
- # do not get sent in HTML requests.
- $::FORM{'ispatch'} = $::FORM{'ispatch'} ? 1 : 0;
+ # Set the ispatch flag to zero if it is undefined, since the UI uses
+ # an HTML checkbox to represent this flag, and unchecked HTML checkboxes
+ # do not get sent in HTML requests.
+ $cgi->param('ispatch', $cgi->param('ispatch') ? 1 : 0);
- # Set the content type to text/plain if the attachment is a patch.
- $::FORM{'contenttype'} = "text/plain" if $::FORM{'ispatch'};
+ # Set the content type to text/plain if the attachment is a patch.
+ $cgi->param('contenttype', 'text/plain') if $cgi->param('ispatch');
}
sub validateContentType
{
- if (!$::FORM{'contenttypemethod'})
+ if (!defined $cgi->param('contenttypemethod'))
{
ThrowUserError("missing_content_type_method");
}
- elsif ($::FORM{'contenttypemethod'} eq 'autodetect')
+ elsif ($cgi->param('contenttypemethod') eq 'autodetect')
{
my $contenttype = $cgi->uploadInfo($cgi->param('data'))->{'Content-Type'};
# The user asked us to auto-detect the content type, so use the type
@@ -321,37 +274,38 @@ sub validateContentType
{
ThrowUserError("missing_content_type");
}
- $::FORM{'contenttype'} = $contenttype;
+ $cgi->param('contenttype', $contenttype);
}
- elsif ($::FORM{'contenttypemethod'} eq 'list')
+ elsif ($cgi->param('contenttypemethod') eq 'list')
{
# The user selected a content type from the list, so use their selection.
- $::FORM{'contenttype'} = $::FORM{'contenttypeselection'};
+ $cgi->param('contenttype', $cgi->param('contenttypeselection'));
}
- elsif ($::FORM{'contenttypemethod'} eq 'manual')
+ elsif ($cgi->param('contenttypemethod') eq 'manual')
{
# The user entered a content type manually, so use their entry.
- $::FORM{'contenttype'} = $::FORM{'contenttypeentry'};
+ $cgi->param('contenttype', $cgi->param('contenttypeentry'));
}
else
{
ThrowCodeError("illegal_content_type_method",
- { contenttypemethod => $::FORM{'contenttypemethod'} });
+ { contenttypemethod => $cgi->param('contenttypemethod') });
}
- if ( $::FORM{'contenttype'} !~ /^(application|audio|image|message|model|multipart|text|video)\/.+$/ )
+ if ( $cgi->param('contenttype') !~
+ /^(application|audio|image|message|model|multipart|text|video)\/.+$/ )
{
ThrowUserError("invalid_content_type",
- { contenttype => $::FORM{'contenttype'} });
+ { contenttype => $cgi->param('contenttype') });
}
}
sub validateIsObsolete
{
- # Set the isobsolete flag to zero if it is undefined, since the UI uses
- # an HTML checkbox to represent this flag, and unchecked HTML checkboxes
- # do not get sent in HTML requests.
- $::FORM{'isobsolete'} = $::FORM{'isobsolete'} ? 1 : 0;
+ # Set the isobsolete flag to zero if it is undefined, since the UI uses
+ # an HTML checkbox to represent this flag, and unchecked HTML checkboxes
+ # do not get sent in HTML requests.
+ $cgi->param('isobsolete', $cgi->param('isobsolete') ? 1 : 0);
}
sub validatePrivate
@@ -359,17 +313,17 @@ sub validatePrivate
# Set the isprivate flag to zero if it is undefined, since the UI uses
# an HTML checkbox to represent this flag, and unchecked HTML checkboxes
# do not get sent in HTML requests.
- $::FORM{'isprivate'} = $::FORM{'isprivate'} ? 1 : 0;
+ $cgi->param('isprivate', $cgi->param('isprivate') ? 1 : 0);
}
sub validateData
{
- my $maxsize = $::FORM{'ispatch'} ? Param('maxpatchsize') : Param('maxattachmentsize');
+ my $maxsize = $cgi->param('ispatch') ? Param('maxpatchsize') : Param('maxattachmentsize');
$maxsize *= 1024; # Convert from K
my $fh;
# Skip uploading into a local variable if the user wants to upload huge
# attachments into local files.
- if (!$::FORM{'bigfile'})
+ if (!$cgi->param('bigfile'))
{
$fh = $cgi->upload('data');
}
@@ -377,7 +331,7 @@ sub validateData
# We could get away with reading only as much as required, except that then
# we wouldn't have a size to print to the error handler below.
- if (!$::FORM{'bigfile'})
+ if (!$cgi->param('bigfile'))
{
# enable 'slurp' mode
local $/;
@@ -385,14 +339,14 @@ sub validateData
}
$data
- || ($::FORM{'bigfile'})
+ || ($cgi->param('bigfile'))
|| ThrowUserError("zero_length_file");
# Make sure the attachment does not exceed the maximum permitted size
my $len = $data ? length($data) : 0;
if ($maxsize && $len > $maxsize) {
my $vars = { filesize => sprintf("%.0f", $len/1024) };
- if ( $::FORM{'ispatch'} ) {
+ if ($cgi->param('ispatch')) {
ThrowUserError("patch_too_large", $vars);
} else {
ThrowUserError("file_too_large", $vars);
@@ -402,13 +356,12 @@ sub validateData
return $data || '';
}
-my $filename;
sub validateFilename
{
defined $cgi->upload('data')
|| ThrowUserError("file_not_specified");
- $filename = $cgi->upload('data');
+ my $filename = $cgi->upload('data');
# Remove path info (if any) from the file name. The browser should do this
# for us, but some are buggy. This may not work on Mac file names and could
@@ -420,13 +373,17 @@ sub validateFilename
# Truncate the filename to 100 characters, counting from the end of the string
# to make sure we keep the filename extension.
$filename = substr($filename, -100, 100);
+
+ return $filename;
}
sub validateObsolete
{
+ my @obsolete_ids = ();
+
# Make sure the attachment id is valid and the user has permissions to view
# the bug to which it is attached.
- foreach my $attachid (@{$::MFORM{'obsolete'}}) {
+ foreach my $attachid ($cgi->param('obsolete')) {
my $vars = {};
$vars->{'attach_id'} = $attachid;
@@ -444,9 +401,9 @@ sub validateObsolete
$vars->{'description'} = $description;
- if ($bugid != $::FORM{'bugid'})
+ if ($bugid != $cgi->param('bugid'))
{
- $vars->{'my_bug_id'} = $::FORM{'bugid'};
+ $vars->{'my_bug_id'} = $cgi->param('bugid');
$vars->{'attach_bug_id'} = $bugid;
ThrowCodeError("mismatched_bug_ids_on_obsolete", $vars);
}
@@ -458,7 +415,10 @@ sub validateObsolete
# Check that the user can modify this attachment
validateCanEdit($attachid);
+ push(@obsolete_ids, $attachid);
}
+
+ return @obsolete_ids;
}
# Returns 1 if the parameter is a content-type viewable in this browser
@@ -497,21 +457,25 @@ sub isViewable
# Functions
################################################################################
+# Display an attachment.
sub view
{
- # Display an attachment.
+ # Retrieve and validate parameters
+ my ($attach_id) = validateID();
# Retrieve the attachment content and its content type from the database.
- SendSQL("SELECT mimetype, filename, thedata FROM attachments WHERE attach_id = $::FORM{'id'}");
+ SendSQL("SELECT mimetype, filename, thedata FROM attachments " .
+ "WHERE attach_id = $attach_id");
my ($contenttype, $filename, $thedata) = FetchSQLData();
- # Bug 111522: allow overriding content-type manually in the posted $::FORM.
- if ($::FORM{'content_type'})
+ # Bug 111522: allow overriding content-type manually in the posted form
+ # params.
+ if (defined $cgi->param('content_type'))
{
- $::FORM{'contenttypemethod'} = 'manual';
- $::FORM{'contenttypeentry'} = $::FORM{'content_type'};
+ $cgi->param('contenttypemethod', 'manual');
+ $cgi->param('contenttypeentry', $cgi->param('content_type'));
validateContentType();
- $contenttype = $::FORM{'content_type'};
+ $contenttype = $cgi->param('content_type');
}
# Return the appropriate HTTP response headers.
@@ -521,10 +485,9 @@ sub view
# stored in a local file
if ($filesize == 0)
{
- my $attachid = $::FORM{'id'};
- my $hash = ($attachid % 100) + 100;
+ my $hash = ($attach_id % 100) + 100;
$hash =~ s/.*(\d\d)$/group.$1/;
- if (open(AH, "$attachdir/$hash/attachment.$attachid")) {
+ if (open(AH, "$attachdir/$hash/attachment.$attach_id")) {
binmode AH;
$filesize = (stat(AH))[7];
}
@@ -556,13 +519,19 @@ sub view
sub interdiff
{
+ # Retrieve and validate parameters
+ my ($old_id) = validateID('oldid');
+ my ($new_id) = validateID('newid');
+ my $format = validateFormat('html', 'raw');
+ my $context = validateContext();
+
# Get old patch data
my ($old_bugid, $old_description, $old_filename, $old_file_list) =
- get_unified_diff($::FORM{'oldid'});
+ get_unified_diff($old_id);
# Get new patch data
my ($new_bugid, $new_description, $new_filename, $new_file_list) =
- get_unified_diff($::FORM{'newid'});
+ get_unified_diff($new_id);
my $warning = warn_if_interdiff_might_fail($old_file_list, $new_file_list);
@@ -574,8 +543,8 @@ sub interdiff
$ENV{'PATH'} = $::diffpath;
open my $interdiff_fh, "$::interdiffbin $old_filename $new_filename|";
binmode $interdiff_fh;
- my ($reader, $last_reader) = setup_patch_readers("");
- if ($::FORM{'format'} eq "raw")
+ my ($reader, $last_reader) = setup_patch_readers("", $context);
+ if ($format eq 'raw')
{
require PatchReader::DiffPrinter::raw;
$last_reader->sends_data_to(new PatchReader::DiffPrinter::raw());
@@ -587,16 +556,16 @@ sub interdiff
{
$vars->{warning} = $warning if $warning;
$vars->{bugid} = $new_bugid;
- $vars->{oldid} = $::FORM{'oldid'};
+ $vars->{oldid} = $old_id;
$vars->{old_desc} = $old_description;
- $vars->{newid} = $::FORM{'newid'};
+ $vars->{newid} = $new_id;
$vars->{new_desc} = $new_description;
delete $vars->{attachid};
delete $vars->{do_context};
delete $vars->{context};
- setup_template_patch_reader($last_reader);
+ setup_template_patch_reader($last_reader, $format, $context);
}
- $reader->iterate_fh($interdiff_fh, "interdiff #$::FORM{'oldid'} #$::FORM{'newid'}");
+ $reader->iterate_fh($interdiff_fh, "interdiff #$old_id #$new_id");
close $interdiff_fh;
$ENV{'PATH'} = '';
@@ -676,7 +645,7 @@ sub warn_if_interdiff_might_fail {
}
sub setup_patch_readers {
- my ($diff_root) = @_;
+ my ($diff_root, $context) = @_;
#
# Parameters:
@@ -700,11 +669,11 @@ sub setup_patch_readers {
$last_reader = $last_reader->sends_data_to;
}
# Add in cvs context if we have the necessary info to do it
- if ($::FORM{'context'} ne "patch" && $::cvsbin && Param('cvsroot_get'))
+ if ($context ne "patch" && $::cvsbin && Param('cvsroot_get'))
{
require PatchReader::AddCVSContext;
$last_reader->sends_data_to(
- new PatchReader::AddCVSContext($::FORM{'context'},
+ new PatchReader::AddCVSContext($context,
Param('cvsroot_get')));
$last_reader = $last_reader->sends_data_to;
}
@@ -713,20 +682,18 @@ sub setup_patch_readers {
sub setup_template_patch_reader
{
- my ($last_reader) = @_;
+ my ($last_reader, $format, $context) = @_;
require PatchReader::DiffPrinter::template;
- my $format = $::FORM{'format'};
-
# Define the vars for templates
- if (defined($::FORM{'headers'})) {
- $vars->{headers} = $::FORM{'headers'};
+ if (defined $cgi->param('headers')) {
+ $vars->{headers} = $cgi->param('headers');
} else {
- $vars->{headers} = 1 if !defined($::FORM{'headers'});
+ $vars->{headers} = 1 if !defined $cgi->param('headers');
}
- $vars->{collapsed} = $::FORM{'collapsed'};
- $vars->{context} = $::FORM{'context'};
+ $vars->{collapsed} = $cgi->param('collapsed');
+ $vars->{context} = $context;
$vars->{do_context} = $::cvsbin && Param('cvsroot_get') && !$vars->{'newid'};
# Print everything out
@@ -745,8 +712,14 @@ sub setup_template_patch_reader
sub diff
{
+ # Retrieve and validate parameters
+ my ($attach_id) = validateID();
+ my $format = validateFormat('html', 'raw');
+ my $context = validateContext();
+
# Get patch data
- SendSQL("SELECT bug_id, description, ispatch, thedata FROM attachments WHERE attach_id = $::FORM{'id'}");
+ SendSQL("SELECT bug_id, description, ispatch, thedata FROM attachments " .
+ "WHERE attach_id = $attach_id");
my ($bugid, $description, $ispatch, $thedata) = FetchSQLData();
# If it is not a patch, view normally
@@ -756,9 +729,9 @@ sub diff
return;
}
- my ($reader, $last_reader) = setup_patch_readers();
+ my ($reader, $last_reader) = setup_patch_readers(undef,$context);
- if ($::FORM{'format'} eq "raw")
+ if ($format eq 'raw')
{
require PatchReader::DiffPrinter::raw;
$last_reader->sends_data_to(new PatchReader::DiffPrinter::raw());
@@ -766,7 +739,7 @@ sub diff
use vars qw($cgi);
print $cgi->header(-type => 'text/plain',
-expires => '+3M');
- $reader->iterate_string("Attachment " . $::FORM{'id'}, $thedata);
+ $reader->iterate_string("Attachment $attach_id", $thedata);
}
else
{
@@ -778,7 +751,7 @@ sub diff
SendSQL("SELECT attach_id, description FROM attachments WHERE bug_id = $bugid AND ispatch = 1 ORDER BY creation_ts DESC");
my $select_next_patch = 0;
while (my ($other_id, $other_desc) = FetchSQLData()) {
- if ($other_id eq $::FORM{'id'}) {
+ if ($other_id eq $attach_id) {
$select_next_patch = 1;
} else {
push @{$vars->{other_patches}}, { id => $other_id, desc => $other_desc, selected => $select_next_patch };
@@ -790,20 +763,24 @@ sub diff
}
$vars->{bugid} = $bugid;
- $vars->{attachid} = $::FORM{'id'};
+ $vars->{attachid} = $attach_id;
$vars->{description} = $description;
- setup_template_patch_reader($last_reader);
+ setup_template_patch_reader($last_reader, $format, $context);
# Actually print out the patch
- $reader->iterate_string("Attachment " . $::FORM{'id'}, $thedata);
+ $reader->iterate_string("Attachment $attach_id", $thedata);
}
}
+# Display all attachments for a given bug in a series of IFRAMEs within one
+# HTML page.
sub viewall
{
- # Display all attachments for a given bug in a series of IFRAMEs within one HTML page.
+ # Retrieve and validate parameters
+ my $bugid = $cgi->param('bugid');
+ ValidateBugID($bugid);
- # Retrieve the attachments from the database and write them into an array
- # of hashes where each hash represents one attachment.
+ # Retrieve the attachments from the database and write them into an array
+ # of hashes where each hash represents one attachment.
my $privacy = "";
my $dbh = Bugzilla->dbh;
@@ -814,7 +791,7 @@ sub viewall
$dbh->sql_date_format('creation_ts', '%Y.%m.%d %H:%i') . ",
mimetype, description, ispatch, isobsolete, isprivate,
LENGTH(thedata)
- FROM attachments WHERE bug_id = $::FORM{'bugid'} $privacy
+ FROM attachments WHERE bug_id = $bugid $privacy
ORDER BY attach_id");
my @attachments; # the attachments array
while (MoreSQLData())
@@ -833,11 +810,11 @@ sub viewall
# Retrieve the bug summary (for displaying on screen) and assignee.
SendSQL("SELECT short_desc, assigned_to FROM bugs " .
- "WHERE bug_id = $::FORM{'bugid'}");
+ "WHERE bug_id = $bugid");
my ($bugsummary, $assignee_id) = FetchSQLData();
# Define the variables and functions that will be passed to the UI template.
- $vars->{'bugid'} = $::FORM{'bugid'};
+ $vars->{'bugid'} = $bugid;
$vars->{'attachments'} = \@attachments;
$vars->{'bugassignee_id'} = $assignee_id;
$vars->{'bugsummary'} = $bugsummary;
@@ -850,20 +827,23 @@ sub viewall
|| ThrowTemplateError($template->error());
}
-
+# Display a form for entering a new attachment.
sub enter
{
- # Display a form for entering a new attachment.
+ # Retrieve and validate parameters
+ my $bugid = $cgi->param('bugid');
+ ValidateBugID($bugid);
+ validateCanChangeBug($bugid);
# Retrieve the attachments the user can edit from the database and write
# them into an array of hashes where each hash represents one attachment.
my $canEdit = "";
if (!UserInGroup("editbugs")) {
- $canEdit = "AND submitter_id = $::userid";
+ $canEdit = "AND submitter_id = " . Bugzilla->user->id;
}
SendSQL("SELECT attach_id, description, isprivate
FROM attachments
- WHERE bug_id = $::FORM{'bugid'}
+ WHERE bug_id = $bugid
AND isobsolete = 0 $canEdit
ORDER BY attach_id");
my @attachments; # the attachments array
@@ -877,18 +857,18 @@ sub enter
# Retrieve the bug summary (for displaying on screen) and assignee.
SendSQL("SELECT short_desc, assigned_to FROM bugs
- WHERE bug_id = $::FORM{'bugid'}");
+ WHERE bug_id = $bugid");
my ($bugsummary, $assignee_id) = FetchSQLData();
# Define the variables and functions that will be passed to the UI template.
- $vars->{'bugid'} = $::FORM{'bugid'};
+ $vars->{'bugid'} = $bugid;
$vars->{'attachments'} = \@attachments;
$vars->{'bugassignee_id'} = $assignee_id;
$vars->{'bugsummary'} = $bugsummary;
$vars->{'GetBugLink'} = \&GetBugLink;
SendSQL("SELECT product_id, component_id FROM bugs
- WHERE bug_id = $::FORM{'bugid'}");
+ WHERE bug_id = $bugid");
my ($product_id, $component_id) = FetchSQLData();
my $flag_types = Bugzilla::FlagType::match({'target_type' => 'attachment',
'product_id' => $product_id,
@@ -904,19 +884,40 @@ sub enter
|| ThrowTemplateError($template->error());
}
-
+# Insert a new attachment into the database.
sub insert
{
- my ($data) = @_;
+ my $dbh = Bugzilla->dbh;
+ my $userid = Bugzilla->user->id;
- # Insert a new attachment into the database.
- my $dbh = Bugzilla->dbh;
-
- # Escape characters in strings that will be used in SQL statements.
- $filename = SqlQuote($filename);
- my $description = SqlQuote($::FORM{'description'});
- my $contenttype = SqlQuote($::FORM{'contenttype'});
- my $isprivate = $::FORM{'isprivate'} ? 1 : 0;
+ # Retrieve and validate parameters
+ my $bugid = $cgi->param('bugid');
+ ValidateBugID($bugid);
+ validateCanChangeBug($bugid);
+ ValidateComment(scalar $cgi->param('comment'));
+ my $filename = validateFilename();
+ validateIsPatch();
+ my $data = validateData();
+ validateDescription();
+ validateContentType() unless $cgi->param('ispatch');
+
+ my @obsolete_ids = ();
+ @obsolete_ids = validateObsolete() if $cgi->param('obsolete');
+
+ # 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($cgi, {
+ '^requestee(_type)?-(\d+)$' => { 'type' => 'single' }
+ });
+ Bugzilla::Flag::validate($cgi, $bugid);
+ Bugzilla::FlagType::validate($cgi, $bugid, $cgi->param('id'));
+
+ # Escape characters in strings that will be used in SQL statements.
+ my $sql_filename = SqlQuote($filename);
+ my $description = SqlQuote($cgi->param('description'));
+ my $contenttype = SqlQuote($cgi->param('contenttype'));
+ my $isprivate = $cgi->param('isprivate') ? 1 : 0;
# Figure out when the changes were made.
my ($timestamp) = Bugzilla->dbh->selectrow_array("SELECT NOW()");
@@ -926,9 +927,9 @@ sub insert
my $sth = $dbh->prepare("INSERT INTO attachments
(thedata, bug_id, creation_ts, filename, description,
mimetype, ispatch, isprivate, submitter_id)
- VALUES (?, $::FORM{'bugid'}, $sql_timestamp, $filename,
- $description, $contenttype, $::FORM{'ispatch'},
- $isprivate, $::userid)");
+ VALUES (?, $bugid, $sql_timestamp, $sql_filename,
+ $description, $contenttype, " . $cgi->param('ispatch') . ",
+ $isprivate, $userid)");
# We only use $data here in this INSERT with a placeholder,
# so it's safe.
trick_taint($data);
@@ -940,7 +941,7 @@ sub insert
# If the file is to be stored locally, stream the file from the webserver
# to the local file without reading it into a local variable.
- if ($::FORM{'bigfile'})
+ if ($cgi->param('bigfile'))
{
my $fh = $cgi->upload('data');
my $hash = ($attachid % 100) + 100;
@@ -967,10 +968,11 @@ sub insert
# Insert a comment about the new attachment into the database.
- my $comment = "Created an attachment (id=$attachid)\n$::FORM{'description'}\n";
- $comment .= ("\n" . $::FORM{'comment'}) if $::FORM{'comment'};
+ my $comment = "Created an attachment (id=$attachid)\n" .
+ $cgi->param('description') . "\n";
+ $comment .= ("\n" . $cgi->param('comment')) if defined $cgi->param('comment');
- AppendComment($::FORM{'bugid'},
+ AppendComment($bugid,
Bugzilla->user->login,
$comment,
$isprivate,
@@ -978,20 +980,23 @@ sub insert
# Make existing attachments obsolete.
my $fieldid = GetFieldID('attachments.isobsolete');
- foreach my $obsolete_id (@{$::MFORM{'obsolete'}}) {
+ foreach my $obsolete_id (@obsolete_ids) {
# If the obsolete attachment has request flags, cancel them.
# This call must be done before updating the 'attachments' table.
- Bugzilla::Flag::CancelRequests($::FORM{'bugid'}, $obsolete_id, $sql_timestamp);
-
- SendSQL("UPDATE attachments SET isobsolete = 1 WHERE attach_id = $obsolete_id");
- SendSQL("INSERT INTO bugs_activity (bug_id, attach_id, who, bug_when, fieldid, removed, added)
- VALUES ($::FORM{'bugid'}, $obsolete_id, $::userid, $sql_timestamp, $fieldid, '0', '1')");
+ Bugzilla::Flag::CancelRequests($bugid, $obsolete_id, $sql_timestamp);
+
+ SendSQL("UPDATE attachments SET isobsolete = 1 " .
+ "WHERE attach_id = $obsolete_id");
+ SendSQL("INSERT INTO bugs_activity (bug_id, attach_id, who, bug_when,
+ fieldid, removed, added)
+ VALUES ($bugid, $obsolete_id, $userid, $sql_timestamp, $fieldid,
+ '0', '1')");
}
# Assign the bug to the user, if they are allowed to take it
my $owner = "";
- if ($::FORM{'takebug'} && UserInGroup("editbugs")) {
+ if ($cgi->param('takebug') && UserInGroup("editbugs")) {
my @fields = ("assigned_to", "bug_status", "resolution", "login_name");
@@ -1000,10 +1005,10 @@ sub insert
"FROM bugs " .
"INNER JOIN profiles " .
"ON profiles.userid = bugs.assigned_to " .
- "WHERE bugs.bug_id = $::FORM{'bugid'}");
+ "WHERE bugs.bug_id = $bugid");
my @oldvalues = FetchSQLData();
- my @newvalues = ($::userid, "ASSIGNED", "", DBID_to_name($::userid));
+ my @newvalues = ($userid, "ASSIGNED", "", Bugzilla->user->login);
# Make sure the person we are taking the bug from gets mail.
$owner = $oldvalues[3];
@@ -1014,7 +1019,7 @@ sub insert
# Update the bug record. Note that this doesn't involve login_name.
SendSQL("UPDATE bugs SET delta_ts = $sql_timestamp, " .
join(", ", map("$fields[$_] = $newvalues[$_]", (0..2))) .
- " WHERE bug_id = $::FORM{'bugid'}");
+ " WHERE bug_id = $bugid");
# We store email addresses in the bugs_activity table rather than IDs.
$oldvalues[0] = $oldvalues[3];
@@ -1026,9 +1031,8 @@ sub insert
my $fieldid = GetFieldID($fields[$i]);
SendSQL("INSERT INTO bugs_activity " .
"(bug_id, who, bug_when, fieldid, removed, added) " .
- " VALUES ($::FORM{'bugid'}, $::userid, " .
- "$sql_timestamp " .
- ", $fieldid, $oldvalues[$i], $newvalues[$i])");
+ "VALUES ($bugid, $userid, $sql_timestamp, " .
+ "$fieldid, $oldvalues[$i], $newvalues[$i])");
}
}
}
@@ -1040,17 +1044,11 @@ sub insert
# Define the variables and functions that will be passed to the UI template.
$vars->{'mailrecipients'} = { 'changer' => Bugzilla->user->login,
'owner' => $owner };
- my $bugid = $::FORM{'bugid'};
- detaint_natural($bugid); # don't bother with error condition, we know it'll work
- # because of ValidateBugID above. This is only needed
- # for Perl 5.6.0. If we ever require Perl 5.6.1 or
- # newer, or detaint something other than $::FORM{'bugid'}
- # in ValidateBugID above, then this can go away.
$vars->{'bugid'} = $bugid;
$vars->{'attachid'} = $attachid;
$vars->{'description'} = $description;
- $vars->{'contenttypemethod'} = $::FORM{'contenttypemethod'};
- $vars->{'contenttype'} = $::FORM{'contenttype'};
+ $vars->{'contenttypemethod'} = $cgi->param('contenttypemethod');
+ $vars->{'contenttype'} = $cgi->param('contenttype');
print Bugzilla->cgi->header();
@@ -1059,18 +1057,20 @@ sub insert
|| ThrowTemplateError($template->error());
}
-
+# Edit an attachment record. Users with "editbugs" privileges, (or the
+# original attachment's submitter) can edit the attachment's description,
+# content type, ispatch and isobsolete flags, and statuses, and they can
+# also submit a comment that appears in the bug.
+# Users cannot edit the content of the attachment itself.
sub edit
{
- # Edit an attachment record. Users with "editbugs" privileges, (or the
- # original attachment's submitter) can edit the attachment's description,
- # content type, ispatch and isobsolete flags, and statuses, and they can
- # also submit a comment that appears in the bug.
- # Users cannot edit the content of the attachment itself.
+ # Retrieve and validate parameters
+ my ($attach_id) = validateID();
+ validateCanEdit($attach_id);
# Retrieve the attachment from the database.
SendSQL("SELECT description, mimetype, filename, bug_id, ispatch, isobsolete, isprivate, LENGTH(thedata)
- FROM attachments WHERE attach_id = $::FORM{'id'}");
+ FROM attachments WHERE attach_id = $attach_id");
my ($description, $contenttype, $filename, $bugid, $ispatch, $isobsolete, $isprivate, $datasize) = FetchSQLData();
my $isviewable = isViewable($contenttype);
@@ -1091,14 +1091,14 @@ sub edit
'component_id' => $component_id });
foreach my $flag_type (@$flag_types) {
$flag_type->{'flags'} = Bugzilla::Flag::match({ 'type_id' => $flag_type->{'id'},
- 'attach_id' => $::FORM{'id'},
+ 'attach_id' => $attach_id,
'is_active' => 1 });
}
$vars->{'flag_types'} = $flag_types;
$vars->{'any_flags_requesteeble'} = grep($_->{'is_requesteeble'}, @$flag_types);
# Define the variables and functions that will be passed to the UI template.
- $vars->{'attachid'} = $::FORM{'id'};
+ $vars->{'attachid'} = $attach_id;
$vars->{'description'} = $description;
$vars->{'contenttype'} = $contenttype;
$vars->{'filename'} = $filename;
@@ -1124,15 +1124,31 @@ sub edit
|| ThrowTemplateError($template->error());
}
-
+# Updates an attachment record.
sub update
{
- # Updates an attachment record.
my $dbh = Bugzilla->dbh;
-
- # Get the bug ID for the bug to which this attachment is attached.
- SendSQL("SELECT bug_id FROM attachments WHERE attach_id = $::FORM{'id'}");
- my $bugid = FetchSQLData();
+ my $userid = Bugzilla->user->id;
+
+ # Retrieve and validate parameters
+ ValidateComment(scalar $cgi->param('comment'));
+ my ($attach_id, $bugid) = validateID();
+ validateCanEdit($attach_id);
+ validateCanChangeAttachment($attach_id);
+ validateDescription();
+ validateIsPatch();
+ validateContentType() unless $cgi->param('ispatch');
+ validateIsObsolete();
+ validatePrivate();
+
+ # 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($cgi, {
+ '^requestee(_type)?-(\d+)$' => { 'type' => 'single' }
+ });
+ Bugzilla::Flag::validate($cgi, $bugid);
+ Bugzilla::FlagType::validate($cgi, $bugid, $attach_id);
# Lock database tables in preparation for updating the attachment.
$dbh->bz_lock_tables('attachments WRITE', 'flags WRITE' ,
@@ -1152,14 +1168,14 @@ sub update
# Get a copy of the attachment record before we make changes
# so we can record those changes in the activity table.
SendSQL("SELECT description, mimetype, filename, ispatch, isobsolete, isprivate
- FROM attachments WHERE attach_id = $::FORM{'id'}");
+ FROM attachments WHERE attach_id = $attach_id");
my ($olddescription, $oldcontenttype, $oldfilename, $oldispatch,
$oldisobsolete, $oldisprivate) = FetchSQLData();
# Quote the description and content type for use in the SQL UPDATE statement.
- my $quoteddescription = SqlQuote($::FORM{'description'});
- my $quotedcontenttype = SqlQuote($::FORM{'contenttype'});
- my $quotedfilename = SqlQuote($::FORM{'filename'});
+ my $quoteddescription = SqlQuote($cgi->param('description'));
+ my $quotedcontenttype = SqlQuote($cgi->param('contenttype'));
+ my $quotedfilename = SqlQuote($cgi->param('filename'));
# Figure out when the changes were made.
SendSQL("SELECT NOW()");
@@ -1169,7 +1185,7 @@ sub update
# to attachments so that we can delete pending requests if the user
# is obsoleting this attachment without deleting any requests
# the user submits at the same time.
- my $target = Bugzilla::Flag::GetTarget(undef, $::FORM{'id'});
+ my $target = Bugzilla::Flag::GetTarget(undef, $attach_id);
Bugzilla::Flag::process($target, $timestamp, $cgi);
# Update the attachment record in the database.
@@ -1177,71 +1193,83 @@ sub update
SET description = $quoteddescription ,
mimetype = $quotedcontenttype ,
filename = $quotedfilename ,
- ispatch = $::FORM{'ispatch'},
- isobsolete = $::FORM{'isobsolete'} ,
- isprivate = $::FORM{'isprivate'}
- WHERE attach_id = $::FORM{'id'}
+ ispatch = " . $cgi->param('ispatch') . ",
+ isobsolete = " . $cgi->param('isobsolete') . ",
+ isprivate = " . $cgi->param('isprivate') . "
+ WHERE attach_id = $attach_id
");
# Record changes in the activity table.
my $sql_timestamp = SqlQuote($timestamp);
- if ($olddescription ne $::FORM{'description'}) {
+ if ($olddescription ne $cgi->param('description')) {
my $quotedolddescription = SqlQuote($olddescription);
my $fieldid = GetFieldID('attachments.description');
- SendSQL("INSERT INTO bugs_activity (bug_id, attach_id, who, bug_when, fieldid, removed, added)
- VALUES ($bugid, $::FORM{'id'}, $::userid, $sql_timestamp, $fieldid, $quotedolddescription, $quoteddescription)");
+ SendSQL("INSERT INTO bugs_activity (bug_id, attach_id, who, bug_when,
+ fieldid, removed, added)
+ VALUES ($bugid, $attach_id, $userid, $sql_timestamp, $fieldid,
+ $quotedolddescription, $quoteddescription)");
}
- if ($oldcontenttype ne $::FORM{'contenttype'}) {
+ if ($oldcontenttype ne $cgi->param('contenttype')) {
my $quotedoldcontenttype = SqlQuote($oldcontenttype);
my $fieldid = GetFieldID('attachments.mimetype');
- SendSQL("INSERT INTO bugs_activity (bug_id, attach_id, who, bug_when, fieldid, removed, added)
- VALUES ($bugid, $::FORM{'id'}, $::userid, $sql_timestamp, $fieldid, $quotedoldcontenttype, $quotedcontenttype)");
+ SendSQL("INSERT INTO bugs_activity (bug_id, attach_id, who, bug_when,
+ fieldid, removed, added)
+ VALUES ($bugid, $attach_id, $userid, $sql_timestamp, $fieldid,
+ $quotedoldcontenttype, $quotedcontenttype)");
}
- if ($oldfilename ne $::FORM{'filename'}) {
+ if ($oldfilename ne $cgi->param('filename')) {
my $quotedoldfilename = SqlQuote($oldfilename);
my $fieldid = GetFieldID('attachments.filename');
- SendSQL("INSERT INTO bugs_activity (bug_id, attach_id, who, bug_when, fieldid, removed, added)
- VALUES ($bugid, $::FORM{'id'}, $::userid, $sql_timestamp, $fieldid, $quotedoldfilename, $quotedfilename)");
+ SendSQL("INSERT INTO bugs_activity (bug_id, attach_id, who, bug_when,
+ fieldid, removed, added)
+ VALUES ($bugid, $attach_id, $userid, $sql_timestamp, $fieldid,
+ $quotedoldfilename, $quotedfilename)");
}
- if ($oldispatch ne $::FORM{'ispatch'}) {
+ if ($oldispatch ne $cgi->param('ispatch')) {
my $fieldid = GetFieldID('attachments.ispatch');
- SendSQL("INSERT INTO bugs_activity (bug_id, attach_id, who, bug_when, fieldid, removed, added)
- VALUES ($bugid, $::FORM{'id'}, $::userid, $sql_timestamp, $fieldid, $oldispatch, $::FORM{'ispatch'})");
+ SendSQL("INSERT INTO bugs_activity (bug_id, attach_id, who, bug_when,
+ fieldid, removed, added)
+ VALUES ($bugid, $attach_id, $userid, $sql_timestamp, $fieldid,
+ $oldispatch, " . $cgi->param('ispatch') . ")");
}
- if ($oldisobsolete ne $::FORM{'isobsolete'}) {
+ if ($oldisobsolete ne $cgi->param('isobsolete')) {
my $fieldid = GetFieldID('attachments.isobsolete');
- SendSQL("INSERT INTO bugs_activity (bug_id, attach_id, who, bug_when, fieldid, removed, added)
- VALUES ($bugid, $::FORM{'id'}, $::userid, $sql_timestamp, $fieldid, $oldisobsolete, $::FORM{'isobsolete'})");
+ SendSQL("INSERT INTO bugs_activity (bug_id, attach_id, who, bug_when,
+ fieldid, removed, added)
+ VALUES ($bugid, $attach_id, $userid, $sql_timestamp, $fieldid,
+ $oldisobsolete, " . $cgi->param('isobsolete') . ")");
}
- if ($oldisprivate ne $::FORM{'isprivate'}) {
+ if ($oldisprivate ne $cgi->param('isprivate')) {
my $fieldid = GetFieldID('attachments.isprivate');
- SendSQL("INSERT INTO bugs_activity (bug_id, attach_id, who, bug_when, fieldid, removed, added)
- VALUES ($bugid, $::FORM{'id'}, $::userid, $sql_timestamp, $fieldid, $oldisprivate, $::FORM{'isprivate'})");
+ SendSQL("INSERT INTO bugs_activity (bug_id, attach_id, who, bug_when,
+ fieldid, removed, added)
+ VALUES ($bugid, $attach_id, $userid, $sql_timestamp, $fieldid,
+ $oldisprivate, " . $cgi->param('isprivate') . ")");
}
# Unlock all database tables now that we are finished updating the database.
$dbh->bz_unlock_tables();
+ # Get the user's login name since the AppendComment and header functions
+ # need it.
+ my $who = Bugzilla->user->login;
+
# If the user submitted a comment while editing the attachment,
# add the comment to the bug.
- if ( $::FORM{'comment'} )
+ if (defined $cgi->param('comment'))
{
- # Prepend a string to the comment to let users know that the comment came from
- # the "edit attachment" screen.
- my $comment = qq|(From update of attachment $::FORM{'id'})\n| . $::FORM{'comment'};
-
- # Get the user's login name since the AppendComment function needs it.
- my $who = DBID_to_name($::userid);
- # Mention $::userid again so Perl doesn't give me a warning about it.
- my $neverused = $::userid;
+ # Prepend a string to the comment to let users know that the comment came
+ # from the "edit attachment" screen.
+ my $comment = qq|(From update of attachment $attach_id)\n| .
+ $cgi->param('comment');
# Append the comment to the list of comments in the database.
- AppendComment($bugid, $who, $comment, $::FORM{'isprivate'}, $timestamp);
+ AppendComment($bugid, $who, $comment, $cgi->param('isprivate'), $timestamp);
}
# Define the variables and functions that will be passed to the UI template.
- $vars->{'mailrecipients'} = { 'changer' => Bugzilla->user->login };
- $vars->{'attachid'} = $::FORM{'id'};
+ $vars->{'mailrecipients'} = { 'changer' => $who };
+ $vars->{'attachid'} = $attach_id;
$vars->{'bugid'} = $bugid;
print Bugzilla->cgi->header();