REGRESSION (r243153): [iOS] TestWebKitAPI.FocusPreservationTests.ChangingFocusedNodeR...
[WebKit-https.git] / Websites / bugs.webkit.org / attachment.cgi
index 3bf8922..c4ada4d 100755 (executable)
@@ -1,44 +1,19 @@
-#!/usr/bin/env perl -wT
-# -*- Mode: perl; indent-tabs-mode: nil -*-
+#!/usr/bin/perl -T
+# This Source Code Form is subject to the terms of the Mozilla Public
+# License, v. 2.0. If a copy of the MPL was not distributed with this
+# file, You can obtain one at http://mozilla.org/MPL/2.0/.
 #
-# The contents of this file are subject to the Mozilla Public
-# License Version 1.1 (the "License"); you may not use this file
-# except in compliance with the License. You may obtain a copy of
-# the License at http://www.mozilla.org/MPL/
-#
-# Software distributed under the License is distributed on an "AS
-# IS" basis, WITHOUT WARRANTY OF ANY KIND, either express or
-# implied. See the License for the specific language governing
-# rights and limitations under the License.
-#
-# The Original Code is the Bugzilla Bug Tracking System.
-#
-# The Initial Developer of the Original Code is Netscape Communications
-# Corporation. Portions created by Netscape are
-# Copyright (C) 1998 Netscape Communications Corporation. All
-# Rights Reserved.
-#
-# Contributor(s): Terry Weissman <terry@mozilla.org>
-#                 Myk Melez <myk@mozilla.org>
-#                 Daniel Raichle <draichle@gmx.net>
-#                 Dave Miller <justdave@syndicomm.com>
-#                 Alexander J. Vincent <ajvincent@juno.com>
-#                 Max Kanat-Alexander <mkanat@bugzilla.org>
-#                 Greg Hendricks <ghendricks@novell.com>
-#                 Frédéric Buclin <LpSolit@gmail.com>
-#                 Marc Schumann <wurblzap@gmail.com>
-#                 Byron Jones <bugzilla@glob.com.au>
+# This Source Code Form is "Incompatible With Secondary Licenses", as
+# defined by the Mozilla Public License, v. 2.0.
 
-################################################################################
-# Script Initialization
-################################################################################
-
-# Make it harder for us to do dangerous things in Perl.
+use 5.10.1;
 use strict;
+use warnings;
 
 use lib qw(. lib);
 
 use Bugzilla;
+use Bugzilla::BugMail;
 use Bugzilla::Constants;
 use Bugzilla::Error;
 use Bugzilla::Flag; 
@@ -46,12 +21,12 @@ use Bugzilla::FlagType;
 use Bugzilla::User;
 use Bugzilla::Util;
 use Bugzilla::Bug;
-use Bugzilla::Field;
 use Bugzilla::Attachment;
 use Bugzilla::Attachment::PatchReader;
 use Bugzilla::Token;
-use Bugzilla::Keyword;
 
+use Encode qw(encode find_encoding);
+use Encode::MIME::Header; # Required to alter Encode::Encoding{'MIME-Q'}.
 #if WEBKIT_CHANGES
 use Apache2::SubProcess ();
 use Apache2::RequestUtil ();
@@ -64,10 +39,7 @@ use Apache2::RequestUtil ();
 local our $cgi = Bugzilla->cgi;
 local our $template = Bugzilla->template;
 local our $vars = {};
-
-################################################################################
-# Main Body Execution
-################################################################################
+local $Bugzilla::CGI::ALLOW_UNSAFE_RESPONSE = 1;
 
 # All calls to this script should contain an "action" variable whose
 # value determines what the user wants to do.  The code below checks
@@ -76,25 +48,20 @@ local our $vars = {};
 
 # Determine whether to use the action specified by the user or the default.
 my $action = $cgi->param('action') || 'view';
+my $format = $cgi->param('format') || '';
 
 # You must use the appropriate urlbase/sslbase param when doing anything
