diff options
| -rwxr-xr-x | attachment.cgi | 546 |
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(); |