Replace URI.encode, removed in Ruby 3.0, with Fog::OpenStack.escape

fog-openstack.gemspec

@@ -24,6 +24,9 @@ Gem::Specification.new do |spec|
   spec.add_dependency 'fog-core',  '~> 2.1'
   spec.add_dependency 'fog-json',  '>= 1.0'
 
+  # base64 became a bundled gem in Ruby 3.4, so it must be declared explicitly.
+  # webmock requires it but does not depend on it.
+  spec.add_development_dependency 'base64'
   spec.add_development_dependency 'bundler'
   spec.add_development_dependency 'coveralls'
   spec.add_development_dependency "mime-types"

lib/fog/openstack/compute/requests/delete_service.rb

@@ -4,7 +4,7 @@ class Compute
       class Real
         def delete_service(uuid, optional_params = nil)
           # Encode all params
-          optional_params = optional_params.each { |k, v| optional_params[k] = URI.encode(v) } if optional_params
+          optional_params = optional_params.transform_values { |v| Fog::OpenStack.escape(v) } if optional_params
 
           request(
             :expects => [202, 204],

lib/fog/openstack/compute/requests/disable_service.rb

@@ -6,7 +6,7 @@ def disable_service(host, binary, optional_params = nil)
           data = {"host" => host, "binary" => binary}
 
           # Encode all params
-          optional_params = optional_params.each { |k, v| optional_params[k] = URI.encode(v) } if optional_params
+          optional_params = optional_params.transform_values { |v| Fog::OpenStack.escape(v) } if optional_params
 
           request(
             :body    => Fog::JSON.encode(data),

lib/fog/openstack/compute/requests/disable_service_log_reason.rb

@@ -6,7 +6,7 @@ def disable_service_log_reason(host, binary, disabled_reason, optional_params =
           data = {"host" => host, "binary" => binary, "disabled_reason" => disabled_reason}
 
           # Encode all params
-          optional_params = optional_params.each { |k, v| optional_params[k] = URI.encode(v) } if optional_params
+          optional_params = optional_params.transform_values { |v| Fog::OpenStack.escape(v) } if optional_params
 
           request(
             :body    => Fog::JSON.encode(data),

lib/fog/openstack/compute/requests/enable_service.rb

@@ -6,7 +6,7 @@ def enable_service(host, binary, optional_params = nil)
           data = {"host" => host, "binary" => binary}
 
           # Encode all params
-          optional_params = optional_params.each { |k, v| optional_params[k] = URI.encode(v) } if optional_params
+          optional_params = optional_params.transform_values { |v| Fog::OpenStack.escape(v) } if optional_params
 
           request(
             :body    => Fog::JSON.encode(data),

lib/fog/openstack/storage/requests/delete_multiple_objects.rb

@@ -45,7 +45,7 @@ class Real
         def delete_multiple_objects(container, object_names, options = {})
           body = object_names.map do |name|
             object_name = container ? "#{container}/#{name}" : name
-            URI.encode(object_name)
+            Fog::OpenStack.escape(object_name, '/')
           end.join("\n")
 
           response = request({

lib/fog/openstack/workflow/v2/requests/delete_action.rb

@@ -7,7 +7,7 @@ def delete_action(name)
             request(
               :expects => 204,
               :method  => "DELETE",
-              :path    => "actions/#{URI.encode(name)}"
+              :path    => "actions/#{Fog::OpenStack.escape(name)}"
             )
           end
         end

lib/fog/openstack/workflow/v2/requests/delete_cron_trigger.rb

@@ -7,7 +7,7 @@ def delete_cron_trigger(name)
             request(
               :expects => 204,
               :method  => "DELETE",
-              :path    => "cron_triggers/#{URI.encode(name)}"
+              :path    => "cron_triggers/#{Fog::OpenStack.escape(name)}"
             )
           end
         end

lib/fog/openstack/workflow/v2/requests/delete_environment.rb

@@ -7,7 +7,7 @@ def delete_environment(name)
             request(
               :expects => 204,
               :method  => "DELETE",
-              :path    => "environments/#{URI.encode(name)}"
+              :path    => "environments/#{Fog::OpenStack.escape(name)}"
             )
           end
         end

lib/fog/openstack/workflow/v2/requests/delete_workbook.rb

@@ -7,7 +7,7 @@ def delete_workbook(name)
             request(
               :expects => 204,
               :method  => "DELETE",
-              :path    => "workbooks/#{URI.encode(name)}"
+              :path    => "workbooks/#{Fog::OpenStack.escape(name)}"
             )
           end
         end

lib/fog/openstack/workflow/v2/requests/get_action.rb

@@ -7,7 +7,7 @@ def get_action(name)
             request(
               :expects => 200,
               :method  => "GET",
-              :path    => "actions/#{URI.encode(name)}"
+              :path    => "actions/#{Fog::OpenStack.escape(name)}"
             )
           end
         end

lib/fog/openstack/workflow/v2/requests/get_cron_trigger.rb

@@ -7,7 +7,7 @@ def get_cron_trigger(name)
             request(
               :expects => 200,
               :method  => "GET",
-              :path    => "cron_triggers/#{URI.encode(name)}"
+              :path    => "cron_triggers/#{Fog::OpenStack.escape(name)}"
             )
           end
         end

lib/fog/openstack/workflow/v2/requests/get_environment.rb

@@ -7,7 +7,7 @@ def get_environment(name)
             request(
               :expects => 200,
               :method  => "GET",
-              :path    => "environments/#{URI.encode(name)}"
+              :path    => "environments/#{Fog::OpenStack.escape(name)}"
             )
           end
         end

lib/fog/openstack/workflow/v2/requests/get_workbook.rb

@@ -7,7 +7,7 @@ def get_workbook(name)
             request(
               :expects => 200,
               :method  => "GET",
-              :path    => "workbooks/#{URI.encode(name)}"
+              :path    => "workbooks/#{Fog::OpenStack.escape(name)}"
             )
           end
         end

spec/escaping_spec.rb

@@ -0,0 +1,98 @@
+require 'spec_helper'
+
+require 'fog/openstack/compute/requests/enable_service'
+require 'fog/openstack/storage/requests/delete_multiple_objects'
+require 'fog/openstack/workflow/v2/requests/get_action'
+
+# Builds a Real instance without running its initializer (which would need
+# credentials and a live endpoint) and captures what it hands to #request.
+def capture_request(klass, response_body = '{}')
+  instance = klass.allocate
+  captured = nil
+  instance.define_singleton_method(:request) do |params, *_args|
+    captured = params
+    Excon::Response.new(:body => response_body)
+  end
+  yield instance
+  captured
+end
+
+describe 'URL escaping' do
+  describe 'Fog::OpenStack.escape' do
+    # This is why these call sites cannot use CGI.escape: in a URL *path* a
+    # '+' is a literal plus, not a space, so CGI.escape corrupts any name
+    # containing a space.
+    it 'encodes a space as %20 rather than +' do
+      assert_equal 'sp%20ace', Fog::OpenStack.escape('sp ace')
+      assert_equal 'sp+ace', CGI.escape('sp ace')
+    end
+
+    it 'leaves unreserved characters alone' do
+      assert_equal 'name-with.dots_and-dashes', Fog::OpenStack.escape('name-with.dots_and-dashes')
+    end
+
+    it 'escapes a slash by default but can be told to keep it' do
+      assert_equal 'a%2Fb', Fog::OpenStack.escape('a/b')
+      assert_equal 'a/b', Fog::OpenStack.escape('a/b', '/')
+    end
+  end
+
+  describe 'Workflow::V2 resource names in the path' do
+    it 'escapes the name into a single path segment' do
+      captured = capture_request(Fog::OpenStack::Workflow::V2::Real) do |real|
+        real.get_action('my action')
+      end
+      assert_equal 'actions/my%20action', captured[:path]
+    end
+
+    it 'escapes a slash inside the name so it cannot forge a path segment' do
+      captured = capture_request(Fog::OpenStack::Workflow::V2::Real) do |real|
+        real.get_action('a/b')
+      end
+      assert_equal 'actions/a%2Fb', captured[:path]
+    end
+  end
+
+  describe 'Storage#delete_multiple_objects' do
+    it 'escapes each name but keeps the container/object separator intact' do
+      captured = capture_request(Fog::OpenStack::Storage::Real) do |real|
+        real.delete_multiple_objects('my container', ['some object', 'plain'])
+      end
+      assert_equal "my%20container/some%20object\nmy%20container/plain", captured[:body]
+    end
+
+    it 'escapes bare names when no container is given' do
+      captured = capture_request(Fog::OpenStack::Storage::Real) do |real|
+        real.delete_multiple_objects(nil, ['some object'])
+      end
+      assert_equal 'some%20object', captured[:body]
+    end
+  end
+
+  describe 'Compute service optional params' do
+    it 'escapes the values' do
+      captured = capture_request(Fog::OpenStack::Compute::Real) do |real|
+        real.enable_service('host1', 'nova-compute', 'reason' => 'out of service')
+      end
+      assert_equal({'reason' => 'out%20of%20service'}, captured[:query])
+    end
+
+    it 'does not mutate the hash the caller passed in' do
+      params = {'reason' => 'out of service'}
+      capture_request(Fog::OpenStack::Compute::Real) do |real|
+        real.enable_service('host1', 'nova-compute', params)
+      end
+      assert_equal({'reason' => 'out of service'}, params)
+    end
+  end
+
+  describe 'the whole library' do
+    it 'no longer calls URI.encode or URI.escape, removed in Ruby 3.0' do
+      lib = File.expand_path('../lib', __dir__)
+      offenders = Dir.glob("#{lib}/**/*.rb").select do |file|
+        File.read(file).match?(/\bURI\.(encode|escape)\b/)
+      end
+      assert_empty(offenders.map { |f| f.sub("#{lib}/", '') })
+    end
+  end
+end