-# but viewing an attachment.
-if ($action ne 'view') {
-    my $urlbase = Bugzilla->params->{'urlbase'};
-    my $sslbase = Bugzilla->params->{'sslbase'};
-    my $path_regexp = $sslbase ? qr/^(\Q$urlbase\E|\Q$sslbase\E)/ : qr/^\Q$urlbase\E/;
-    if (use_attachbase() && $cgi->self_url !~ /$path_regexp/) {
+# but viewing an attachment, or a raw diff.
+if ($action ne 'view'
+    && (($action !~ /^(?:interdiff|diff)$/) || $format ne 'raw'))
+{
+    do_ssl_redirect_if_required();
+    if ($cgi->url_is_attachment_base) {
         $cgi->redirect_to_urlbase;
     }
     Bugzilla->login();
 }
 
-# Determine if PatchReader is installed
-eval {
-    require PatchReader;
-    $vars->{'patchviewerinstalled'} = 1;
-};
-
 # When viewing an attachment, do not request credentials if we are on
 # the alternate host. Let view() decide when to call Bugzilla->login.
 if ($action eq "view")
@@ -136,10 +103,6 @@ elsif ($action eq "reviewform")
 {
     edit("reviewform");
 }
-elsif ($action eq "rietveldreview")
-{
-    edit("rietveldreview");
-}
 #endif // WEBKIT_CHANGES
 elsif ($action eq "update") 
 { 
@@ -157,7 +120,7 @@ elsif ($action eq "delete") {
 }
 else 
 { 
-  ThrowCodeError("unknown_action", { action => $action });
+  ThrowUserError('unknown_action', {action => $action});
 }
 
 exit;
@@ -197,11 +160,12 @@ sub validateID {
     # non-natural, so use the original value from $cgi in our exception
     # message here.
     detaint_natural($attach_id)
-     || ThrowUserError("invalid_attach_id", { attach_id => $cgi->param($param) });
+        || ThrowUserError("invalid_attach_id",
+                          { attach_id => scalar $cgi->param($param) });
   
     # Make sure the attachment exists in the database.
-    my $attachment = Bugzilla::Attachment->get($attach_id)
-      || ThrowUserError("invalid_attach_id", { attach_id => $attach_id });
+    my $attachment = new Bugzilla::Attachment({ id => $attach_id, cache => 1 })
+        || ThrowUserError("invalid_attach_id", { attach_id => $attach_id });
 
     return $attachment if ($dont_validate_access || check_can_access($attachment));
 }
@@ -212,10 +176,13 @@ sub check_can_access {
     my $user = Bugzilla->user;
 
     # Make sure the user is authorized to access this attachment's bug.
-    ValidateBugID($attachment->bug_id);
-    if ($attachment->isprivate && $user->id != $attachment->attacher->id && !$user->is_insider) {
+    Bugzilla::Bug->check({ id => $attachment->bug_id, cache => 1 });
+    if ($attachment->isprivate && $user->id != $attachment->attacher->id 
+        && !$user->is_insider) 
+    {
         ThrowUserError('auth_failure', {action => 'access',
-                                        object => 'attachment'});
+                                        object => 'attachment',
+                                        attach_id => $attachment->id});
     }
     return 1;
 }
@@ -235,88 +202,61 @@ sub attachmentIsPublic {
 # 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
-{
+sub validateFormat {
   # receives a list of legal formats; first item is a default
   my $format = $cgi->param('format') || $_[0];
-  if ( lsearch(\@_, $format) == -1)
-  {
+  if (not grep($_ eq $format, @_)) {
      ThrowUserError("invalid_format", { format  => $format, formats => \@_ });
   }
 
   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";
-  if ($context ne "file" && $context ne "patch") {
-    detaint_natural($context)
-      || ThrowUserError("invalid_context", { context => $cgi->param('context') });
-  }
+# Gets the attachment object(s) generated by validateID, while ensuring
+# attachbase and token authentication is used when required.
+sub get_attachment {
+    my @field_names = @_ ? @_ : qw(id);
 
-  return $context;
-}
-
-sub validateCanChangeBug
-{
-    my ($bugid) = @_;
-    my $dbh = Bugzilla->dbh;
-    my ($productid) = $dbh->selectrow_array(
-            "SELECT product_id
-             FROM bugs 
-             WHERE bug_id = ?", undef, $bugid);
-
-    Bugzilla->user->can_edit_product($productid)
-      || ThrowUserError("illegal_attachment_edit_bug",
-                        { bug_id => $bugid });
-}
-
-################################################################################
-# Functions
-################################################################################
-
-# Display an attachment.
-sub view {
-    my $attachment;
+    my %attachments;
 
     if (use_attachbase()) {
-        $attachment = validateID(undef, 1);
-        # Replace %bugid% by the ID of the bug the attachment belongs to, if present.
-        my $attachbase = Bugzilla->params->{'attachment_base'};
-        my $bug_id = $attachment->bug_id;
-        $attachbase =~ s/%bugid%/$bug_id/;
-        my $path = 'attachment.cgi?id=' . $attachment->id;
+        # Load each attachment, and ensure they are all from the same bug
+        my $bug_id = 0;
+        foreach my $field_name (@field_names) {
+            my $attachment = validateID($field_name, 1);
+            if (!$bug_id) {
+                $bug_id = $attachment->bug_id;
+            } elsif ($attachment->bug_id != $bug_id) {
+                ThrowUserError('attachment_bug_id_mismatch');
+            }
+            $attachments{$field_name} = $attachment;
+        }
+        my @args = map { $_ . '=' . $attachments{$_}->id } @field_names;
+        my $cgi_params = $cgi->canonicalise_query(@field_names, 't',
+            'Bugzilla_login', 'Bugzilla_password');
+        push(@args, $cgi_params) if $cgi_params;
+        my $path = 'attachment.cgi?' . join('&', @args);
 
         # Make sure the attachment is served from the correct server.
-        if ($cgi->self_url !~ /^\Q$attachbase\E/) {
-            # We couldn't call Bugzilla->login earlier as we first had to make sure
-            # we were not going to request credentials on the alternate host.
-            Bugzilla->login();
-            if (attachmentIsPublic($attachment)) {
-                # No need for a token; redirect to attachment base.
-                print $cgi->redirect(-location => $attachbase . $path);
-                exit;
-            } else {
-                # Make sure the user can view the attachment.
-                check_can_access($attachment);
-                # Create a token and redirect.
-                my $token = url_quote(issue_session_token($attachment->id));
-                print $cgi->redirect(-location => $attachbase . "$path&t=$token");
-                exit;
-            }
-        } else {
+        if ($cgi->url_is_attachment_base($bug_id)) {
             # No need to validate the token for public attachments. We cannot request
             # credentials as we are on the alternate host.
-            if (!attachmentIsPublic($attachment)) {
+            if (!all_attachments_are_public(\%attachments)) {
                 my $token = $cgi->param('t');
-                my ($userid, undef, $token_attach_id) = Bugzilla::Token::GetTokenData($token);
-                unless ($userid
-                        && detaint_natural($token_attach_id)
-                        && ($token_attach_id == $attachment->id))
-                {
+                my ($userid, undef, $token_data) = Bugzilla::Token::GetTokenData($token);
+                my %token_data = unpack_token_data($token_data);
+                my $valid_token = 1;
+                foreach my $field_name (@field_names) {
+                    my $token_id = $token_data{$field_name};
+                    if (!$token_id
+                        || !detaint_natural($token_id)
+                        || $attachments{$field_name}->id != $token_id)
+                    {
+                        $valid_token = 0;
+                        last;
+                    }
+                }
+                unless ($userid && $valid_token) {
                     # Not a valid token.
                     print $cgi->redirect('-location' => correct_urlbase() . $path);
                     exit;
@@ -327,11 +267,82 @@ sub view {
                 delete_token($token);
             }
         }
+        elsif ($cgi->url_is_attachment_base) {
+            # If we come here, this means that each bug has its own host
+            # for attachments, and that we are trying to view one attachment
+            # using another bug's host. That's not desired.
+            $cgi->redirect_to_urlbase;
+        }
+        else {
+            # We couldn't call Bugzilla->login earlier as we first had to
+            # make sure we were not going to request credentials on the
+            # alternate host.
+            Bugzilla->login();
+            my $attachbase = Bugzilla->params->{'attachment_base'};
+            # Replace %bugid% by the ID of the bug the attachment 
+            # belongs to, if present.
+            $attachbase =~ s/\%bugid\%/$bug_id/;
+            if (all_attachments_are_public(\%attachments)) {
+                # No need for a token; redirect to attachment base.
+                print $cgi->redirect(-location => $attachbase . $path);
+                exit;
+            } else {
+                # Make sure the user can view the attachment.
+                foreach my $field_name (@field_names) {
+                    check_can_access($attachments{$field_name});
+                }
+                # Create a token and redirect.
+                my $token = url_quote(issue_session_token(pack_token_data(\%attachments)));
+                print $cgi->redirect(-location => $attachbase . "$path&t=$token");
+                exit;
+            }
+        }
     } else {
+        do_ssl_redirect_if_required();
         # No alternate host is used. Request credentials if required.
         Bugzilla->login();
-        $attachment = validateID();
+        foreach my $field_name (@field_names) {
+            $attachments{$field_name} = validateID($field_name);
+        }
+    }
+
+    return wantarray
+        ? map { $attachments{$_} } @field_names
+        : $attachments{$field_names[0]};
+}
+
+sub all_attachments_are_public {
+    my $attachments = shift;
+    foreach my $field_name (keys %$attachments) {
+        if (!attachmentIsPublic($attachments->{$field_name})) {
+            return 0;
+        }
+    }
+    return 1;
+}
+
+sub pack_token_data {
+    my $attachments = shift;
+    return join(' ', map { $_ . '=' . $attachments->{$_}->id } keys %$attachments);
+}
+
+sub unpack_token_data {
+    my @token_data = split(/ /, shift || '');
+    my %data;
+    foreach my $token (@token_data) {
+        my ($field_name, $attach_id) = split('=', $token);
+        $data{$field_name} = $attach_id;
     }
+    return %data;
+}
+
+################################################################################
+# Functions
+################################################################################
+
+# Display an attachment.
+sub view {
+    my $attachment = get_attachment();
 
     # At this point, Bugzilla->login has been called if it had to.
     my $contenttype = $attachment->contenttype;
@@ -339,12 +350,8 @@ sub view {
 
     # Bug 111522: allow overriding content-type manually in the posted form
     # params.
-    if (defined $cgi->param('content_type'))
-    {
-        $cgi->param('contenttypemethod', 'manual');
-        $cgi->param('contenttypeentry', $cgi->param('content_type'));
-        Bugzilla::Attachment->validate_content_type(THROW_ERROR);
-        $contenttype = $cgi->param('content_type');
+    if (defined $cgi->param('content_type')) {
+        $contenttype = $attachment->_check_content_type($cgi->param('content_type'));
     }
 
     # Return the appropriate HTTP response headers.
@@ -355,8 +362,27 @@ sub view {
     $filename =~ s/\\/\\\\/g; # escape backslashes
     $filename =~ s/"/\\"/g; # escape quotes
 
+    # Avoid line wrapping done by Encode, which we don't need for HTTP
+    # headers. See discussion in bug 328628 for details.
+    local $Encode::Encoding{'MIME-Q'}->{'bpl'} = 10000;
+    $filename = encode('MIME-Q', $filename);
+
     my $disposition = Bugzilla->params->{'allow_attachment_display'} ? 'inline' : 'attachment';
 
+    # Don't send a charset header with attachments--they might not be UTF-8.
+    # However, we do allow people to explicitly specify a charset if they
+    # want.
+    if ($contenttype !~ /\bcharset=/i) {
+        # In order to prevent Apache from adding a charset, we have to send a
+        # charset that's a single space.
+        $cgi->charset(' ');
+        if (Bugzilla->feature('detect_charset') && $contenttype =~ /^text\//) {
+            my $encoding = detect_encoding($attachment->data);
+            if ($encoding) {
+                $cgi->charset(find_encoding($encoding)->mime_name);
+            }
+        }
+    }
     print $cgi->header(-type=>"$contenttype; name=\"$filename\"",
                        -content_disposition=> "$disposition; filename=\"$filename\"",
                        -content_length => $attachment->datasize);
@@ -366,13 +392,17 @@ sub view {
 
 sub interdiff {
     # Retrieve and validate parameters
-    my $old_attachment = validateID('oldid');
-    my $new_attachment = validateID('newid');
     my $format = validateFormat('html', 'raw');
-    my $context = validateContext();
+    my($old_attachment, $new_attachment);
+    if ($format eq 'raw') {
+        ($old_attachment, $new_attachment) = get_attachment('oldid', 'newid');
+    } else {
+        $old_attachment = validateID('oldid');
+        $new_attachment = validateID('newid');
+    }
 
     Bugzilla::Attachment::PatchReader::process_interdiff(
-        $old_attachment, $new_attachment, $format, $context);
+        $old_attachment, $new_attachment, $format);
 }
 
 #if WEBKIT_CHANGES
@@ -381,7 +411,6 @@ sub prettyPatch
     # Retrieve and validate parameters
     my $attachment = validateID();
     my $format = validateFormat('html', 'raw');
-    my $context = validateContext();
 
     # If it is not a patch, view normally.
     if (!$attachment->ispatch) {
@@ -411,9 +440,8 @@ sub prettyPatch
 
 sub diff {
     # Retrieve and validate parameters
-    my $attachment = validateID();
     my $format = validateFormat('html', 'raw');
-    my $context = validateContext();
+    my $attachment = $format eq 'raw' ? get_attachment() : validateID();
 
     # If it is not a patch, view normally.
     if (!$attachment->ispatch) {
@@ -421,18 +449,23 @@ sub diff {
         return;
     }
 
-    Bugzilla::Attachment::PatchReader::process_diff($attachment, $format, $context);
+    Bugzilla::Attachment::PatchReader::process_diff($attachment, $format);
 }
 
 # Display all attachments for a given bug in a series of IFRAMEs within one
 # HTML page.
 sub viewall {
     # Retrieve and validate parameters
-    my $bugid = $cgi->param('bugid');
-    ValidateBugID($bugid);
-    my $bug = new Bugzilla::Bug($bugid);
+    my $bug = Bugzilla::Bug->check({ id => scalar $cgi->param('bugid'), cache => 1 });
+
+    my $attachments = Bugzilla::Attachment->get_attachments_by_bug($bug);
+    # Ignore deleted attachments.
+    @$attachments = grep { $_->datasize } @$attachments;
 
-    my $attachments = Bugzilla::Attachment->get_attachments_by_bug($bugid);
+    if ($cgi->param('hide_obsolete')) {
+        @$attachments = grep { !$_->isobsolete } @$attachments;
+        $vars->{'hide_obsolete'} = 1;
+    }
 
     # Define the variables and functions that will be passed to the UI template.
     $vars->{'bug'} = $bug;
@@ -447,40 +480,51 @@ sub viewall {
 
 # Display a form for entering a new attachment.
 sub enter {
-  # Retrieve and validate parameters
-  my $bugid = $cgi->param('bugid');
-  ValidateBugID($bugid);
-  validateCanChangeBug($bugid);
-  my $dbh = Bugzilla->dbh;
-  my $user = Bugzilla->user;
-
-  my $bug = new Bugzilla::Bug($bugid, $user->id);
-  # 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 (!$user->in_group('editbugs', $bug->product_id)) {
-      $canEdit = "AND submitter_id = " . $user->id;
-  }
-  my $attach_ids = $dbh->selectcol_arrayref("SELECT attach_id FROM attachments
-                                             WHERE bug_id = ? AND isobsolete = 0 $canEdit
-                                             ORDER BY attach_id", undef, $bugid);
+    # Retrieve and validate parameters
+    my $bug = Bugzilla::Bug->check(scalar $cgi->param('bugid'));
+    my $bugid = $bug->id;
+    Bugzilla::Attachment->_check_bug($bug);
+    my $dbh = Bugzilla->dbh;
+    my $user = Bugzilla->user;
 
-  # Define the variables and functions that will be passed to the UI template.
-  $vars->{'bug'} = $bug;
-  $vars->{'attachments'} = Bugzilla::Attachment->get_list($attach_ids);
+    # 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 ($can_edit, $not_private) = ('', '');
+    if (!$user->in_group('editbugs', $bug->product_id)) {
+        $can_edit = "AND submitter_id = " . $user->id;
+    }
+    if (!$user->is_insider) {
+        $not_private = "AND isprivate = 0";
+    }
+    my $attach_ids = $dbh->selectcol_arrayref(
+        "SELECT attach_id
+           FROM attachments
+          WHERE bug_id = ?
+                AND isobsolete = 0
+                $can_edit $not_private
+       ORDER BY attach_id",
+         undef, $bugid);
+
+    # Define the variables and functions that will be passed to the UI template.
+    $vars->{'bug'} = $bug;
+    $vars->{'attachments'} = Bugzilla::Attachment->new_from_list($attach_ids);
 
-  my $flag_types = Bugzilla::FlagType::match({'target_type'  => 'attachment',
-                                              'product_id'   => $bug->product_id,
-                                              'component_id' => $bug->component_id});
-  $vars->{'flag_types'} = $flag_types;
-  $vars->{'any_flags_requesteeble'} = grep($_->is_requesteeble, @$flag_types);
-  $vars->{'token'} = issue_session_token('createattachment:');
+    my $flag_types = Bugzilla::FlagType::match({
+        'target_type'  => 'attachment',
+        'product_id'   => $bug->product_id,
+        'component_id' => $bug->component_id
+    });
+    $vars->{'flag_types'} = $flag_types;
+    $vars->{'any_flags_requesteeble'} =
+        grep { $_->is_requestable && $_->is_requesteeble } @$flag_types;
+    $vars->{'token'} = issue_session_token('create_attachment');
 
-  print $cgi->header();
+    print $cgi->header();
 
-  # Generate and return the UI (HTML page) from the appropriate template.
-  $template->process("attachment/create.html.tmpl", $vars)
-    || ThrowTemplateError($template->error());
+    # Generate and return the UI (HTML page) from the appropriate template.
+    $template->process("attachment/create.html.tmpl", $vars)
+      || ThrowTemplateError($template->error());
 }
 
 # Insert a new attachment into the database.
@@ -491,89 +535,102 @@ sub insert {
     $dbh->bz_start_transaction;
 
     # Retrieve and validate parameters
-    my $bugid = $cgi->param('bugid');
-    ValidateBugID($bugid);
-    validateCanChangeBug($bugid);
-    my ($timestamp) = Bugzilla->dbh->selectrow_array("SELECT NOW()");
+    my $bug = Bugzilla::Bug->check(scalar $cgi->param('bugid'));
+    my $bugid = $bug->id;
+    my ($timestamp) = $dbh->selectrow_array("SELECT NOW()");
 
     # Detect if the user already used the same form to submit an attachment
     my $token = trim($cgi->param('token'));
-    if ($token) {
-        my ($creator_id, $date, $old_attach_id) = Bugzilla::Token::GetTokenData($token);
-        unless ($creator_id 
-            && ($creator_id == $user->id) 
-                && ($old_attach_id =~ "^createattachment:")) 
+    check_token_data($token, 'create_attachment', 'index.cgi');
+
+    # Check attachments the user tries to mark as obsolete.
+    my @obsolete_attachments;
+    if ($cgi->param('obsolete')) {
+        my @obsolete = $cgi->param('obsolete');
+        @obsolete_attachments = Bugzilla::Attachment->validate_obsolete($bug, \@obsolete);
+    }
+
+    # Must be called before create() as it may alter $cgi->param('ispatch').
+    my $content_type = Bugzilla::Attachment::get_content_type();
+
+    # Get the filehandle of the attachment.
+    my $data_fh = $cgi->upload('data');
+    my $attach_text = $cgi->param('attach_text');
+
+    my $attachment = Bugzilla::Attachment->create(
+        {bug           => $bug,
+         creation_ts   => $timestamp,
+         data          => $attach_text || $data_fh,
+         description   => scalar $cgi->param('description'),
+         filename      => $attach_text ? "file_$bugid.txt" : $data_fh,
+         ispatch       => scalar $cgi->param('ispatch'),
+         isprivate     => scalar $cgi->param('isprivate'),
+         mimetype      => $content_type,
+         });
+
+    # Delete the token used to create this attachment.
+    delete_token($token);
+
+    foreach my $obsolete_attachment (@obsolete_attachments) {
+        $obsolete_attachment->set_is_obsolete(1);
+        $obsolete_attachment->update($timestamp);
+    }
+
+    my ($flags, $new_flags) = Bugzilla::Flag->extract_flags_from_cgi(
+                                  $bug, $attachment, $vars, SKIP_REQUESTEE_ON_ERROR);
+    $attachment->set_flags($flags, $new_flags);
+
+    # Insert a comment about the new attachment into the database.
+    my $comment = $cgi->param('comment');
+    $comment = '' unless defined $comment;
+    $bug->add_comment($comment, { isprivate => $attachment->isprivate,
+                                  type => CMT_ATTACHMENT_CREATED,
+                                  extra_data => $attachment->id });
+
+    # Assign the bug to the user, if they are allowed to take it
+    my $owner = "";
+    if ($cgi->param('takebug') && $user->in_group('editbugs', $bug->product_id)) {
+        # When taking a bug, we have to follow the workflow.
+        my $bug_status = $cgi->param('bug_status') || '';
+        ($bug_status) = grep { $_->name eq $bug_status }
+                        @{ $bug->status->can_change_to };
+
+        if ($bug_status && $bug_status->is_open
+            && ($bug_status->name ne 'UNCONFIRMED'
+                || $bug->product_obj->allows_unconfirmed))
         {
-            # The token is invalid.
-            ThrowUserError('token_does_not_exist');
-        }
-    
-        $old_attach_id =~ s/^createattachment://;
-   
-        if ($old_attach_id) {
-            $vars->{'bugid'} = $bugid;
-            $vars->{'attachid'} = $old_attach_id;
-            print $cgi->header();
-            $template->process("attachment/cancel-create-dupe.html.tmpl",  $vars)
-                || ThrowTemplateError($template->error());
-            exit;
+            $bug->set_bug_status($bug_status->name);
+            $bug->clear_resolution();
         }
+        # Make sure the person we are taking the bug from gets mail.
+        $owner = $bug->assigned_to->login;
+        $bug->set_assigned_to($user);
     }
 
-    my $bug = new Bugzilla::Bug($bugid);
-    my $attachment =
-        Bugzilla::Attachment->insert_attachment_for_bug(THROW_ERROR, $bug, $user,
-                                                        $timestamp, $vars);
+    $bug->add_cc($user) if $cgi->param('addselfcc');
+    $bug->update($timestamp);
 
-    # Insert a comment about the new attachment into the database.
-    my $comment = "Created an attachment (id=" . $attachment->id . ")\n" .
-                  $attachment->description . "\n";
-    $comment .= ("\n" . $cgi->param('comment')) if defined $cgi->param('comment');
-
-    $bug->add_comment($comment, { isprivate => $attachment->isprivate });
-
-  # Assign the bug to the user, if they are allowed to take it
-  my $owner = "";
-  if ($cgi->param('takebug') && $user->in_group('editbugs', $bug->product_id)) {
-      # When taking a bug, we have to follow the workflow.
-      my $bug_status = $cgi->param('bug_status') || '';
-      ($bug_status) = grep {$_->name eq $bug_status} @{$bug->status->can_change_to};
-
-      if ($bug_status && $bug_status->is_open
-          && ($bug_status->name ne 'UNCONFIRMED' || $bug->product_obj->votes_to_confirm))
-      {
-          $bug->set_status($bug_status->name);
-          $bug->clear_resolution();
-      }
-      # Make sure the person we are taking the bug from gets mail.
-      $owner = $bug->assigned_to->login;
-      $bug->set_assigned_to($user);
-  }
-  $bug->update($timestamp);
+    # We have to update the attachment after updating the bug, to ensure new
+    # comments are available.
+    $attachment->update($timestamp);
 
-  if ($token) {
-      trick_taint($token);
-      $dbh->do('UPDATE tokens SET eventdata = ? WHERE token = ?', undef,
-               ("createattachment:" . $attachment->id, $token));
-  }
+    $dbh->bz_commit_transaction;
 
-  $dbh->bz_commit_transaction;
+    # Define the variables and functions that will be passed to the UI template.
+    $vars->{'attachment'} = $attachment;
+    # We cannot reuse the $bug object as delta_ts has eventually been updated
+    # since the object was created.
+    $vars->{'bugs'} = [new Bugzilla::Bug($bugid)];
+    $vars->{'header_done'} = 1;
+    $vars->{'contenttypemethod'} = $cgi->param('contenttypemethod');
 
-  # Define the variables and functions that will be passed to the UI template.
-  $vars->{'mailrecipients'} =  { 'changer' => $user->login,
-                                 'owner'   => $owner };
-  $vars->{'attachment'} = $attachment;
-  # We cannot reuse the $bug object as delta_ts has eventually been updated
-  # since the object was created.
-  $vars->{'bugs'} = [new Bugzilla::Bug($bugid)];
-  $vars->{'header_done'} = 1;
-  $vars->{'contenttypemethod'} = $cgi->param('contenttypemethod');
-  $vars->{'use_keywords'} = 1 if Bugzilla::Keyword::keyword_count();
+    my $recipients = { 'changer' => $user, 'owner' => $owner };
+    $vars->{'sent_bugmail'} = Bugzilla::BugMail::Send($bugid, $recipients);
 
-  print $cgi->header();
-  # Generate and return the UI (HTML page) from the appropriate template.
-  $template->process("attachment/created.html.tmpl", $vars)
-    || ThrowTemplateError($template->error());
+    print $cgi->header();
+    # Generate and return the UI (HTML page) from the appropriate template.
+    $template->process("attachment/created.html.tmpl", $vars)
+        || ThrowTemplateError($template->error());
 }
 
 # Displays a form for editing attachment properties.
@@ -582,96 +639,88 @@ sub insert {
 # Validations are done later when the user submits changes.
 sub edit {
 #if WEBKIT_CHANGES
-  my ($template_name) = @_;
-  $template_name = $template_name || "edit";
+    my ($template_name) = @_;
+    $template_name = $template_name || "edit";
 #endif // WEBKIT_CHANGES
 
-  my $attachment = validateID();
-  my $dbh = Bugzilla->dbh;
-
-  # Retrieve a list of attachments for this bug as well as a summary of the bug
-  # to use in a navigation bar across the top of the screen.
-  my $bugattachments =
-      Bugzilla::Attachment->get_attachments_by_bug($attachment->bug_id);
-  # We only want attachment IDs.
-  @$bugattachments = map { $_->id } @$bugattachments;
-
-  my ($bugsummary, $product_id, $component_id) =
-      $dbh->selectrow_array('SELECT short_desc, product_id, component_id
-                               FROM bugs
-                              WHERE bug_id = ?', undef, $attachment->bug_id);
-
-  # Get a list of flag types that can be set for this attachment.
-  my $flag_types = Bugzilla::FlagType::match({ 'target_type'  => 'attachment' ,
-                                               'product_id'   => $product_id ,
-                                               'component_id' => $component_id });
-  foreach my $flag_type (@$flag_types) {
-    $flag_type->{'flags'} = Bugzilla::Flag->match({ 'type_id'   => $flag_type->id,
-                                                    'attach_id' => $attachment->id });
-  }
-  $vars->{'flag_types'} = $flag_types;
-  $vars->{'any_flags_requesteeble'} = grep($_->is_requesteeble, @$flag_types);
-  $vars->{'attachment'} = $attachment;
-  $vars->{'bugsummary'} = $bugsummary; 
-  $vars->{'attachments'} = $bugattachments;
+    my $attachment = validateID();
+
+    my $bugattachments =
+        Bugzilla::Attachment->get_attachments_by_bug($attachment->bug);
+
+    my $any_flags_requesteeble = grep { $_->is_requestable && $_->is_requesteeble }
+                                 @{ $attachment->flag_types };
+    # Useful in case a flagtype is no longer requestable but a requestee
+    # has been set before we turned off that bit.
+    $any_flags_requesteeble ||= grep { $_->requestee_id } @{ $attachment->flags };
+    $vars->{'any_flags_requesteeble'} = $any_flags_requesteeble;
+    $vars->{'attachment'} = $attachment;
+    $vars->{'attachments'} = $bugattachments;
 
 #if WEBKIT_CHANGES
-  if ($attachment->ispatch) {
-      my $quotedpatch = $attachment->data;
-      $quotedpatch =~ s/^/> /mg;
-      $vars->{'quotedpatch'} = $quotedpatch;
-  }
+    if ($attachment->ispatch) {
+        my $quotedpatch = $attachment->data;
+        $quotedpatch =~ s/^/> /mg;
+        $vars->{'quotedpatch'} = $quotedpatch;
+    }
 #endif // WEBKIT_CHANGES
 
-  print $cgi->header();
+    print $cgi->header();
 
-  # Generate and return the UI (HTML page) from the appropriate template.
-  $template->process("attachment/$template_name.html.tmpl", $vars) # WEBKIT_CHANGES
-    || ThrowTemplateError($template->error());
+    # Generate and return the UI (HTML page) from the appropriate template.
+    $template->process("attachment/${template_name}.html.tmpl", $vars) # WEBKIT_CHANGES
+        || ThrowTemplateError($template->error());
 }
 
-# Updates 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.
+# Updates an attachment record. Only users with "editbugs" privileges,
+# (or the original attachment's submitter) can edit the attachment.
 # Users cannot edit the content of the attachment itself.
 sub update {
     my $user = Bugzilla->user;
     my $dbh = Bugzilla->dbh;
 
+    # Start a transaction in preparation for updating the attachment.
+    $dbh->bz_start_transaction();
+
     # Retrieve and validate parameters
     my $attachment = validateID();
-    my $bug = new Bugzilla::Bug($attachment->bug_id);
-    $attachment->validate_can_edit($bug->product_id);
-    validateCanChangeBug($bug->id);
-    Bugzilla::Attachment->validate_description(THROW_ERROR);
-    Bugzilla::Attachment->validate_is_patch(THROW_ERROR);
-    Bugzilla::Attachment->validate_content_type(THROW_ERROR) unless $cgi->param('ispatch');
-    $cgi->param('isobsolete', $cgi->param('isobsolete') ? 1 : 0);
-    $cgi->param('isprivate', $cgi->param('isprivate') ? 1 : 0);
-
-    # Now make sure the attachment has not been edited since we loaded the page.
-    if (defined $cgi->param('delta_ts')
-        && $cgi->param('delta_ts') ne $attachment->modification_time)
-    {
-        ($vars->{'operations'}) =
-            Bugzilla::Bug::GetBugActivity($bug->id, $attachment->id, $cgi->param('delta_ts'));
-
-        # The token contains the old modification_time. We need a new one.
-        $cgi->param('token', issue_hash_token([$attachment->id, $attachment->modification_time]));
-
-        # If the modification date changed but there is no entry in
-        # the activity table, this means someone commented only.
-        # In this case, there is no reason to midair.
-        if (scalar(@{$vars->{'operations'}})) {
-            $cgi->param('delta_ts', $attachment->modification_time);
-            $vars->{'attachment'} = $attachment;
-
-            print $cgi->header();
-            # Warn the user about the mid-air collision and ask them what to do.
-            $template->process("attachment/midair.html.tmpl", $vars)
-              || ThrowTemplateError($template->error());
-            exit;
+    my $bug = $attachment->bug;
+    $attachment->_check_bug;
+    my $can_edit = $attachment->validate_can_edit;
+
+    if ($can_edit) {
+        $attachment->set_description(scalar $cgi->param('description'));
+        $attachment->set_is_patch(scalar $cgi->param('ispatch'));
+        $attachment->set_content_type(scalar $cgi->param('contenttypeentry'));
+        $attachment->set_is_obsolete(scalar $cgi->param('isobsolete'));
+        $attachment->set_is_private(scalar $cgi->param('isprivate'));
+        $attachment->set_filename(scalar $cgi->param('filename'));
+
+        # Now make sure the attachment has not been edited since we loaded the page.
+        my $delta_ts = $cgi->param('delta_ts');
+        my $modification_time = $attachment->modification_time;
+
+        if ($delta_ts && $delta_ts ne $modification_time) {
+            datetime_from($delta_ts)
+              or ThrowCodeError('invalid_timestamp', { timestamp => $delta_ts });
+            ($vars->{'operations'}) = $bug->get_activity($attachment->id, $delta_ts);
+
+            # If the modification date changed but there is no entry in
+            # the activity table, this means someone commented only.
+            # In this case, there is no reason to midair.
+            if (scalar(@{$vars->{'operations'}})) {
+                $cgi->param('delta_ts', $modification_time);
+                # The token contains the old modification_time. We need a new one.
+                $cgi->param('token', issue_hash_token([$attachment->id, $modification_time]));
+
+                $vars->{'attachment'} = $attachment;
+
+                print $cgi->header();
+                # Warn the user about the mid-air collision and ask them what to do.
+                $template->process("attachment/midair.html.tmpl", $vars)
+                  || ThrowTemplateError($template->error());
+                exit;
+            }
         }
     }
 
@@ -680,128 +729,75 @@ sub update {
     my $token = $cgi->param('token');
     check_hash_token($token, [$attachment->id, $attachment->modification_time]);
 
-    # If the submitter of the attachment is not in the insidergroup,
-    # be sure that he cannot overwrite the private bit.
-    # This check must be done before calling Bugzilla::Flag*::validate(),
-    # because they will look at the private bit when checking permissions.
-    # XXX - This is a ugly hack. Ideally, we shouldn't have to look at the
-    # old private bit twice (first here, and then below again), but this is
-    # the less risky change.
-    unless ($user->is_insider) {
-        $cgi->param('isprivate', $attachment->isprivate);
-    }
-
     # If the user submitted a comment while editing the attachment,
     # add the comment to the bug. Do this after having validated isprivate!
-    if ($cgi->param('comment')) {
-        # Prepend a string to the comment to let users know that the comment came
-        # from the "edit attachment" screen.
-        my $comment = "(From update of attachment " . $attachment->id . ")\n" .
-                      $cgi->param('comment');
+    my $comment = $cgi->param('comment');
+    if (defined $comment && trim($comment) ne '') {
+        $bug->add_comment($comment, { isprivate => $attachment->isprivate,
+                                      type => CMT_ATTACHMENT_UPDATED,
+                                      extra_data => $attachment->id });
+    }
 
-        $bug->add_comment($comment, { isprivate => $cgi->param('isprivate') });
+    $bug->add_cc($user) if $cgi->param('addselfcc');
+
+    my ($flags, $new_flags) =
+      Bugzilla::Flag->extract_flags_from_cgi($bug, $attachment, $vars);
+
+    if ($can_edit) {
+        $attachment->set_flags($flags, $new_flags);
     }
+    # Requestees can set flags targetted to them, even if they cannot
+    # edit the attachment. Flag setters can edit their own flags too.
+    elsif (scalar @$flags) {
+        my %flag_list = map { $_->{id} => $_ } @$flags;
+        my $flag_objs = Bugzilla::Flag->new_from_list([keys %flag_list]);
+
+        my @editable_flags;
+        foreach my $flag_obj (@$flag_objs) {
+            if ($flag_obj->setter_id == $user->id
+                || ($flag_obj->requestee_id && $flag_obj->requestee_id == $user->id))
+            {
+                push(@editable_flags, $flag_list{$flag_obj->id});
+            }
+        }
 
-    # 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($bug->id, $attachment->id);
+        if (scalar @editable_flags) {
+            $attachment->set_flags(\@editable_flags, []);
+            # Flag changes must be committed.
+            $can_edit = 1;
+        }
+    }
 
-    # Start a transaction in preparation for updating the attachment.
-    $dbh->bz_start_transaction();
+    # Figure out when the changes were made.
+    my $timestamp = $dbh->selectrow_array('SELECT LOCALTIMESTAMP(0)');
 
-  # Quote the description and content type for use in the SQL UPDATE statement.
-  my $description = $cgi->param('description');
-  my $contenttype = $cgi->param('contenttype');
-  my $filename = $cgi->param('filename');
-  # we can detaint this way thanks to placeholders
-  trick_taint($description);
-  trick_taint($contenttype);
-  trick_taint($filename);
-
-  # Figure out when the changes were made.
-  my ($timestamp) = $dbh->selectrow_array("SELECT NOW()");
-    
-  # Update flags.  We have to do this before committing changes
-  # 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.
-  Bugzilla::Flag->process($bug, $attachment, $timestamp, $vars);
-
-  # Update the attachment record in the database.
-  $dbh->do("UPDATE  attachments 
-            SET     description = ?,
-                    mimetype    = ?,
-                    filename    = ?,
-                    ispatch     = ?,
-                    isobsolete  = ?,
-                    isprivate   = ?,
-                    modification_time = ?
-            WHERE   attach_id   = ?",
-            undef, ($description, $contenttype, $filename,
-            $cgi->param('ispatch'), $cgi->param('isobsolete'), 
-            $cgi->param('isprivate'), $timestamp, $attachment->id));
-
-  my $updated_attachment = Bugzilla::Attachment->get($attachment->id);
-  # Record changes in the activity table.
-  my $sth = $dbh->prepare('INSERT INTO bugs_activity (bug_id, attach_id, who, bug_when,
-                                                      fieldid, removed, added)
-                           VALUES (?, ?, ?, ?, ?, ?, ?)');
-
-  if ($attachment->description ne $updated_attachment->description) {
-    my $fieldid = get_field_id('attachments.description');
-    $sth->execute($bug->id, $attachment->id, $user->id, $timestamp, $fieldid,
-                  $attachment->description, $updated_attachment->description);
-  }
-  if ($attachment->contenttype ne $updated_attachment->contenttype) {
-    my $fieldid = get_field_id('attachments.mimetype');
-    $sth->execute($bug->id, $attachment->id, $user->id, $timestamp, $fieldid,
-                  $attachment->contenttype, $updated_attachment->contenttype);
-  }
-  if ($attachment->filename ne $updated_attachment->filename) {
-    my $fieldid = get_field_id('attachments.filename');
-    $sth->execute($bug->id, $attachment->id, $user->id, $timestamp, $fieldid,
-                  $attachment->filename, $updated_attachment->filename);
-  }
-  if ($attachment->ispatch != $updated_attachment->ispatch) {
-    my $fieldid = get_field_id('attachments.ispatch');
-    $sth->execute($bug->id, $attachment->id, $user->id, $timestamp, $fieldid,
-                  $attachment->ispatch, $updated_attachment->ispatch);
-  }
-  if ($attachment->isobsolete != $updated_attachment->isobsolete) {
-    my $fieldid = get_field_id('attachments.isobsolete');
-    $sth->execute($bug->id, $attachment->id, $user->id, $timestamp, $fieldid,
-                  $attachment->isobsolete, $updated_attachment->isobsolete);
-  }
-  if ($attachment->isprivate != $updated_attachment->isprivate) {
-    my $fieldid = get_field_id('attachments.isprivate');
-    $sth->execute($bug->id, $attachment->id, $user->id, $timestamp, $fieldid,
-                  $attachment->isprivate, $updated_attachment->isprivate);
-  }
-  
-  # Commit the transaction now that we are finished updating the database.
-  $dbh->bz_commit_transaction();
+    # Commit the comment, if any.
+    # This has to happen before updating the attachment, to ensure new comments
+    # are available to $attachment->update.
+    $bug->update($timestamp);
 
-  # Commit the comment, if any.
-  $bug->update();
+    if ($can_edit) {
+        my $changes = $attachment->update($timestamp);
+        # If there are changes, we updated delta_ts in the DB. We have to
+        # reflect this change in the bug object.
+        $bug->{delta_ts} = $timestamp if scalar(keys %$changes);
+    }
 
-  # Define the variables and functions that will be passed to the UI template.
-  $vars->{'mailrecipients'} = { 'changer' => Bugzilla->user->login };
-  $vars->{'attachment'} = $attachment;
-  # We cannot reuse the $bug object as delta_ts has eventually been updated
-  # since the object was created.
-  $vars->{'bugs'} = [new Bugzilla::Bug($bug->id)];
-  $vars->{'header_done'} = 1;
-  $vars->{'use_keywords'} = 1 if Bugzilla::Keyword::keyword_count();
+    # Commit the transaction now that we are finished updating the database.
+    $dbh->bz_commit_transaction();
+
+    # Define the variables and functions that will be passed to the UI template.
+    $vars->{'attachment'} = $attachment;
+    $vars->{'bugs'} = [$bug];
+    $vars->{'header_done'} = 1;
+    $vars->{'sent_bugmail'} = 
+        Bugzilla::BugMail::Send($bug->id, { 'changer' => $user });
 
-  print $cgi->header();
+    print $cgi->header();
 
-  # Generate and return the UI (HTML page) from the appropriate template.
-  $template->process("attachment/updated.html.tmpl", $vars)
-    || ThrowTemplateError($template->error());
+    # Generate and return the UI (HTML page) from the appropriate template.
+    $template->process("attachment/updated.html.tmpl", $vars)
+      || ThrowTemplateError($template->error());
 }
 
 # Only administrators can delete attachments.
@@ -821,7 +817,7 @@ sub delete_attachment {
 
     # Make sure the administrator is allowed to edit this attachment.
     my $attachment = validateID();
-    validateCanChangeBug($attachment->bug_id);
+    Bugzilla::Attachment->_check_bug($attachment->bug);
 
     $attachment->datasize || ThrowUserError('attachment_removed');
 
@@ -831,7 +827,7 @@ sub delete_attachment {
         my ($creator_id, $date, $event) = Bugzilla::Token::GetTokenData($token);
         unless ($creator_id
                   && ($creator_id == $user->id)
-                  && ($event eq 'attachment' . $attachment->id))
+                  && ($event eq 'delete_attachment' . $attachment->id))
         {
             # The token is invalid.
             ThrowUserError('token_does_not_exist');
@@ -842,9 +838,7 @@ sub delete_attachment {
         # The token is valid. Delete the content of the attachment.
         my $msg;
         $vars->{'attachment'} = $attachment;
-        $vars->{'date'} = $date;
         $vars->{'reason'} = clean_text($cgi->param('reason') || '');
-        $vars->{'mailrecipients'} = { 'changer' => $user->login };
 
         $template->process("attachment/delete_reason.txt.tmpl", $vars, \$msg)
           || ThrowTemplateError($template->error());
@@ -852,10 +846,6 @@ sub delete_attachment {
         # Paste the reason provided by the admin into a comment.
         $bug->add_comment($msg);
 
-        # If the attachment is stored locally, remove it.
-        if (-e $attachment->_get_local_filename) {
-            unlink $attachment->_get_local_filename;
-        }
         $attachment->remove_from_db();
 
         # Now delete the token.
@@ -867,14 +857,16 @@ sub delete_attachment {
         # Required to display the bug the deleted attachment belongs to.
         $vars->{'bugs'} = [$bug];
         $vars->{'header_done'} = 1;
-        $vars->{'use_keywords'} = 1 if Bugzilla::Keyword::keyword_count();
+
+        $vars->{'sent_bugmail'} =
+            Bugzilla::BugMail::Send($bug->id, { 'changer' => $user });
 
         $template->process("attachment/updated.html.tmpl", $vars)
           || ThrowTemplateError($template->error());
     }
     else {
         # Create a token.
-        $token = issue_session_token('attachment' . $attachment->id);
+        $token = issue_session_token('delete_attachment' . $attachment->id);
 
         $vars->{'a'} = $attachment;
         $vars->{'token'} = $token;