From: Dagfinn Ilmari Mannsåker <ilmari@ilmari.org>
Date: Mon, 11 Sep 2017 14:19:44 +0000 (+0100)
Subject: Fix double-encoding of spaces in query parameter keys in ->uri_for
X-Git-Tag: 5.90116~17
X-Git-Url: http://git.shadowcat.co.uk/gitweb/gitweb.cgi?p=catagits%2FCatalyst-Runtime.git;a=commitdiff_plain;h=901b4331956b16740851040d0b9c109066a05620

Fix double-encoding of spaces in query parameter keys in ->uri_for

Commit 5c779e9841d052721162a48ad96fdbda2acd1035 moved half of the key-
encoding part into the map over the values, but left the `s/ /+/g` in
place, causing spaces in query parameter keys to first be converted to
+, then to %2B.

This moves the current key encoding block out of the inner map and
removes the premature `s/ /+g/`.
---

diff --git a/lib/Catalyst.pm b/lib/Catalyst.pm
index 77bd7c7..b92f01f 100644
--- a/lib/Catalyst.pm
+++ b/lib/Catalyst.pm
@@ -1701,23 +1701,20 @@ sub uri_for {
       # somewhat lifted from URI::_query's query_form
       $query = '?'.join('&', map {
           my $val = $params->{$_};
-          #s/([;\/?:@&=+,\$\[\]%])/$URI::Escape::escapes{$1}/go; ## Commented out because seems to lead to double encoding - JNAP
-          s/ /+/g;
-          my $key = $_;
+          my $key = encode_utf8($_);
+          # using the URI::Escape pattern here so utf8 chars survive
+          $key =~ s/([^A-Za-z0-9\-_.!~*'() ])/$URI::Escape::escapes{$1}/go;
+          $key =~ s/ /+/g;
+
           $val = '' unless defined $val;
           (map {
-              my $param = "$_";
-              $param = encode_utf8($param);
+              my $param = encode_utf8($_);
               # using the URI::Escape pattern here so utf8 chars survive
               $param =~ s/([^A-Za-z0-9\-_.!~*'() ])/$URI::Escape::escapes{$1}/go;
               $param =~ s/ /+/g;
 
-              $key = encode_utf8($key);
-              # using the URI::Escape pattern here so utf8 chars survive
-              $key =~ s/([^A-Za-z0-9\-_.!~*'() ])/$URI::Escape::escapes{$1}/go;
-              $key =~ s/ /+/g;
-
-              "${key}=$param"; } ( ref $val eq 'ARRAY' ? @$val : $val ));
+              "${key}=$param";
+          } ( ref $val eq 'ARRAY' ? @$val : $val ));
       } @keys);
     }
 
diff --git a/t/aggregate/unit_core_uri_for.t b/t/aggregate/unit_core_uri_for.t
index ab256f0..887aa4a 100644
--- a/t/aggregate/unit_core_uri_for.t
+++ b/t/aggregate/unit_core_uri_for.t
@@ -60,6 +60,12 @@ is(
 );
 
 is(
+    Catalyst::uri_for( $context, '/bar', 'with space', { 'also with' => 'space here' })->as_string,
+    'http://127.0.0.1/foo/bar/with%20space?also+with=space+here',
+    'Spaces encoded correctly'
+);
+
+is(
     Catalyst::uri_for( $context, '/bar#fragment', { param1 => 'value1' } )->as_string,
     'http://127.0.0.1/foo/bar?param1=value1#fragment',
     'URI for path with fragment and query params 1'
@@ -127,6 +133,12 @@ is(
     'Plus is not encoded, called with only class name'
 );
 
+is(
+    Catalyst::uri_for( 'TestApp', '/bar', 'with space', { 'also with' => 'space here' })->as_string,
+    '/bar/with%20space?also+with=space+here',
+    'Spaces encoded correctly, called with only class name'
+);
+
 TODO: {
     local $TODO = 'broken by 5.7008';
     is(