From 1fc5a998bece38e9aa5d05e9bd1b883168115ba3 Mon Sep 17 00:00:00 2001 From: Mykola Mokhnach Date: Wed, 22 Apr 2020 10:23:21 +0200 Subject: [PATCH] fix: Properly translate desiredCapabilities into a command line argument --- .../service/local/AppiumServiceBuilder.java | 92 +++---------------- .../service/local/ServerBuilderTest.java | 6 ++ 2 files changed, 18 insertions(+), 80 deletions(-) diff --git a/src/main/java/io/appium/java_client/service/local/AppiumServiceBuilder.java b/src/main/java/io/appium/java_client/service/local/AppiumServiceBuilder.java index 10fdefe56..caef4d36a 100644 --- a/src/main/java/io/appium/java_client/service/local/AppiumServiceBuilder.java +++ b/src/main/java/io/appium/java_client/service/local/AppiumServiceBuilder.java @@ -23,8 +23,8 @@ import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; -import io.appium.java_client.remote.AndroidMobileCapabilityType; -import io.appium.java_client.remote.MobileCapabilityType; +import com.google.gson.Gson; +import com.google.gson.GsonBuilder; import io.appium.java_client.service.local.flags.ServerArgument; import org.apache.commons.io.IOUtils; @@ -32,7 +32,6 @@ import org.apache.commons.lang3.SystemUtils; import org.apache.commons.validator.routines.InetAddressValidator; import org.openqa.selenium.Capabilities; -import org.openqa.selenium.Platform; import org.openqa.selenium.os.ExecutableFinder; import org.openqa.selenium.remote.BrowserType; import org.openqa.selenium.remote.DesiredCapabilities; @@ -54,7 +53,6 @@ import java.util.concurrent.TimeUnit; import java.util.function.Function; - public final class AppiumServiceBuilder extends DriverService.Builder { @@ -73,8 +71,6 @@ public final class AppiumServiceBuilder private static final String NODE_PATH = "NODE_BINARY_PATH"; public static final String BROADCAST_IP_ADDRESS = "0.0.0.0"; - private static final List PATH_CAPABILITIES = ImmutableList.of(AndroidMobileCapabilityType.KEYSTORE_PATH, - AndroidMobileCapabilityType.CHROMEDRIVER_EXECUTABLE, MobileCapabilityType.APP); private static final Path APPIUM_PATH_SUFFIX = Paths.get("appium", "build", "lib", "main.js"); public static final int DEFAULT_APPIUM_PORT = 4723; private final Map serverArguments = new HashMap<>(); @@ -300,79 +296,15 @@ private void loadPathToMainScript() { this.appiumJS = findMainScript(); } - private String parseCapabilitiesIfWindows() { - String result = StringUtils.EMPTY; - - if (capabilities != null) { - Map capabilitiesMap = capabilities.asMap(); - Set> entries = capabilitiesMap.entrySet(); - - for (Map.Entry entry : entries) { - Object value = entry.getValue(); - - if (value == null) { - continue; - } - - if (String.class.isAssignableFrom(value.getClass())) { - if (PATH_CAPABILITIES.contains(entry.getKey())) { - value = "\\\"" + String.valueOf(value).replace("\\", "/") + "\\\""; - } else { - value = "\\\"" + value + "\\\""; - } - } else { - value = String.valueOf(value); - } - - String key = "\\\"" + entry.getKey() + "\\\""; - if (StringUtils.isBlank(result)) { - result = key + ": " + value; - } else { - result = result + ", " + key + ": " + value; - } - } - } - - return "{" + result + "}"; - } - - private String parseCapabilitiesIfUNIX() { - String result = StringUtils.EMPTY; - - if (capabilities != null) { - Map capabilitiesMap = capabilities.asMap(); - Set> entries = capabilitiesMap.entrySet(); - - for (Map.Entry entry : entries) { - Object value = entry.getValue(); - - if (value == null) { - continue; - } - - if (String.class.isAssignableFrom(value.getClass())) { - value = "\"" + value + "\""; - } else { - value = String.valueOf(value); - } - - String key = "\"" + entry.getKey() + "\""; - if (StringUtils.isBlank(result)) { - result = key + ": " + value; - } else { - result = result + ", " + key + ": " + value; - } - } - } - - return "{" + result + "}"; - } - - private String parseCapabilities() { - if (Platform.getCurrent().is(Platform.WINDOWS)) { - return parseCapabilitiesIfWindows(); - } - return parseCapabilitiesIfUNIX(); + private String capabilitiesToCmdlineArg() { + Gson gson = new GsonBuilder() + .disableHtmlEscaping() + .serializeNulls() + .create(); + // Selenium internally uses org.apache.commons.exec.CommandLine + // which has the following known bug in its arguments parser: + // https://issues.apache.org/jira/browse/EXEC-54 + return gson.toJson(capabilities.asMap()); } @Override @@ -418,7 +350,7 @@ protected ImmutableList createArgs() { if (capabilities != null) { argList.add("--default-capabilities"); - argList.add(parseCapabilities()); + argList.add(capabilitiesToCmdlineArg()); } return new ImmutableList.Builder().addAll(argList).build(); diff --git a/src/test/java/io/appium/java_client/service/local/ServerBuilderTest.java b/src/test/java/io/appium/java_client/service/local/ServerBuilderTest.java index 03fc62a80..05a9109f9 100644 --- a/src/test/java/io/appium/java_client/service/local/ServerBuilderTest.java +++ b/src/test/java/io/appium/java_client/service/local/ServerBuilderTest.java @@ -28,6 +28,7 @@ import static org.junit.Assert.assertThat; import static org.junit.Assert.assertTrue; +import com.google.common.collect.ImmutableMap; import io.github.bonigarcia.wdm.WebDriverManager; import org.junit.After; import org.junit.BeforeClass; @@ -178,6 +179,11 @@ public void checkAbilityToStartServiceUsingCapabilitiesAndFlags() { caps.setCapability(APP_PACKAGE, "io.appium.android.apis"); caps.setCapability(APP_ACTIVITY, ".view.WebView1"); caps.setCapability(APP, app.getAbsolutePath()); + caps.setCapability("winPath", "C:\\selenium\\app.apk"); + caps.setCapability("unixPath", "/selenium/app.apk"); + caps.setCapability("quotes", "\"'"); + caps.setCapability("goog:chromeOptions", + ImmutableMap.of("env", ImmutableMap.of("test", "value"), "val2", 0)); caps.setCapability(CHROMEDRIVER_EXECUTABLE, chromeManager.getBinaryPath()); service = new AppiumServiceBuilder()