Bug 30345: Can mark bugs duplicates of each other [dupe loops] - Patch by Frédéric Buclin <LpSolit@gmail.com> r/a=myk

2 files changed, 47 insertions, 11 deletions

diff --git a/process_bug.cgi b/process_bug.cgi
index e64a8692d..c554e8181 100755
--- a/process_bug.cgi
+++ b/process_bug.cgi
@@ -1107,20 +1107,18 @@ SWITCH: for ($cgi->param('knob')) {
         last SWITCH;
     };
     /^duplicate$/ && CheckonComment( "duplicate" ) && do {
+        # You cannot mark bugs as duplicates when changing
+        # several bugs at once.
+        unless (defined $cgi->param('id')) {
+            ThrowUserError('dupe_not_allowed');
+        }
+
         # Make sure we can change the original bug (issue A on bug 96085)
         check_form_field_defined($cgi, 'dup_id');
         $duplicate = $cgi->param('dup_id');
         ValidateBugID($duplicate, 'dup_id');
         $cgi->param('dup_id', $duplicate);
 
-        # Also, let's see if the reporter has authorization to see
-        # the bug to which we are duping. If not we need to prompt.
-        DuplicateUserConfirm();
-
-        if (!defined $cgi->param('id') || $duplicate == $cgi->param('id')) {
-            ThrowUserError("dupe_of_self_disallowed");
-        }
-
         # Make sure the bug is not already marked as a dupe
         # (may appear in race condition)
         my $dupe_of =
@@ -1131,6 +1129,32 @@ SWITCH: for ($cgi->param('knob')) {
             ThrowUserError("dupe_entry_found", { dupe_of => $dupe_of });
         }
 
+        # Make sure a loop isn't created when marking this bug
+        # as duplicate.
+        my %dupes;
+        $dupe_of = $duplicate;
+        my $sth = $dbh->prepare('SELECT dupe_of FROM duplicates
+                                 WHERE dupe = ?');
+
+        while ($dupe_of) {
+            if ($dupe_of == $cgi->param('id')) {
+                ThrowUserError('dupe_loop_detected', { bug_id  => $cgi->param('id'),
+                                                       dupe_of => $duplicate });
+            }
+            # If $dupes{$dupe_of} is already set to 1, then a loop
+            # already exists which does not involve this bug.
+            # As the user is not responsible for this loop, do not
+            # prevent him from marking this bug as a duplicate.
+            last if exists $dupes{"$dupe_of"};
+            $dupes{"$dupe_of"} = 1;
+            $sth->execute($dupe_of);
+            $dupe_of = $sth->fetchrow_array;
+        }
+
+        # Also, let's see if the reporter has authorization to see
+        # the bug to which we are duping. If not we need to prompt.
+        DuplicateUserConfirm();
+
         # DUPLICATE bugs should have no time remaining.
         _remove_remaining_time();
 
diff --git a/template/en/default/global/user-error.html.tmpl b/template/en/default/global/user-error.html.tmpl
index e7798b068..3e1a2a3ef 100644
--- a/template/en/default/global/user-error.html.tmpl
+++ b/template/en/default/global/user-error.html.tmpl
@@ -336,9 +336,21 @@
     This [% terms.bug %] has already been marked as a duplicate
     of [% terms.bug %] [%+ dupe_of FILTER html %].
 
-  [% ELSIF error == "dupe_of_self_disallowed" %]
-    [% title = "Cannot mark $terms.abug as a duplicate of itself" %]
-    You can't mark [% terms.abug %] as a duplicate of itself.
+  [% ELSIF error == "dupe_not_allowed" %]
+    [% title = "Cannot mark $terms.bugs as duplicates" %]
+    You cannot mark [% terms.bugs %] as duplicates when
+    changing several [% terms.bugs %] at once.
+
+  [% ELSIF error == "dupe_loop_detected" %]
+    [% title = "Loop detected among duplicates" %]
+    You cannot mark [% terms.bug %] [%+ bug_id FILTER html %] as
+    a duplicate of
+    [% IF dupe_of == bug_id %]
+      itself
+    [% ELSE %]
+      [%+ terms.bug %] [%+ dupe_of FILTER html %], because it
+      would create a duplicate loop
+    [% END %].
 
   [% ELSIF error == "email_change_in_progress" %]
     [% title = "Email Change Already In Progress" %]