[splinter] Add review link from "Created an attachment"

[Owen Taylor <otaylor src gnome org>]

commit 1d26f75b50ca5d8c293337af6afe1c9b0b473fc3
Author: Owen W. Taylor <otaylor fishsoup net>
Date:   Sat Oct 17 14:21:59 2009 -0400

    Add review link from "Created an attachment"
    
    Use the bug-format_comment hoook to add a [review] link after
    [details] for "Created an attachment".
    
    Move get_review_link() to a new SplinterUtil.pm, along with
    code to check if an attachment is visible. (We need that to
    prevent some trivial information leakage when linkifying)

 Makefile                             |    1 +
 extension/code/bug-format_comment.pl |   30 +++++++++----
 extension/lib/SplinterUtil.pm        |   77 ++++++++++++++++++++++++++++++++++
 extension/lib/WSSplinter.pm          |   10 ++---
 4 files changed, 103 insertions(+), 15 deletions(-)
---
diff --git a/Makefile b/Makefile
index ea63677..5de31fe 100644
--- a/Makefile
+++ b/Makefile
@@ -46,6 +46,7 @@ EXTENSION_FILES =							\
 	extension/code/webservice.pl					\
 	extension/info.pl						\
 	extension/lib/ConfigSplinter.pm					\
+	extension/lib/SplinterUtil.pm					\
 	extension/lib/WSSplinter.pm					\
 	extension/template/en/attachment/list-action.html.tmpl		\
 	extension/template/en/default/admin/params/splinter.html.tmpl	\
diff --git a/extension/code/bug-format_comment.pl b/extension/code/bug-format_comment.pl
index 9dfc50d..0dc88f0 100644
--- a/extension/code/bug-format_comment.pl
+++ b/extension/code/bug-format_comment.pl
@@ -24,21 +24,33 @@ use warnings;
 use Bugzilla;
 use Bugzilla::Template;
 
-my $REVIEW_RE = qr/Review\s+of\s+attachment\s+(\d+)\s*:/;
+use extensions::splinter::lib::SplinterUtil;
 
 my $bug = Bugzilla->hook_args->{'bug'};
 my $regexes = Bugzilla->hook_args->{'regexes'};
 my $text = Bugzilla->hook_args->{'text'};
 
