From cd3be7b750394c4a2273bb109c16839f2537a8c2 Mon Sep 17 00:00:00 2001
From: Vitaliy Filippov <vitalif@yourcmc.ru>
Date: Thu, 24 Jul 2014 18:32:50 +0400
Subject: [PATCH] Move clean_search_url out of CGI

---
 Bugzilla/CGI.pm          | 65 ----------------------------------------
 Bugzilla/Report.pm       | 10 +++----
 Bugzilla/Search.pm       | 64 +++++++++++++++++++++++++++++++++++++++
 Bugzilla/Search/Saved.pm |  8 ++---
 buglist.cgi              |  8 ++---
 5 files changed, 76 insertions(+), 79 deletions(-)

diff --git a/Bugzilla/CGI.pm b/Bugzilla/CGI.pm
index e04a27c60..f0cfb6325 100644
--- a/Bugzilla/CGI.pm
+++ b/Bugzilla/CGI.pm
@@ -148,71 +148,6 @@ sub canonicalise_query
     return join("&", @parameters);
 }
 
-# FIXME: Move clean_search_url away from CGI to search-related code.
-sub clean_search_url
-{
-    my $self = shift;
-    # Delete any empty URL parameter.
-    my @cgi_params = $self->param;
-
-    foreach my $param (@cgi_params)
-    {
-        if (defined $self->param($param) && $self->param($param) eq '')
-        {
-            $self->delete($param);
-            $self->delete("${param}_type");
-        }
-
-        # Boolean Chart stuff is empty if it's "noop"
-        if ($param =~ /\d-\d-\d/ && defined $self->param($param) && $self->param($param) eq 'noop')
-        {
-            $self->delete($param);
-        }
-    }
-
-    # Delete leftovers from the login form
-    $self->delete('Bugzilla_remember', 'GoAheadAndLogIn');
-
-    foreach my $num (1, 2)
-    {
-        # If there's no value in the email field, delete the related fields.
-        if (!$self->param("email$num"))
-        {
-            foreach my $field (qw(type assigned_to reporter qa_contact cc longdesc))
-            {
-                $self->delete("email$field$num");
-            }
-        }
-    }
-
-    # chfieldto is set to "Now" by default in query.cgi.
-    if (lc($self->param('chfieldto')) eq 'now')
-    {
-        $self->delete('chfieldto');
-    }
-
-    # cmdtype "doit" is the default from query.cgi, but it's only meaningful
-    # if there's a remtype parameter.
-    if (defined $self->param('cmdtype') && $self->param('cmdtype') eq 'doit' &&
-        !defined $self->param('remtype'))
-    {
-        $self->delete('cmdtype');
-    }
-
-    # "Reuse same sort as last time" is actually the default, so we don't
-    # need it in the URL.
-    if ($self->param('order') && $self->param('order') eq 'Reuse same sort as last time')
-    {
-        $self->delete('order');
-    }
-
-    # And now finally, if query_format is our only parameter, that
-    # really means we have no parameters, so we should delete query_format.
-    if ($self->param('query_format') && scalar($self->param()) == 1) {
-        $self->delete('query_format');
-    }
-}
-
 # Overwrite to ensure nph doesn't get set, and unset HEADERS_ONCE
 sub multipart_init
 {
diff --git a/Bugzilla/Report.pm b/Bugzilla/Report.pm
index efd61e7ba..bbd3a44a2 100644
--- a/Bugzilla/Report.pm
+++ b/Bugzilla/Report.pm
@@ -15,6 +15,7 @@ use Bugzilla::CGI;
 use Bugzilla::Constants;
 use Bugzilla::Error;
 use Bugzilla::Util;
+use Bugzilla::Search;
 
 use constant DB_TABLE => 'reports';
 
@@ -61,12 +62,11 @@ sub _check_name {
     return $name;
 }
 
-sub _check_query {
+sub _check_query
+{
     my ($invocant, $query) = @_;
-    $query || ThrowUserError("buglist_parameters_required");
-    my $cgi = new Bugzilla::CGI($query);
-    $cgi->clean_search_url;
-    return $cgi->query_string;
+    $query || ThrowUserError('buglist_parameters_required');
+    return http_build_query(Bugzilla::Search->clean_search_params(http_decode_query($query)));
 }
 
 #############
diff --git a/Bugzilla/Search.pm b/Bugzilla/Search.pm
index a070c1935..d75a40fcd 100644
--- a/Bugzilla/Search.pm
+++ b/Bugzilla/Search.pm
@@ -999,6 +999,70 @@ sub new
     return $self;
 }
 
+sub clean_search_params
+{
+    shift if $_[0] eq __PACKAGE__;
+    my ($params) = @_;
+
+    # Delete any empty URL parameter.
+    foreach my $key (keys %$params)
+    {
+        if (!defined $params->{$key} || $params->{$key} eq '')
+        {
+            delete $params->{$key};
+            delete $params->{$key.'_type'};
+        }
+        elsif ($key =~ /\d-\d-\d/ && (!defined $params->{$key} || $params->{$key} eq 'noop'))
+        {
+            # Boolean Chart stuff is empty if it's "noop"
+            delete $params->{$key};
+        }
+    }
+
+    # Delete leftovers from the login form
+    delete $params->{$_} for qw(Bugzilla_remember GoAheadAndLogIn);
+
+    foreach my $num (1, 2)
+    {
+        # If there's no value in the email field, delete the related fields.
+        if (!$params->{"email$num"})
+        {
+            foreach my $field (qw(type assigned_to reporter qa_contact cc longdesc))
+            {
+                delete $params->{"email$field$num"};
+            }
+        }
+    }
+
+    # chfieldto is set to "Now" by default in query.cgi.
+    if (lc($params->{chfieldto} || '') eq 'now')
+    {
+        delete $params->{chfieldto};
+    }
+
+    # cmdtype "doit" is the default from query.cgi, but it's only meaningful
+    # if there's a remtype parameter.
+    if (defined $params->{cmdtype} && $params->{cmdtype} eq 'doit' && !defined $params->{remtype})
+    {
+        delete $params->{cmdtype};
+    }
+
+    # "Reuse same sort as last time" is actually the default, so we don't need it in the URL.
+    if (($params->{order} || '') eq 'Reuse same sort as last time')
+    {
+        delete $params->{order};
+    }
+
+    # And now finally, if query_format is our only parameter, that
+    # really means we have no parameters, so we should delete query_format.
+    if ($params->{query_format} && scalar(keys %$params) == 1)
+    {
+        delete $params->{query_format};
+    }
+
+    return $params;
+}
+
 # The main Bugzilla::Search function - parses search
 # parameters, builds search terms and SQL query
 sub init
diff --git a/Bugzilla/Search/Saved.pm b/Bugzilla/Search/Saved.pm
index ba63bcc5c..87bf02263 100644
--- a/Bugzilla/Search/Saved.pm
+++ b/Bugzilla/Search/Saved.pm
@@ -33,6 +33,7 @@ use Bugzilla::Group;
 use Bugzilla::Error;
 use Bugzilla::User;
 use Bugzilla::Util;
+use Bugzilla::Search;
 
 use Scalar::Util qw(blessed);
 
@@ -137,11 +138,10 @@ sub _check_name {
 sub _check_query {
     my ($invocant, $query) = @_;
     $query || ThrowUserError("buglist_parameters_required");
-    my $cgi = new Bugzilla::CGI($query);
-    $cgi->clean_search_url;
+    my $params = http_decode_query($query);
     # Don't store the query name as a parameter.
-    $cgi->delete('known_name');
-    return $cgi->query_string;
+    delete $params->{known_name};
+    return http_build_query(Bugzilla::Search->clean_search_params($params));
 }
 
 #########################
diff --git a/buglist.cgi b/buglist.cgi
index 40ad68a73..6b04c897d 100755
--- a/buglist.cgi
+++ b/buglist.cgi
@@ -99,14 +99,12 @@ if (grep { $_ =~ /^cmd\-/ } $cgi->param())
 
 # If query was POSTed, clean the URL from empty parameters and redirect back to
 # itself. This will make advanced search URLs more tolerable.
-#
 if ($cgi->request_method() eq 'POST')
 {
-    $cgi->clean_search_url();
-    my $uri_length = length($cgi->self_url());
-    if ($uri_length < CGI_URI_LIMIT)
+    my $clean = http_build_query(Bugzilla::Search->clean_search_params({ %{ $cgi->Vars } }));
+    if (length($clean) < CGI_URI_LIMIT)
     {
-        print $cgi->redirect(-url => $cgi->self_url());
+        print $cgi->redirect(-url => $clean);
         exit;
     }
 }