Bug 759030: Clean up Bugzilla::BugUrl modules

r=timello a=LpSolit

7 files changed, 58 insertions, 96 deletions

diff --git a/Bugzilla/BugUrl/Debian.pm b/Bugzilla/BugUrl/Debian.pm
index c11a49910..78397bdd9 100644
--- a/Bugzilla/BugUrl/Debian.pm
+++ b/Bugzilla/BugUrl/Debian.pm
@@ -9,16 +9,20 @@ package Bugzilla::BugUrl::Debian;
 use strict;
 use base qw(Bugzilla::BugUrl);
 
-use Bugzilla::Error;
-use Bugzilla::Util;
-
 ###############################
 ####        Methods        ####
 ###############################
 
 sub should_handle {
     my ($class, $uri) = @_;
-    return ($uri->authority =~ /^bugs.debian.org$/i) ? 1 : 0;
+
+    # Debian BTS URLs can look like various things:
+    #   http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=1234
+    #   http://bugs.debian.org/1234
+    return ($uri->authority =~ /^bugs.debian.org$/i
+            and (($uri->path =~ /bugreport\.cgi$/
+                  and $uri->query_param('bug') =~ m|^\d+$|)
+                 or $uri->path =~ m|^/\d+$|)) ? 1 : 0;
 }
 
 sub _check_value {
@@ -26,24 +30,12 @@ sub _check_value {
 
     my $uri = $class->SUPER::_check_value(@_);
 
-    # Debian BTS URLs can look like various things:
-    #   http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=1234
-    #   http://bugs.debian.org/1234
-    my $bug_id;
-    if ($uri->path =~ m|^/(\d+)$|) {
-        $bug_id = $1;
-    }
-    elsif ($uri->path =~ /bugreport\.cgi$/) {
-        $bug_id = $uri->query_param('bug');
-        detaint_natural($bug_id);
-    }
-    if (!$bug_id) {
-        ThrowUserError('bug_url_invalid',
-                       { url => $uri->path, reason => 'id' });
-    }
     # This is the shortest standard URL form for Debian BTS URLs,
     # and so we reduce all URLs to this.
-    return new URI("http://bugs.debian.org/" . $bug_id);
+    $uri->path =~ m|^/(\d+)$| || $uri->query_param('bug') =~ m|^(\d+)$|;
+    $uri = new URI("http://bugs.debian.org/$1");
+
+    return $uri;
 }
 
 1;
diff --git a/Bugzilla/BugUrl/Google.pm b/Bugzilla/BugUrl/Google.pm
index 08722c94f..8b8638c7a 100644
--- a/Bugzilla/BugUrl/Google.pm
+++ b/Bugzilla/BugUrl/Google.pm
@@ -9,16 +9,18 @@ package Bugzilla::BugUrl::Google;
 use strict;
 use base qw(Bugzilla::BugUrl);
 
-use Bugzilla::Error;
-use Bugzilla::Util;
-
 ###############################
 ####        Methods        ####
 ###############################
 
 sub should_handle {
     my ($class, $uri) = @_;
-    return ($uri->authority =~ /^code.google.com$/i) ? 1 : 0;
+
+    # Google Code URLs only have one form:
+    #   http(s)://code.google.com/p/PROJECT_NAME/issues/detail?id=1234
+    return ($uri->authority =~ /^code.google.com$/i
+            and $uri->path =~ m|^/p/[^/]+/issues/detail$|
+            and $uri->query_param('id') =~ /^\d+$/) ? 1 : 0;
 }
 
 sub _check_value {
@@ -26,26 +28,13 @@ sub _check_value {
     
     $uri = $class->SUPER::_check_value($uri);
 
-    my $value = $uri->as_string;
-    # Google Code URLs only have one form:
-    #   http(s)://code.google.com/p/PROJECT_NAME/issues/detail?id=1234
-    my $project_name;
-    if ($uri->path =~ m|^/p/([^/]+)/issues/detail$|) {
-        $project_name = $1;
-    } else {
-        ThrowUserError('bug_url_invalid', { url => $value });
-    }
-    my $bug_id = $uri->query_param('id');
-    detaint_natural($bug_id);
-    if (!$bug_id) {
-        ThrowUserError('bug_url_invalid', { url => $value, reason => 'id' });
-    }
     # While Google Code URLs can be either HTTP or HTTPS,
     # always go with the HTTP scheme, as that's the default.
-    $value = "http://code.google.com/p/" . $project_name .
-             "/issues/detail?id=" . $bug_id;
+    if ($uri->scheme eq 'https') {
+        $uri->scheme('http');
+    }
 
-    return new URI($value);
+    return $uri;
 }
 
 1;
diff --git a/Bugzilla/BugUrl/JIRA.pm b/Bugzilla/BugUrl/JIRA.pm
index 8a7b90c3e..f5f7ee5fa 100644
--- a/Bugzilla/BugUrl/JIRA.pm
+++ b/Bugzilla/BugUrl/JIRA.pm
@@ -9,15 +9,16 @@ package Bugzilla::BugUrl::JIRA;
 use strict;
 use base qw(Bugzilla::BugUrl);
 
-use Bugzilla::Error;
-use Bugzilla::Util;
-
 ###############################
 ####        Methods        ####
 ###############################
 
 sub should_handle {
     my ($class, $uri) = @_;
+
+    # JIRA URLs have only one basic form (but the jira is optional):
+    #   https://issues.apache.org/jira/browse/KEY-1234
+    #   http://issues.example.com/browse/KEY-1234
     return ($uri->path =~ m|/browse/[A-Z][A-Z]+-\d+$|) ? 1 : 0;
 }
 
@@ -26,10 +27,6 @@ sub _check_value {
 
     my $uri = $class->SUPER::_check_value(@_);
 
-    # JIRA URLs have only one basic form (but the jira is optional):
-    #   https://issues.apache.org/jira/browse/KEY-1234
-    #   http://issues.example.com/browse/KEY-1234
-
     # Make sure there are no query parameters.
     $uri->query(undef);
     # And remove any # part if there is one.
diff --git a/Bugzilla/BugUrl/Launchpad.pm b/Bugzilla/BugUrl/Launchpad.pm
index 37a238be4..87fb71a5d 100644
--- a/Bugzilla/BugUrl/Launchpad.pm
+++ b/Bugzilla/BugUrl/Launchpad.pm
@@ -9,15 +9,19 @@ package Bugzilla::BugUrl::Launchpad;
 use strict;
 use base qw(Bugzilla::BugUrl);
 
-use Bugzilla::Error;
-
 ###############################
 ####        Methods        ####
 ###############################
 
 sub should_handle {
     my ($class, $uri) = @_;
-    return ($uri->authority =~ /launchpad.net$/) ? 1 : 0;
+
+    # Launchpad bug URLs can look like various things:
+    #   https://bugs.launchpad.net/ubuntu/+bug/1234
+    #   https://launchpad.net/bugs/1234
+    # All variations end with either "/bugs/1234" or "/+bug/1234"
+    return ($uri->authority =~ /launchpad.net$/
+            and $uri->path =~ m|bugs?/\d+$|) ? 1 : 0;
 }
 
 sub _check_value {
@@ -25,21 +29,12 @@ sub _check_value {
 
     $uri = $class->SUPER::_check_value($uri);
 
-    my $value = $uri->as_string;
-    # Launchpad bug URLs can look like various things:
-    #   https://bugs.launchpad.net/ubuntu/+bug/1234
-    #   https://launchpad.net/bugs/1234
-    # All variations end with either "/bugs/1234" or "/+bug/1234"
-    if ($uri->path =~ m|bugs?/(\d+)$|) {
-        # This is the shortest standard URL form for Launchpad bugs,
-        # and so we reduce all URLs to this.
-        $value = "https://launchpad.net/bugs/$1";
-    }
-    else {
-        ThrowUserError('bug_url_invalid', { url => $value, reason => 'id' });
-    }
-
-    return new URI($value);
+    # This is the shortest standard URL form for Launchpad bugs,
+    # and so we reduce all URLs to this.
+    $uri->path =~ m|bugs?/(\d+)$|;
+    $uri = new URI("https://launchpad.net/bugs/$1");
+
+    return $uri;
 }
 
 1;
diff --git a/Bugzilla/BugUrl/MantisBT.pm b/Bugzilla/BugUrl/MantisBT.pm
index 02a17a64d..3d49ede69 100644
--- a/Bugzilla/BugUrl/MantisBT.pm
+++ b/Bugzilla/BugUrl/MantisBT.pm
@@ -9,15 +9,15 @@ package Bugzilla::BugUrl::MantisBT;
 use strict;
 use base qw(Bugzilla::BugUrl);
 
-use Bugzilla::Error;
-use Bugzilla::Util;
-
 ###############################
 ####        Methods        ####
 ###############################
 
 sub should_handle {
     my ($class, $uri) = @_;
+
+    # MantisBT URLs look like the following ('bugs' directory is optional):
+    #   http://www.mantisbt.org/bugs/view.php?id=1234
     return ($uri->path_query =~ m|view\.php\?id=\d+$|) ? 1 : 0;
 }
 
@@ -26,9 +26,6 @@ sub _check_value {
 
     my $uri = $class->SUPER::_check_value(@_);
 
-    # MantisBT URLs look like the following ('bugs' directory is optional):
-    #   http://www.mantisbt.org/bugs/view.php?id=1234
-
     # Remove any # part if there is one.
     $uri->fragment(undef);
 
diff --git a/Bugzilla/BugUrl/SourceForge.pm b/Bugzilla/BugUrl/SourceForge.pm
index 69d4f98c2..11cdd0ff1 100644
--- a/Bugzilla/BugUrl/SourceForge.pm
+++ b/Bugzilla/BugUrl/SourceForge.pm
@@ -9,17 +9,21 @@ package Bugzilla::BugUrl::SourceForge;
 use strict;
 use base qw(Bugzilla::BugUrl);
 
-use Bugzilla::Error;
-use Bugzilla::Util;
-
 ###############################
 ####        Methods        ####
 ###############################
 
 sub should_handle {
     my ($class, $uri) = @_;
+
+    # SourceForge tracker URLs have only one form:
+    #  http://sourceforge.net/tracker/?func=detail&aid=111&group_id=111&atid=111
     return ($uri->authority =~ /^sourceforge.net$/i
-            and $uri->path =~ m|/tracker/|) ? 1 : 0;
+            and $uri->path =~ m|/tracker/|
+            and $uri->query_param('func') eq 'detail'
+            and $uri->query_param('aid')
+            and $uri->query_param('group_id')
+            and $uri->query_param('atid')) ? 1 : 0;
 }
 
 sub _check_value {
@@ -27,19 +31,10 @@ sub _check_value {
 
     my $uri = $class->SUPER::_check_value(@_);
 
-    # SourceForge tracker URLs have only one form:
-    #  http://sourceforge.net/tracker/?func=detail&aid=111&group_id=111&atid=111
-    if ($uri->query_param('func') eq 'detail' and $uri->query_param('aid')
-        and $uri->query_param('group_id') and $uri->query_param('atid'))
-    {
-        # Remove any # part if there is one.
-        $uri->fragment(undef);
-        return $uri;
-    }
-    else {
-        my $value = $uri->as_string;
-        ThrowUserError('bug_url_invalid', { url => $value });
-    }
+    # Remove any # part if there is one.
+    $uri->fragment(undef);
+
+    return $uri;
 }
 
 1;
diff --git a/Bugzilla/BugUrl/Trac.pm b/Bugzilla/BugUrl/Trac.pm
index 500d99909..8f6e9cd0e 100644
--- a/Bugzilla/BugUrl/Trac.pm
+++ b/Bugzilla/BugUrl/Trac.pm
@@ -9,15 +9,16 @@ package Bugzilla::BugUrl::Trac;
 use strict;
 use base qw(Bugzilla::BugUrl);
 
-use Bugzilla::Error;
-use Bugzilla::Util;
-
 ###############################
 ####        Methods        ####
 ###############################
 
 sub should_handle {
     my ($class, $uri) = @_;
+
+    # Trac URLs can look like various things:
+    #   http://dev.mutt.org/trac/ticket/1234
+    #   http://trac.roundcube.net/ticket/1484130
     return ($uri->path =~ m|/ticket/\d+$|) ? 1 : 0;
 }
 
@@ -26,10 +27,6 @@ sub _check_value {
 
     my $uri = $class->SUPER::_check_value(@_);
 
-    # Trac URLs can look like various things:
-    #   http://dev.mutt.org/trac/ticket/1234
-    #   http://trac.roundcube.net/ticket/1484130
-
     # Make sure there are no query parameters.
     $uri->query(undef);
     # And remove any # part if there is one.