+# Add [review] link to the end of "Created an attachment" comments
+#
+# We need to work around the way that the hook works, which is intended
+# to avoid overlapping matches, since we *want* an overlapping match
+# here (the normal handling of "Created an attachment"), so we add in
+# dummy text and then replace in the regular expression we return from
+# the hook.
+$$text =~ s~((?:^Created\ an\ |\b)attachment\s*\(id=(\d+)\)(\s\[edit\])?)
+           ~(push(@$regexes, { match => qr/__REVIEW__$2/,
+                               replace => get_review_link($bug, "$2", "[review]") })) &&
+            (attachment_id_is_patch($2) ? "$1 __REVIEW__$2" : $1)
+           ~egmx;
+
+# And linkify "Review of attachment", this is less of a workaround since
+# there is no issue with overlap; note that there is an assumption that
+# there is only one match in the text we are linkifying, since they all
+# get the same link.
+my $REVIEW_RE = qr/Review\s+of\s+attachment\s+(\d+)\s*:/;
+
 if ($$text =~ $REVIEW_RE) {
-    my $base = Bugzilla->params->{'splinter_base'};
-    my $bug_id = $bug->id;
-    my $review_link;
-    if ($base =~ /\?/) {
-        $review_link = "<a href='$base&bug=$bug_id&attachment=$1'>Review</a>";
-    } else {
-        $review_link = "<a href='$base?bug=$bug_id&attachment=$1'>Review</a>";
-    }
+    my $review_link = get_review_link($bug, $1, "Review");
     my $attach_link = Bugzilla::Template::get_attachment_link($1, "attachment $1");
 
     push(@$regexes, { match => $REVIEW_RE,
diff --git a/extension/lib/SplinterUtil.pm b/extension/lib/SplinterUtil.pm
new file mode 100644
index 0000000..8aa48db
--- /dev/null
+++ b/extension/lib/SplinterUtil.pm
@@ -0,0 +1,77 @@
+# -*- Mode: perl; indent-tabs-mode: nil -*-
+#
+# 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 Splinter Bugzilla Extension.
+#
+# The Initial Developer of the Original Code is Red Hat, Inc.
+# Portions created by Red Hat, Inc. are Copyright (C) 2009
+# Red Hat Inc. All Rights Reserved.
+#
+# Contributor(s):
+#   Owen Taylor <otaylor fishsoup net>
+
+package extensions::splinter::lib::SplinterUtil;
+
+use Bugzilla;
+use Bugzilla::Util;
+
+use base qw(Exporter);
+ extensions::splinter::lib::SplinterUtil::EXPORT = qw(attachment_is_visible attachment_id_is_patch
+                                                      get_review_url get_review_link);
+
+# Checks if the current user can see an attachment
+# Based on code from attachment.cgi
+sub attachment_is_visible {
+    my $attachment = shift;
+
+    return (Bugzilla->user->can_see_bug($attachment->bug->id) &&
+            (!$attachment->isprivate ||
+             $user->id == $attachment->attacher->id ||
+             $user->is_insider));
+}
+
+sub attachment_id_is_patch {
+    my $attach_id = shift;
+
+    my $attachment = new Bugzilla::Attachment($attach_id);
+
+    # The check on attachment_is_visible here is to prevent a tiny
+    # information leak where someone could check if a private
+    # attachment was a patch by creating text that would get linkified
+    # differently. Likely excess paranoia
+    return (defined $attachment &&
+            attachment_is_visible ($attachment) &&
+            $attachment->ispatch);
+}
+
+sub get_review_url {
+    my ($bug, $attach_id, $absolute) = @_;
+    my $base = Bugzilla->params->{'splinter_base'};
+    my $bug_id = $bug->id;
+
+    if (defined $absolute && $absolute) {
+	my $urlbase = correct_urlbase();
+	$urlbase =~ s!/$!! if $base =~ "^/";
+	$base = $urlbase . $base;
+    }
+
+    if ($base =~ /\?/) {
+        return "$base&bug=$bug_id&attachment=$attach_id";
+    } else {
+        return "$base?bug=$bug_id&attachment=$attach_id";
+    }
+}
+
+sub get_review_link {
+    my ($bug, $attach_id, $link_text) = @_;
+    return "<a href='" . html_quote(get_review_url($bug, $attach_id)) . "'>$link_text</a>";
+}
diff --git a/extension/lib/WSSplinter.pm b/extension/lib/WSSplinter.pm
index 771a7fd..8bc4c21 100644
--- a/extension/lib/WSSplinter.pm
+++ b/extension/lib/WSSplinter.pm
@@ -27,6 +27,8 @@ use Bugzilla::Constants;
 use Bugzilla::Field;
 use Bugzilla::Util qw(trim);
 
+use extensions::splinter::lib::SplinterUtil;
+
 use base qw(Bugzilla::WebService);
 
 sub info {
@@ -48,15 +50,11 @@ sub info {
 }
 
 # Make sure the current user has access to the specified attachment;
-# cut and paste from attachmnet.cgi
+# Based on cut-and-paste from attachment.cgi
 sub check_can_access {
     my $attachment = shift;
-    my $user = Bugzilla->user;
 
-    # Make sure the user is authorized to access this attachment's bug.
-    Bugzilla::Bug->check($attachment->bug_id);
-    if ($attachment->isprivate && $user->id != $attachment->attacher->id 
-        && !$user->is_insider) 
+    if (!attachment_is_visible ($attachment))
     {
         ThrowUserError('auth_failure', {action => 'access',
                                         object => 'attachment'});