Bug 779862: shift PatchReader into bugzilla namespace and fix long standing issues

13 files changed, 1150 insertions, 20 deletions

diff --git a/Bugzilla/Attachment/PatchReader.pm b/Bugzilla/Attachment/PatchReader.pm
index 01a624a8f..152b210d6 100644
--- a/Bugzilla/Attachment/PatchReader.pm
+++ b/Bugzilla/Attachment/PatchReader.pm
@@ -33,8 +33,8 @@ sub process_diff {
     my ($reader, $last_reader) = setup_patch_readers(undef, $context);
 
     if ($format eq 'raw') {
-        require PatchReader::DiffPrinter::raw;
-        $last_reader->sends_data_to(new PatchReader::DiffPrinter::raw());
+        require Bugzilla::PatchReader::DiffPrinter::raw;
+        $last_reader->sends_data_to(new Bugzilla::PatchReader::DiffPrinter::raw());
         # Actually print out the patch.
         print $cgi->header(-type => 'text/plain',
                            -x_content_type_options => "nosniff",
@@ -115,8 +115,8 @@ sub process_interdiff {
     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());
+        require Bugzilla::PatchReader::DiffPrinter::raw;
+        $last_reader->sends_data_to(new Bugzilla::PatchReader::DiffPrinter::raw());
         # Actually print out the patch.
         print $cgi->header(-type => 'text/plain',
                            -x_content_type_options => "nosniff",
@@ -154,29 +154,29 @@ sub get_unified_diff {
     my ($attachment, $format) = @_;
 
     # Bring in the modules we need.
-    require PatchReader::Raw;
-    require PatchReader::FixPatchRoot;
-    require PatchReader::DiffPrinter::raw;
-    require PatchReader::PatchInfoGrabber;
+    require Bugzilla::PatchReader::Raw;
+    require Bugzilla::PatchReader::FixPatchRoot;
+    require Bugzilla::PatchReader::DiffPrinter::raw;
+    require Bugzilla::PatchReader::PatchInfoGrabber;
     require File::Temp;
 
     $attachment->ispatch
       || ThrowCodeError('must_be_patch', { 'attach_id' => $attachment->id });
 
     # Reads in the patch, converting to unified diff in a temp file.
-    my $reader = new PatchReader::Raw;
+    my $reader = new Bugzilla::PatchReader::Raw;
     my $last_reader = $reader;
 
     # Fixes patch root (makes canonical if possible).
     if (Bugzilla->params->{'cvsroot'}) {
         my $fix_patch_root =
-            new PatchReader::FixPatchRoot(Bugzilla->params->{'cvsroot'});
+            new Bugzilla::PatchReader::FixPatchRoot(Bugzilla->params->{'cvsroot'});
         $last_reader->sends_data_to($fix_patch_root);
         $last_reader = $fix_patch_root;
     }
 
     # Grabs the patch file info.
-    my $patch_info_grabber = new PatchReader::PatchInfoGrabber();
+    my $patch_info_grabber = new Bugzilla::PatchReader::PatchInfoGrabber();
     $last_reader->sends_data_to($patch_info_grabber);
     $last_reader = $patch_info_grabber;
 
@@ -186,7 +186,7 @@ sub get_unified_diff {
         # The HTML page will be displayed with the UTF-8 encoding.
         binmode $fh, ':utf8';
     }
-    my $raw_printer = new PatchReader::DiffPrinter::raw($fh);
+    my $raw_printer = new Bugzilla::PatchReader::DiffPrinter::raw($fh);
     $last_reader->sends_data_to($raw_printer);
     $last_reader = $raw_printer;
 
@@ -230,13 +230,13 @@ sub setup_patch_readers {
 
     # Define the patch readers.
     # The reader that reads the patch in (whatever its format).
-    require PatchReader::Raw;
-    my $reader = new PatchReader::Raw;
+    require Bugzilla::PatchReader::Raw;
+    my $reader = new Bugzilla::PatchReader::Raw;
     my $last_reader = $reader;
     # Fix the patch root if we have a cvs root.
     if (Bugzilla->params->{'cvsroot'}) {
-        require PatchReader::FixPatchRoot;
-        $last_reader->sends_data_to(new PatchReader::FixPatchRoot(Bugzilla->params->{'cvsroot'}));
+        require Bugzilla::PatchReader::FixPatchRoot;
+        $last_reader->sends_data_to(new Bugzilla::PatchReader::FixPatchRoot(Bugzilla->params->{'cvsroot'}));
         $last_reader->sends_data_to->diff_root($diff_root) if defined($diff_root);
         $last_reader = $last_reader->sends_data_to;
     }
@@ -245,12 +245,12 @@ sub setup_patch_readers {
     if ($context ne 'patch' && Bugzilla->localconfig->{cvsbin} 
         && Bugzilla->params->{'cvsroot_get'}) 
     {
-        require PatchReader::AddCVSContext;
+        require Bugzilla::PatchReader::AddCVSContext;
         # We need to set $cvsbin as global, because PatchReader::CVSClient
         # needs it in order to find 'cvs'.
         $main::cvsbin = Bugzilla->localconfig->{cvsbin};
         $last_reader->sends_data_to(
-          new PatchReader::AddCVSContext($context, Bugzilla->params->{'cvsroot_get'}));
+          new Bugzilla::PatchReader::AddCVSContext($context, Bugzilla->params->{'cvsroot_get'}));
         $last_reader = $last_reader->sends_data_to;
     }
 
@@ -262,7 +262,7 @@ sub setup_template_patch_reader {
     my $cgi = Bugzilla->cgi;
     my $template = Bugzilla->template;
 
-    require PatchReader::DiffPrinter::template;
+    require Bugzilla::PatchReader::DiffPrinter::template;
 
     # Define the vars for templates.
     if (defined $cgi->param('headers')) {
@@ -281,7 +281,7 @@ sub setup_template_patch_reader {
     print $cgi->header(-type => 'text/html',
                        -expires => '+3M');
 
-    $last_reader->sends_data_to(new PatchReader::DiffPrinter::template($template,
+    $last_reader->sends_data_to(new Bugzilla::PatchReader::DiffPrinter::template($template,
                                 "attachment/diff-header.$format.tmpl",
                                 "attachment/diff-file.$format.tmpl",
                                 "attachment/diff-footer.$format.tmpl",
diff --git a/Bugzilla/PatchReader.pm b/Bugzilla/PatchReader.pm
new file mode 100644
index 000000000..b5c3b957b
--- /dev/null
+++ b/Bugzilla/PatchReader.pm
@@ -0,0 +1,117 @@
+package Bugzilla::PatchReader;
+
+use strict;
+
+=head1 NAME
+
+PatchReader - Utilities to read and manipulate patches and CVS
+
+=head1 SYNOPSIS
+
+  # Script that reads in a patch (in any known format), and prints
+  # out some information about it.  Other common operations are
+  # outputting the patch in a raw unified diff format, outputting
+  # the patch information to Template::Toolkit templates, adding
+  # context to a patch from CVS, and narrowing the patch down to
+  # apply only to a single file or set of files.
+
+  use PatchReader::Raw;
+  use PatchReader::PatchInfoGrabber;
+  my $filename = 'filename.patch';
+
+  # Create the reader that parses the patch and the object that
+  # extracts info from the reader's datastream
+  my $reader = new PatchReader::Raw();
+  my $patch_info_grabber = new PatchReader::PatchInfoGrabber();
+  $reader->sends_data_to($patch_info_grabber);
+
+  # Iterate over the file
+  $reader->iterate_file($filename);
+
+  # Print the output
+  my $patch_info = $patch_info_grabber->patch_info();
+  print "Summary of Changed Files:\n";
+  while (my ($file, $info) = each %{$patch_info->{files}}) {
+    print "$file: +$info->{plus_lines} -$info->{minus_lines}\n";
+  }
+
+=head1 ABSTRACT
+
+This perl library allows you to manipulate patches programmatically by
+chaining together a variety of objects that read, manipulate, and output
+patch information:
+
+=over
+
+=item PatchReader::Raw
+
+Parse a patch in any format known to this author (unified, normal, cvs diff,
+among others)
+
+=item PatchReader::PatchInfoGrabber
+
+Grab summary info for sections of a patch in a nice hash
+
+=item PatchReader::AddCVSContext
+
+Add context to the patch by grabbing the original files from CVS
+
+=item PatchReader::NarrowPatch
+
+Narrow a patch down to only apply to a specific set of files
+
+=item PatchReader::DiffPrinter::raw
+
+Output the parsed patch in raw unified diff format
+
+=item PatchReader::DiffPrinter::template
+
+Output the parsed patch to L<Template::Toolkit> templates (can be used to make
+HTML output or anything else you please)
+
+=back
+
+Additionally, it is designed so that you can plug in your own objects that
+read the parsed data while it is being parsed (no need for the performance or
+memory problems that can come from reading in the entire patch all at once).
+You can do this by mimicking one of the existing readers (such as
+PatchInfoGrabber) and overriding the methods start_patch, start_file, section,
+end_file and end_patch.
+
+=head1 AUTHORS
+
+ John Keiser <jkeiser@cpan.org>
+ Teemu Mannermaa <tmannerm@cpan.org>
+
+=head1 COPYRIGHT AND LICENSE
+
+ Copyright (C) 2003-2004, John Keiser and
+ Copyright (C) 2011-2012, Teemu Mannermaa.
+
+This module is free software; you can redistribute it and/or modify it under
+the terms of the Artistic License 1.0. For details, see the full text of the
+license at
+ <http://www.perlfoundation.org/artistic_license_1_0>.
+
+This module is distributed in the hope that it will be useful, but it is
+provided “as is” and without any warranty; without even the implied warranty
+of merchantability or fitness for a particular purpose.
+
+Files with different licenses or copyright holders:
+
+=over 
+
+=item F<lib/PatchReader/CVSClient.pm>
+
+Portions created by Netscape are
+Copyright (C) 2003, Netscape Communications Corporation. All rights reserved.
+
+This file is subject to the terms of the Mozilla Public License, v. 2.0.
+
+=back
+
+=cut
+
+$Bugzilla::PatchReader::VERSION = '0.9.7';
+
+1
diff --git a/Bugzilla/PatchReader/AddCVSContext.pm b/Bugzilla/PatchReader/AddCVSContext.pm
new file mode 100644
index 000000000..910e45669
--- /dev/null
+++ b/Bugzilla/PatchReader/AddCVSContext.pm
@@ -0,0 +1,226 @@
+package Bugzilla::PatchReader::AddCVSContext;
+
+use Bugzilla::PatchReader::FilterPatch;
+use Bugzilla::PatchReader::CVSClient;
+use Cwd;
+use File::Temp;
+
+use strict;
+
+@Bugzilla::PatchReader::AddCVSContext::ISA = qw(Bugzilla::PatchReader::FilterPatch);
+
+# XXX If you need to, get the entire patch worth of files and do a single
+# cvs update of all files as soon as you find a file where you need to do a
+# cvs update, to avoid the significant connect overhead
+sub new {
+  my $class = shift;
+  $class = ref($class) || $class;
+  my $this = $class->SUPER::new();
+  bless $this, $class;
+
+  $this->{CONTEXT} = $_[0];
+  $this->{CVSROOT} = $_[1];
+
+  return $this;
+}
+
+sub my_rmtree {
+  my ($this, $dir) = @_;
+  foreach my $file (glob("$dir/*")) {
+    if (-d $file) {
+      $this->my_rmtree($file);
+    } else {
+      trick_taint($file);
+      unlink $file;
+    }
+  }
+  trick_taint($dir);
+  rmdir $dir;
+}
+
+sub end_patch {
+  my $this = shift;
+  if (exists($this->{TMPDIR})) {
+    # Set as variable to get rid of taint
+    # One would like to use rmtree here, but that is not taint-safe.
+    $this->my_rmtree($this->{TMPDIR});
+  }
+}
+
+sub start_file {
+  my $this = shift;
+  my ($file) = @_;
+  $this->{HAS_CVS_CONTEXT} = !$file->{is_add} && !$file->{is_remove} &&
+                             $file->{old_revision};
+  $this->{REVISION} = $file->{old_revision};
+  $this->{FILENAME} = $file->{filename};
+  $this->{SECTION_END} = -1;
+  $this->{TARGET}->start_file(@_) if $this->{TARGET};
+}
+
+sub end_file {
+  my $this = shift;
+  $this->flush_section();
+
+  if ($this->{FILE}) {
+    close $this->{FILE};
+    unlink $this->{FILE}; # If it fails, it fails ...
+    delete $this->{FILE};
+  }
+  $this->{TARGET}->end_file(@_) if $this->{TARGET};
+}
+
+sub next_section {
+  my $this = shift;
+  my ($section) = @_;
+  $this->{NEXT_PATCH_LINE} = $section->{old_start};
+  $this->{NEXT_NEW_LINE} = $section->{new_start};
+  foreach my $line (@{$section->{lines}}) {
+    # If this is a line requiring context ...
+    if ($line =~ /^[-\+]/) {
+      # Determine how much context is needed for both the previous section line
+      # and this one:
+      # - If there is no old line, start new section
+      # - If this is file context, add (old section end to new line) context to
+      # the existing section
+      # - If old end context line + 1 < new start context line, there is an empty
+      #   space and therefore we end the old section and start the new one
+      # - Else we add (old start context line through new line) context to
+      #   existing section
+      if (! exists($this->{SECTION})) {
+        $this->_start_section();
+      } elsif ($this->{CONTEXT} eq "file") {
+        $this->push_context_lines($this->{SECTION_END} + 1,
+                                  $this->{NEXT_PATCH_LINE} - 1);
+      } else {
+        my $start_context = $this->{NEXT_PATCH_LINE} - $this->{CONTEXT};
+        $start_context = $start_context > 0 ? $start_context : 0;
+        if (($this->{SECTION_END} + $this->{CONTEXT} + 1) < $start_context) {
+          $this->flush_section();
+          $this->_start_section();
+        } else {
+          $this->push_context_lines($this->{SECTION_END} + 1,
+                                    $this->{NEXT_PATCH_LINE} - 1);
+        }
+      }
+      push @{$this->{SECTION}{lines}}, $line;
+      if (substr($line, 0, 1) eq "+") {
+        $this->{SECTION}{plus_lines}++;
+        $this->{SECTION}{new_lines}++;
+        $this->{NEXT_NEW_LINE}++;
+      } else {
+        $this->{SECTION_END}++;
+        $this->{SECTION}{minus_lines}++;
+        $this->{SECTION}{old_lines}++;
+        $this->{NEXT_PATCH_LINE}++;
+      }
+    } else {
+      $this->{NEXT_PATCH_LINE}++;
+      $this->{NEXT_NEW_LINE}++;
+    }
+    # If this is context, for now lose it (later we should try and determine if
+    # we can just use it instead of pulling the file all the time)
+  }
+}
+
+sub determine_start {
+  my ($this, $line) = @_;
+  return 0 if $line < 0;
+  if ($this->{CONTEXT} eq "file") {
+    return 1;
+  } else {
+    my $start = $line - $this->{CONTEXT};
+    $start = $start > 0 ? $start : 1;
+    return $start;
+  }
+}
+
+sub _start_section {
+  my $this = shift;
+
+  # Add the context to the beginning
+  $this->{SECTION}{old_start} = $this->determine_start($this->{NEXT_PATCH_LINE});
+  $this->{SECTION}{new_start} = $this->determine_start($this->{NEXT_NEW_LINE});
+  $this->{SECTION}{old_lines} = 0;
+  $this->{SECTION}{new_lines} = 0;
+  $this->{SECTION}{minus_lines} = 0;
+  $this->{SECTION}{plus_lines} = 0;
+  $this->{SECTION_END} = $this->{SECTION}{old_start} - 1;
+  $this->push_context_lines($this->{SECTION}{old_start},
+                            $this->{NEXT_PATCH_LINE} - 1);
+}
+
+sub flush_section {
+  my $this = shift;
+
+  if ($this->{SECTION}) {
+    # Add the necessary context to the end
+    if ($this->{CONTEXT} eq "file") {
+      $this->push_context_lines($this->{SECTION_END} + 1, "file");
+    } else {
+      $this->push_context_lines($this->{SECTION_END} + 1,
+                                $this->{SECTION_END} + $this->{CONTEXT});
+    }
+    # Send the section and line notifications
+    $this->{TARGET}->next_section($this->{SECTION}) if $this->{TARGET};
+    delete $this->{SECTION};
+    $this->{SECTION_END} = 0;
+  }
+}
+
+sub push_context_lines {
+  my $this = shift;
+  # Grab from start to end
+  my ($start, $end) = @_;
+  return if $end ne "file" && $start > $end;
+
+  # If it's an added / removed file, don't do anything
+  return if ! $this->{HAS_CVS_CONTEXT};
+
+  # Get and open the file if necessary
+  if (!$this->{FILE}) {
+    my $olddir = getcwd();
+    if (! exists($this->{TMPDIR})) {
+      $this->{TMPDIR} = File::Temp::tempdir();
+      if (! -d $this->{TMPDIR}) {
+        die "Could not get temporary directory";
+      }
+    }
+    chdir($this->{TMPDIR}) or die "Could not cd $this->{TMPDIR}";
+    if (Bugzilla::PatchReader::CVSClient::cvs_co_rev($this->{CVSROOT}, $this->{REVISION}, $this->{FILENAME})) {
+      die "Could not check out $this->{FILENAME} r$this->{REVISION} from $this->{CVSROOT}";
+    }
+    open my $fh, $this->{FILENAME} or die "Could not open $this->{FILENAME}";
+    $this->{FILE} = $fh;
+    $this->{NEXT_FILE_LINE} = 1;
+    trick_taint($olddir); # $olddir comes from getcwd()
+    chdir($olddir) or die "Could not cd back to $olddir";
+  }
+
+  # Read through the file to reach the line we need
+  die "File read too far!" if $this->{NEXT_FILE_LINE} && $this->{NEXT_FILE_LINE} > $start;
+  my $fh = $this->{FILE};
+  while ($this->{NEXT_FILE_LINE} < $start) {
+    my $dummy = <$fh>;
+    $this->{NEXT_FILE_LINE}++;
+  }
+  my $i = $start;
+  for (; $end eq "file" || $i <= $end; $i++) {
+    my $line = <$fh>;
+    last if !defined($line);
+    $line =~ s/\r\n/\n/g;
+    push @{$this->{SECTION}{lines}}, " $line";
+    $this->{NEXT_FILE_LINE}++;
+    $this->{SECTION}{old_lines}++;
+    $this->{SECTION}{new_lines}++;
+  }
+  $this->{SECTION_END} = $i - 1;
+}
+
+sub trick_taint {
+  $_[0] =~ /^(.*)$/s;
+  $_[0] = $1;
+  return (defined($_[0]));
+}
+
+1;
diff --git a/Bugzilla/PatchReader/Base.pm b/Bugzilla/PatchReader/Base.pm
new file mode 100644
index 000000000..f2fd69a68
--- /dev/null
+++ b/Bugzilla/PatchReader/Base.pm
@@ -0,0 +1,23 @@
+package Bugzilla::PatchReader::Base;
+
+use strict;
+
+sub new {
+  my $class = shift;
+  $class = ref($class) || $class;
+  my $this = {};
+  bless $this, $class;
+
+  return $this;
+}
+
+sub sends_data_to {
+  my $this = shift;
+  if (defined($_[0])) {
+    $this->{TARGET} = $_[0];
+  } else {
+    return $this->{TARGET};
+  }
+}
+
+1
diff --git a/Bugzilla/PatchReader/CVSClient.pm b/Bugzilla/PatchReader/CVSClient.pm
new file mode 100644
index 000000000..2f76fc18d
--- /dev/null
+++ b/Bugzilla/PatchReader/CVSClient.pm
@@ -0,0 +1,48 @@
+# -*- Mode: perl; indent-tabs-mode: nil -*-
+#
+# 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/.
+#
+# This Source Code Form is "Incompatible With Secondary Licenses", as
+# defined by the Mozilla Public License, v. 2.0.
+
+package Bugzilla::PatchReader::CVSClient;
+
+use strict;
+
+sub parse_cvsroot {
+    my $cvsroot = $_[0];
+    # Format: :method:[user[:password]@]server[:[port]]/path
+    if ($cvsroot =~ /^:([^:]*):(.*?)(\/.*)$/) {
+        my %retval;
+        $retval{protocol} = $1;
+        $retval{rootdir} = $3;
+        my $remote = $2;
+        if ($remote =~ /^(([^\@:]*)(:([^\@]*))?\@)?([^:]*)(:(.*))?$/) {
+            $retval{user} = $2;
+            $retval{password} = $4;
+            $retval{server} = $5;
+            $retval{port} = $7;
+            return %retval;
+        }
+    }
+
+    return (
+        rootdir => $cvsroot
+    );
+}
+
+sub cvs_co {
+    my ($cvsroot, @files) = @_;
+    my $cvs = $::cvsbin || "cvs";
+    return system($cvs, "-Q", "-d$cvsroot", "co", @files);
+}
+
+sub cvs_co_rev {
+    my ($cvsroot, $rev, @files) = @_;
+    my $cvs = $::cvsbin || "cvs";
+    return system($cvs, "-Q", "-d$cvsroot", "co", "-r$rev", @files);
+}
+
+1
diff --git a/Bugzilla/PatchReader/DiffPrinter/raw.pm b/Bugzilla/PatchReader/DiffPrinter/raw.pm
new file mode 100644
index 000000000..ceb425800
--- /dev/null
+++ b/Bugzilla/PatchReader/DiffPrinter/raw.pm
@@ -0,0 +1,61 @@
+package Bugzilla::PatchReader::DiffPrinter::raw;
+
+use strict;
+
+sub new {
+  my $class = shift;
+  $class = ref($class) || $class;
+  my $this = {};
+  bless $this, $class;
+
+  $this->{OUTFILE} = @_ ? $_[0] : *STDOUT;
+  my $fh = $this->{OUTFILE};
+
+  return $this;
+}
+
+sub start_patch {
+}
+
+sub end_patch {
+}
+
+sub start_file {
+  my $this = shift;
+  my ($file) = @_;
+
+  my $fh = $this->{OUTFILE};
+  if ($file->{rcs_filename}) {
+    print $fh "Index: $file->{filename}\n";
+    print $fh "===================================================================\n";
+    print $fh "RCS file: $file->{rcs_filename}\n";
+  }
+  my $old_file = $file->{is_add} ? "/dev/null" : $file->{filename};
+  my $old_date = $file->{old_date_str} || "";
+  print $fh "--- $old_file\t$old_date";
+  print $fh "\t$file->{old_revision}" if $file->{old_revision};
+  print $fh "\n";
+  my $new_file = $file->{is_remove} ? "/dev/null" : $file->{filename};
+  my $new_date = $file->{new_date_str} || "";
+  print $fh "+++ $new_file\t$new_date";
+  print $fh "\t$file->{new_revision}" if $file->{new_revision};
+  print $fh "\n";
+}
+
+sub end_file {
+}
+
+sub next_section {
+  my $this = shift;
+  my ($section) = @_;
+
+  return unless $section->{old_start} || $section->{new_start};
+  my $fh = $this->{OUTFILE};
+  print $fh "@@ -$section->{old_start},$section->{old_lines} +$section->{new_start},$section->{new_lines} @@ $section->{func_info}\n";
+  foreach my $line (@{$section->{lines}}) {
+    $line =~ s/(\r?\n?)$/\n/;
+    print $fh $line;
+  }
+}
+
+1
diff --git a/Bugzilla/PatchReader/DiffPrinter/template.pm b/Bugzilla/PatchReader/DiffPrinter/template.pm
new file mode 100644
index 000000000..6545e9336
--- /dev/null
+++ b/Bugzilla/PatchReader/DiffPrinter/template.pm
@@ -0,0 +1,119 @@
+package Bugzilla::PatchReader::DiffPrinter::template;
+
+use strict;
+
+sub new {
+  my $class = shift;
+  $class = ref($class) || $class;
+  my $this = {};
+  bless $this, $class;
+
+  $this->{TEMPLATE_PROCESSOR} = $_[0];
+  $this->{HEADER_TEMPLATE} = $_[1];
+  $this->{FILE_TEMPLATE} = $_[2];
+  $this->{FOOTER_TEMPLATE} = $_[3];
+  $this->{ARGS} = $_[4] || {};
+
+  $this->{ARGS}{file_count} = 0;
+  return $this;
+}
+
+sub start_patch {
+  my $this = shift;
+  $this->{TEMPLATE_PROCESSOR}->process($this->{HEADER_TEMPLATE}, $this->{ARGS})
+      || ::ThrowTemplateError($this->{TEMPLATE_PROCESSOR}->error());
+}
+
+sub end_patch {
+  my $this = shift;
+  $this->{TEMPLATE_PROCESSOR}->process($this->{FOOTER_TEMPLATE}, $this->{ARGS})
+      || ::ThrowTemplateError($this->{TEMPLATE_PROCESSOR}->error());
+}
+
+sub start_file {
+  my $this = shift;
+  $this->{ARGS}{file_count}++;
+  $this->{ARGS}{file} = shift;
+  $this->{ARGS}{file}{plus_lines} = 0;
+  $this->{ARGS}{file}{minus_lines} = 0;
+  @{$this->{ARGS}{sections}} = ();
+}
+
+sub end_file {
+  my $this = shift;
+  my $file = $this->{ARGS}{file};
+  if ($file->{canonical} && $file->{old_revision} && $this->{ARGS}{bonsai_url}) {
+    $this->{ARGS}{bonsai_prefix} = "$this->{ARGS}{bonsai_url}/cvsblame.cgi?file=$file->{filename}&amp;rev=$file->{old_revision}";
+  }
+  if ($file->{canonical} && $this->{ARGS}{lxr_url}) {
+    # Cut off the lxr root, if any
+    my $filename = $file->{filename};
+    $filename = substr($filename, length($this->{ARGS}{lxr_root}));
+    $this->{ARGS}{lxr_prefix} = "$this->{ARGS}{lxr_url}/source/$filename";
+  }
+
+  $this->{TEMPLATE_PROCESSOR}->process($this->{FILE_TEMPLATE}, $this->{ARGS})
+      || ::ThrowTemplateError($this->{TEMPLATE_PROCESSOR}->error());
+  @{$this->{ARGS}{sections}} = ();
+  delete $this->{ARGS}{file};
+}
+
+sub next_section {
+  my $this = shift;
+  my ($section) = @_;
+
+  $this->{ARGS}{file}{plus_lines} += $section->{plus_lines};
+  $this->{ARGS}{file}{minus_lines} += $section->{minus_lines};
+
+  # Get groups of lines and print them
+  my $last_line_char = '';
+  my $context_lines = [];
+  my $plus_lines = [];
+  my $minus_lines = [];
+  foreach my $line (@{$section->{lines}}) {
+    $line =~ s/\r?\n?$//;
+    if ($line =~ /^ /) {
+      if ($last_line_char ne ' ') {
+        push @{$section->{groups}}, {context => $context_lines,
+                                     plus => $plus_lines,
+                                     minus => $minus_lines};
+        $context_lines = [];
+        $plus_lines = [];
+        $minus_lines = [];
+      }
+      $last_line_char = ' ';
+      push @{$context_lines}, substr($line, 1);
+    } elsif ($line =~ /^\+/) {
+      if ($last_line_char eq ' ' || $last_line_char eq '-' && @{$plus_lines}) {
+        push @{$section->{groups}}, {context => $context_lines,
+                                     plus => $plus_lines,
+                                     minus => $minus_lines};
+        $context_lines = [];
+        $plus_lines = [];
+        $minus_lines = [];
+        $last_line_char = '';
+      }
+      $last_line_char = '+';
+      push @{$plus_lines}, substr($line, 1);
+    } elsif ($line =~ /^-/) {
+      if ($last_line_char eq '+' && @{$minus_lines}) {
+        push @{$section->{groups}}, {context => $context_lines,
+                                     plus => $plus_lines,
+                                     minus => $minus_lines};
+        $context_lines = [];
+        $plus_lines = [];
+        $minus_lines = [];
+        $last_line_char = '';
+      }
+      $last_line_char = '-';
+      push @{$minus_lines}, substr($line, 1);
+    }
+  }
+
+  push @{$section->{groups}}, {context => $context_lines,
+                               plus => $plus_lines,
+                               minus => $minus_lines};
+  push @{$this->{ARGS}{sections}}, $section;
+}
+
+1
diff --git a/Bugzilla/PatchReader/FilterPatch.pm b/Bugzilla/PatchReader/FilterPatch.pm
new file mode 100644
index 000000000..dfe42e750
--- /dev/null
+++ b/Bugzilla/PatchReader/FilterPatch.pm
@@ -0,0 +1,43 @@
+package Bugzilla::PatchReader::FilterPatch;
+
+use strict;
+
+use Bugzilla::PatchReader::Base;
+
+@Bugzilla::PatchReader::FilterPatch::ISA = qw(Bugzilla::PatchReader::Base);
+
+sub new {
+  my $class = shift;
+  $class = ref($class) || $class;
+  my $this  = $class->SUPER::new();
+  bless $this, $class;
+
+  return $this;
+}
+
+sub start_patch {
+  my $this = shift;
+  $this->{TARGET}->start_patch(@_) if $this->{TARGET};
+}
+
+sub end_patch {
+  my $this = shift;
+  $this->{TARGET}->end_patch(@_) if $this->{TARGET};
+}
+
+sub start_file {
+  my $this = shift;
+  $this->{TARGET}->start_file(@_) if $this->{TARGET};
+}
+
+sub end_file {
+  my $this = shift;
+  $this->{TARGET}->end_file(@_) if $this->{TARGET};
+}
+
+sub next_section {
+  my $this = shift;
+  $this->{TARGET}->next_section(@_) if $this->{TARGET};
+}
+
+1
diff --git a/Bugzilla/PatchReader/FixPatchRoot.pm b/Bugzilla/PatchReader/FixPatchRoot.pm
new file mode 100644
index 000000000..e67fb2796
--- /dev/null
+++ b/Bugzilla/PatchReader/FixPatchRoot.pm
@@ -0,0 +1,130 @@
+package Bugzilla::PatchReader::FixPatchRoot;
+
+use Bugzilla::PatchReader::FilterPatch;
+use Bugzilla::PatchReader::CVSClient;
+
+use strict;
+
+@Bugzilla::PatchReader::FixPatchRoot::ISA = qw(Bugzilla::PatchReader::FilterPatch);
+
+sub new {
+  my $class = shift;
+  $class = ref($class) || $class;
+  my $this = $class->SUPER::new();
+  bless $this, $class;
+
+  my %parsed = Bugzilla::PatchReader::CVSClient::parse_cvsroot($_[0]);
+  $this->{REPOSITORY_ROOT} = $parsed{rootdir};
+  $this->{REPOSITORY_ROOT} .= "/" if substr($this->{REPOSITORY_ROOT}, -1) ne "/";
+
+  return $this;
+}
+
+sub diff_root {
+  my $this = shift;
+  if (@_) {
+    $this->{DIFF_ROOT} = $_[0];
+  } else {
+    return $this->{DIFF_ROOT};
+  }
+}
+
+sub flush_delayed_commands {
+  my $this = shift;
+  return if ! $this->{DELAYED_COMMANDS};
+
+  my $commands = $this->{DELAYED_COMMANDS};
+  delete $this->{DELAYED_COMMANDS};
+  $this->{FORCE_COMMANDS} = 1;
+  foreach my $command_arr (@{$commands}) {
+    my $command = $command_arr->[0];
+    my $arg = $command_arr->[1];
+    if ($command eq "start_file") {
+      $this->start_file($arg);
+    } elsif ($command eq "end_file") {
+      $this->end_file($arg);
+    } elsif ($command eq "section") {
+      $this->next_section($arg);
+    }
+  }
+}
+
+sub end_patch {
+  my $this = shift;
+  $this->flush_delayed_commands();
+  $this->{TARGET}->end_patch(@_) if $this->{TARGET};
+}
+
+sub start_file {
+  my $this = shift;
+  my ($file) = @_;
+  # If the file is new, it will not have a filename that fits the repository
+  # root and therefore needs to be fixed up to have the same root as everyone
+  # else.  At the same time we need to fix DIFF_ROOT too.
+  if (exists($this->{DIFF_ROOT})) {
+    # XXX Return error if there are multiple roots in the patch by verifying
+    # that the DIFF_ROOT is not different from the calculated diff root on this
+    # filename
+
+    $file->{filename} = $this->{DIFF_ROOT} . $file->{filename};
+
+    $file->{canonical} = 1;
+  } elsif ($file->{rcs_filename} &&
+           substr($file->{rcs_filename}, 0, length($this->{REPOSITORY_ROOT})) eq
+           $this->{REPOSITORY_ROOT}) {
+    # Since we know the repository we can determine where the user was in the
+    # repository when they did the diff by chopping off the repository root
+    # from the rcs filename
+    $this->{DIFF_ROOT} = substr($file->{rcs_filename},
+                                length($this->{REPOSITORY_ROOT}));
+    $this->{DIFF_ROOT} =~ s/,v$//;
+    # If the RCS file exists in the Attic then we need to correct for
+    # this, stripping off the '/Attic' suffix in order to reduce the name
+    # to just the CVS root.
+    if ($this->{DIFF_ROOT} =~ m/Attic/) {
+      $this->{DIFF_ROOT} = substr($this->{DIFF_ROOT}, 0, -6);
+    }
+    # XXX More error checking--that filename exists and that it is in fact
+    # part of the rcs filename
+    $this->{DIFF_ROOT} = substr($this->{DIFF_ROOT}, 0,
+                                -length($file->{filename}));
+    $this->flush_delayed_commands();
+
+    $file->{filename} = $this->{DIFF_ROOT} . $file->{filename};
+
+    $file->{canonical} = 1;
+  } else {
+    # DANGER Will Robinson.  The first file in the patch is new.  We will try
+    # "delayed command mode"
+    #
+    # (if force commands is on we are already in delayed command mode, and sadly
+    # this means the entire patch was unintelligible to us, so we just output
+    # whatever the hell was in the patch)
+
+    if (!$this->{FORCE_COMMANDS}) {
+      push @{$this->{DELAYED_COMMANDS}}, [ "start_file", { %{$file} } ];
+      return;
+    }
+  }
+  $this->{TARGET}->start_file($file) if $this->{TARGET};
+}
+
+sub end_file {
+  my $this = shift;
+  if (exists($this->{DELAYED_COMMANDS})) {
+    push @{$this->{DELAYED_COMMANDS}}, [ "end_file", { %{$_[0]} } ];
+  } else {
+    $this->{TARGET}->end_file(@_) if $this->{TARGET};
+  }
+}
+
+sub next_section {
+  my $this = shift;
+  if (exists($this->{DELAYED_COMMANDS})) {
+    push @{$this->{DELAYED_COMMANDS}}, [ "section", { %{$_[0]} } ];
+  } else {
+    $this->{TARGET}->next_section(@_) if $this->{TARGET};
+  }
+}
+
+1
diff --git a/Bugzilla/PatchReader/NarrowPatch.pm b/Bugzilla/PatchReader/NarrowPatch.pm
new file mode 100644
index 000000000..b6502f2f3
--- /dev/null
+++ b/Bugzilla/PatchReader/NarrowPatch.pm
@@ -0,0 +1,44 @@
+package Bugzilla::PatchReader::NarrowPatch;
+
+use Bugzilla::PatchReader::FilterPatch;
+
+use strict;
+
+@Bugzilla::PatchReader::NarrowPatch::ISA = qw(Bugzilla::PatchReader::FilterPatch);
+
+sub new {
+  my $class = shift;
+  $class = ref($class) || $class;
+  my $this = $class->SUPER::new();
+  bless $this, $class;
+
+  $this->{INCLUDE_FILES} = [@_];
+
+  return $this;
+}
+
+sub start_file {
+  my $this = shift;
+  my ($file) = @_;
+  if (grep { $_ eq substr($file->{filename}, 0, length($_)) } @{$this->{INCLUDE_FILES}}) {
+    $this->{IS_INCLUDED} = 1;
+    $this->{TARGET}->start_file(@_) if $this->{TARGET};
+  }
+}
+
+sub end_file {
+  my $this = shift;
+  if ($this->{IS_INCLUDED}) {
+    $this->{TARGET}->end_file(@_) if $this->{TARGET};
+    $this->{IS_INCLUDED} = 0;
+  }
+}
+
+sub next_section {
+  my $this = shift;
+  if ($this->{IS_INCLUDED}) {
+    $this->{TARGET}->next_section(@_) if $this->{TARGET};
+  }
+}
+
+1
diff --git a/Bugzilla/PatchReader/PatchInfoGrabber.pm b/Bugzilla/PatchReader/PatchInfoGrabber.pm
new file mode 100644
index 000000000..8c52931ba
--- /dev/null
+++ b/Bugzilla/PatchReader/PatchInfoGrabber.pm
@@ -0,0 +1,45 @@
+package Bugzilla::PatchReader::PatchInfoGrabber;
+
+use Bugzilla::PatchReader::FilterPatch;
+
+use strict;
+
+@Bugzilla::PatchReader::PatchInfoGrabber::ISA = qw(Bugzilla::PatchReader::FilterPatch);
+
+sub new {
+  my $class = shift;
+  $class = ref($class) || $class;
+  my $this = $class->SUPER::new();
+  bless $this, $class;
+
+  return $this;
+}
+
+sub patch_info {
+  my $this = shift;
+  return $this->{PATCH_INFO};
+}
+
+sub start_patch {
+  my $this = shift;
+  $this->{PATCH_INFO} = {};
+  $this->{TARGET}->start_patch(@_) if $this->{TARGET};
+}
+
+sub start_file {
+  my $this = shift;
+  my ($file) = @_;
+  $this->{PATCH_INFO}{files}{$file->{filename}} = { %{$file} };
+  $this->{FILE} = { %{$file} };
+  $this->{TARGET}->start_file(@_) if $this->{TARGET};
+}
+
+sub next_section {
+  my $this = shift;
+  my ($section) = @_;
+  $this->{PATCH_INFO}{files}{$this->{FILE}{filename}}{plus_lines} += $section->{plus_lines};
+  $this->{PATCH_INFO}{files}{$this->{FILE}{filename}}{minus_lines} += $section->{minus_lines};
+  $this->{TARGET}->next_section(@_) if $this->{TARGET};
+}
+
+1
diff --git a/Bugzilla/PatchReader/Raw.pm b/Bugzilla/PatchReader/Raw.pm
new file mode 100644
index 000000000..b58ed3a2d
--- /dev/null
+++ b/Bugzilla/PatchReader/Raw.pm
@@ -0,0 +1,268 @@
+package Bugzilla::PatchReader::Raw;
+
+#
+# USAGE:
+# use PatchReader::Raw;
+# my $parser = new PatchReader::Raw();
+# $parser->sends_data_to($my_target);
+# $parser->start_lines();
+# open FILE, "mypatch.patch";
+# while (<FILE>) {
+#   $parser->next_line($_);
+# }
+# $parser->end_lines();
+#
+
+use strict;
+
+use Bugzilla::PatchReader::Base;
+
+@Bugzilla::PatchReader::Raw::ISA = qw(Bugzilla::PatchReader::Base);
+
+sub new {
+  my $class = shift;
+  $class = ref($class) || $class;
+  my $this  = $class->SUPER::new();
+  bless $this, $class;
+
+  return $this;
+}
+
+# We send these notifications:
+# start_patch({ patchname })
+# start_file({ filename, rcs_filename, old_revision, old_date_str, new_revision, new_date_str, is_add, is_remove })
+# next_section({ old_start, new_start, old_lines, new_lines, @lines })
+# end_patch
+# end_file
+sub next_line {
+  my $this = shift;
+  my ($line) = @_;
+
+  return if $line =~ /^\?/;
+
+  # patch header parsing
+  if ($line =~ /^---\s*([\S ]+)\s*\t([^\t\r\n]*)\s*(\S*)/) {
+    $this->_maybe_end_file();
+
+    if ($1 eq "/dev/null") {
+      $this->{FILE_STATE}{is_add} = 1;
+    } else {
+      $this->{FILE_STATE}{filename} = $1;
+    }
+    $this->{FILE_STATE}{old_date_str} = $2;
+    $this->{FILE_STATE}{old_revision} = $3 if $3;
+
+    $this->{IN_HEADER} = 1;
+
+  } elsif ($line =~ /^\+\+\+\s*([\S ]+)\s*\t([^\t\r\n]*)(\S*)/) {
+    if ($1 eq "/dev/null") {
+      $this->{FILE_STATE}{is_remove} = 1;
+    }
+    $this->{FILE_STATE}{new_date_str} = $2;
+    $this->{FILE_STATE}{new_revision} = $3 if $3;
+
+    $this->{IN_HEADER} = 1;
+
+  } elsif ($line =~ /^RCS file: ([\S ]+)/) {
+    $this->{FILE_STATE}{rcs_filename} = $1;
+
+    $this->{IN_HEADER} = 1;
+
+  } elsif ($line =~ /^retrieving revision (\S+)/) {
+    $this->{FILE_STATE}{old_revision} = $1;
+
+    $this->{IN_HEADER} = 1;
+
+  } elsif ($line =~ /^Index:\s*([\S ]+)/) {
+    $this->_maybe_end_file();
+
+    $this->{FILE_STATE}{filename} = $1;
+
+    $this->{IN_HEADER} = 1;
+
+  } elsif ($line =~ /^diff\s*(-\S+\s*)*(\S+)\s*(\S*)/ && $3) {
+    # Simple diff <dir> <dir>
+    $this->_maybe_end_file();
+    $this->{FILE_STATE}{filename} = $2;
+
+    $this->{IN_HEADER} = 1;
+
+  # section parsing
+  } elsif ($line =~ /^@@\s*-(\d+),?(\d*)\s*\+(\d+),?(\d*)\s*(?:@@\s*(.*))?/) {
+    $this->{IN_HEADER} = 0;
+
+    $this->_maybe_start_file();
+    $this->_maybe_end_section();
+    $2 = 0 if !defined($2);
+    $4 = 0 if !defined($4);
+    $this->{SECTION_STATE} = { old_start => $1, old_lines => $2,
+                               new_start => $3, new_lines => $4,
+                               func_info => $5,
+                               minus_lines => 0, plus_lines => 0,
+                             };
+
+  } elsif ($line =~ /^(\d+),?(\d*)([acd])(\d+),?(\d*)/) {
+    # Non-universal diff.  Calculate as though it were universal.
+    $this->{IN_HEADER} = 0;
+
+    $this->_maybe_start_file();
+    $this->_maybe_end_section();
+
+    my $old_start;
+    my $old_lines;
+    my $new_start;
+    my $new_lines;
+    if ($3 eq 'a') {
+      # 'a' has the old number one off from diff -u ("insert after this line"
+      # vs. "insert at this line")
+      $old_start = $1 + 1;
+      $old_lines = 0;
+    } else {
+      $old_start = $1;
+      $old_lines = $2 ? ($2 - $1 + 1) : 1;
+    }
+    if ($3 eq 'd') {
+      # 'd' has the new number one off from diff -u ("delete after this line"
+      # vs. "delete at this line")
+      $new_start = $4 + 1;
+      $new_lines = 0;
+    } else {
+      $new_start = $4;
+      $new_lines = $5 ? ($5 - $4 + 1) : 1;
+    }
+
+    $this->{SECTION_STATE} = { old_start => $old_start, old_lines => $old_lines,
+                               new_start => $new_start, new_lines => $new_lines,
+                               minus_lines => 0, plus_lines => 0
+                             };
+  }
+
+  # line parsing (only when inside a section)
+  return if $this->{IN_HEADER};
+  if ($line =~ /^ /) {
+    push @{$this->{SECTION_STATE}{lines}}, $line;
+  } elsif ($line =~ /^-/) {
+    $this->{SECTION_STATE}{minus_lines}++;
+    push @{$this->{SECTION_STATE}{lines}}, $line;
+  } elsif ($line =~ /^\+/) {
+    $this->{SECTION_STATE}{plus_lines}++;
+    push @{$this->{SECTION_STATE}{lines}}, $line;
+  } elsif ($line =~ /^< /) {
+    $this->{SECTION_STATE}{minus_lines}++;
+    push @{$this->{SECTION_STATE}{lines}}, "-" . substr($line, 2);
+  } elsif ($line =~ /^> /) {
+    $this->{SECTION_STATE}{plus_lines}++;
+    push @{$this->{SECTION_STATE}{lines}}, "+" . substr($line, 2);
+  }
+}
+
+sub start_lines {
+  my $this = shift;
+  die "No target specified: call sends_data_to!" if !$this->{TARGET};
+  delete $this->{FILE_STARTED};
+  delete $this->{FILE_STATE};
+  delete $this->{SECTION_STATE};
+  $this->{FILE_NEVER_STARTED} = 1;
+
+  $this->{TARGET}->start_patch(@_);
+}
+
+sub end_lines {
+  my $this = shift;
+  $this->_maybe_end_file();
+  $this->{TARGET}->end_patch(@_);
+}
+
+sub _init_state {
+  my $this = shift;
+  $this->{SECTION_STATE}{minus_lines} ||= 0;
+  $this->{SECTION_STATE}{plus_lines} ||= 0;
+}
+
+sub _maybe_start_file {
+  my $this = shift;
+  $this->_init_state();
+  if (exists($this->{FILE_STATE}) && !$this->{FILE_STARTED} ||
+      $this->{FILE_NEVER_STARTED}) {
+    $this->_start_file();
+  }
+}
+
+sub _maybe_end_file {
+  my $this = shift;
+  $this->_init_state();
+  return if $this->{IN_HEADER};
+
+  $this->_maybe_end_section();
+  if (exists($this->{FILE_STATE})) {
+    # Handle empty patch sections (if the file has not been started and we're
+    # already trying to end it, start it first!)
+    if (!$this->{FILE_STARTED}) {
+      $this->_start_file();
+    }
+    
+    # Send end notification and set state
+    $this->{TARGET}->end_file($this->{FILE_STATE});
+    delete $this->{FILE_STATE};
+    delete $this->{FILE_STARTED};
+  }
+}
+
+sub _start_file {
+  my $this = shift;
+
+  # Send start notification and set state
+  if (!$this->{FILE_STATE}) {
+    $this->{FILE_STATE} = { filename => "file_not_specified_in_diff" };
+  }
+
+  # Send start notification and set state
+  $this->{TARGET}->start_file($this->{FILE_STATE});
+  $this->{FILE_STARTED} = 1;
+  delete $this->{FILE_NEVER_STARTED};
+}
+
+sub _maybe_end_section {
+  my $this = shift;
+  if (exists($this->{SECTION_STATE})) {
+    $this->{TARGET}->next_section($this->{SECTION_STATE});
+    delete $this->{SECTION_STATE};
+  }
+}
+
+sub iterate_file {
+  my $this = shift;
+  my ($filename) = @_;
+
+  open FILE, $filename or die "Could not open $filename: $!";
+  $this->start_lines($filename);
+  while (<FILE>) {
+    $this->next_line($_);
+  }
+  $this->end_lines($filename);
+  close FILE;
+}
+
+sub iterate_fh {
+  my $this = shift;
+  my ($fh, $filename) = @_;
+
+  $this->start_lines($filename);
+  while (<$fh>) {
+    $this->next_line($_);
+  }
+  $this->end_lines($filename);
+}
+
+sub iterate_string {
+  my $this = shift;
+  my ($id, $data) = @_;
+
+  $this->start_lines($id);
+  while ($data =~ /([^\n]*(\n|$))/g) {
+    $this->next_line($1);
+  }
+  $this->end_lines($id);
+}
+
+1
diff --git a/template/en/default/attachment/diff-footer.html.tmpl b/template/en/default/attachment/diff-footer.html.tmpl
index 49c662a98..e9965a9a8 100644
--- a/template/en/default/attachment/diff-footer.html.tmpl
+++ b/template/en/default/attachment/diff-footer.html.tmpl
@@ -20,6 +20,12 @@
 
 </form>
 
+[% IF !file_count %]
+<div id="error_msg" class="throw_error">
+  No valid patch files were found in the attachment.
+</div>
+[% END %]
+
 [% IF headers %]
 
   <br>