Do not escape selenium-manager arguments

p0deje · p0deje · commit 1a22a8b9315d · 2023-03-29T08:17:14.000-05:00
diff --git a/rb/Gemfile.lock b/rb/Gemfile.lock
@@ -3,7 +3,7 @@ PATH
   specs:
     selenium-devtools (0.111.0)
       selenium-webdriver (~> 4.2)
-    selenium-webdriver (4.8.3)
+    selenium-webdriver (4.8.5)
       rexml (~> 3.2, >= 3.2.5)
       rubyzip (>= 1.2.2, < 3.0)
       websocket (~> 1.0)
diff --git a/rb/lib/selenium/webdriver/common/selenium_manager.rb b/rb/lib/selenium/webdriver/common/selenium_manager.rb
@@ -40,18 +40,18 @@ def driver_path(options)
             raise ArgumentError, "SeleniumManager requires a WebDriver::Options instance, not #{options.inspect}"
           end
 
-          command = [binary, '--browser', "'#{options.browser_name}'", '--output', 'json']
+          command = [binary, '--browser', options.browser_name, '--output', 'json']
           if options.browser_version
             command << '--browser-version'
             command << options.browser_version
           end
           if options.respond_to?(:binary) && !options.binary.nil?
             command << '--browser-path'
-            command << "\"#{options.binary.gsub('\ ', ' ').gsub(' ', '\ ')}\""
+            command << options.binary
           end
           command << '--debug' if WebDriver.logger.debug?
 
-          location = run(command.join(' '))
+          location = run(*command)
           WebDriver.logger.debug("Driver found at #{location}")
           Platform.assert_executable location
 
@@ -81,11 +81,11 @@ def binary
           end
         end
 
-        def run(command)
+        def run(*command)
           WebDriver.logger.debug("Executing Process #{command}")
 
           begin
-            stdout, stderr, status = Open3.capture3(command)
+            stdout, stderr, status = Open3.capture3(*command)
             json_output = stdout.empty? ? nil : JSON.parse(stdout)
             result = json_output&.dig('result', 'message')
           rescue StandardError => e
diff --git a/rb/spec/unit/selenium/webdriver/common/selenium_manager_spec.rb b/rb/spec/unit/selenium/webdriver/common/selenium_manager_spec.rb
@@ -85,7 +85,8 @@ module WebDriver
 
           described_class.driver_path(Options.chrome)
 
-          expect(described_class).to have_received(:run).with("selenium-manager --browser 'chrome' --output json")
+          expect(described_class).to have_received(:run)
+            .with('selenium-manager', '--browser', 'chrome', '--output', 'json')
         end
 
         it 'uses browser version if specified' do
@@ -97,7 +98,7 @@ module WebDriver
           described_class.driver_path(options)
 
           expect(described_class).to have_received(:run)
-            .with("selenium-manager --browser 'chrome' --output json --browser-version 1")
+            .with('selenium-manager', '--browser', 'chrome', '--output', 'json', '--browser-version', 1)
         end
 
         it 'uses browser location if specified' do
@@ -109,7 +110,7 @@ module WebDriver
           described_class.driver_path(options)
 
           expect(described_class).to have_received(:run)
-            .with("selenium-manager --browser 'chrome' --output json --browser-path \"/path/to/browser\"")
+            .with('selenium-manager', '--browser', 'chrome', '--output', 'json', '--browser-path', '/path/to/browser')
         end
 
         it 'properly escapes plain spaces in browser location' do
@@ -121,19 +122,8 @@ module WebDriver
           described_class.driver_path(options)
 
           expect(described_class).to have_received(:run)
-            .with("selenium-manager --browser 'chrome' --output json --browser-path \"/path\\ to/the/browser\"")
-        end
-
-        it 'properly escapes escaped spaces in browser location' do
-          allow(described_class).to receive(:run)
-          allow(described_class).to receive(:binary).and_return('selenium-manager')
-          allow(Platform).to receive(:assert_executable)
-          options = Options.chrome(binary: '/path\ to/the/browser')
-
-          described_class.driver_path(options)
-
-          expect(described_class).to have_received(:run)
-            .with("selenium-manager --browser 'chrome' --output json --browser-path \"/path\\ to/the/browser\"")
+            .with('selenium-manager', '--browser', 'chrome', '--output', 'json',
+                  '--browser-path', '/path to/the/browser')
         end
       end
     end
