Fix #2261 - Make sure query string parameters are properly escaped

The problem is that URI.escape by default doesn't escape '+' (and
some other characters). But some web framework (at least webrick)
unescape the query string behind Puppet's back changing all '+'
to spaces corrupting facts containing '+' characters (like base64
encoded values).

The current fix makes sure we use CGI.escape for all query string
parameters. Indirection keys/path are still using URI escaping because
this part of the URI format shouldn't be handled like query string
parameters (otherwise '/' url separators are encoded which changes
the uri path).

Signed-off-by: Brice Figureau <brice-puppet@daysofwonder.com>

3 files changed, 7 insertions, 6 deletions

diff --git a/lib/puppet/configurer/fact_handler.rb b/lib/puppet/configurer/fact_handler.rb
index 87176496c..43e9f35f4 100644
--- a/lib/puppet/configurer/fact_handler.rb
+++ b/lib/puppet/configurer/fact_handler.rb
@@ -33,7 +33,7 @@ module Puppet::Configurer::FactHandler
 
         text = facts.render(format)
 
-        return {:facts_format => format, :facts => URI.escape(text)}
+        return {:facts_format => format, :facts => CGI.escape(text)}
     end
 
     # Retrieve facts from the central server.
diff --git a/lib/puppet/indirector/request.rb b/lib/puppet/indirector/request.rb
index 2ffed60e2..d9e66cb5b 100644
--- a/lib/puppet/indirector/request.rb
+++ b/lib/puppet/indirector/request.rb
@@ -1,3 +1,4 @@
+require 'cgi'
 require 'uri'
 require 'puppet/indirector'
 
@@ -115,9 +116,9 @@ class Puppet::Indirector::Request
             when nil; next
             when true, false; value = value.to_s
             when Fixnum, Bignum, Float; value = value # nothing
-            when String; value = URI.escape(value)
-            when Symbol; value = URI.escape(value.to_s)
-            when Array; value = URI.escape(YAML.dump(value))
+            when String; value = CGI.escape(value)
+            when Symbol; value = CGI.escape(value.to_s)
+            when Array; value = CGI.escape(YAML.dump(value))
             else
                 raise ArgumentError, "HTTP REST queries cannot handle values of type '%s'" % value.class
             end
@@ -159,7 +160,7 @@ class Puppet::Indirector::Request
         begin
             uri = URI.parse(URI.escape(key))
         rescue => detail
-            raise ArgumentError, "Could not understand URL %s: %s" % [source, detail.to_s]
+            raise ArgumentError, "Could not understand URL %s: %s" % [key, detail.to_s]
         end
 
         # Just short-circuit these to full paths
diff --git a/lib/puppet/network/http/handler.rb b/lib/puppet/network/http/handler.rb
index 4df2c4141..27e8dbd1d 100644
--- a/lib/puppet/network/http/handler.rb
+++ b/lib/puppet/network/http/handler.rb
@@ -208,7 +208,7 @@ module Puppet::Network::HTTP::Handler
             # in the query string, for security reasons.
             next result if param == :node
             next result if param == :ip
-            value = URI.unescape(value)
+            value = CGI.unescape(value)
             if value =~ /^---/
                 value = YAML.load(value)
             else