fix: Possible NPE in initBiDi() (#2325)

Author: panchenko

src/main/java/io/appium/java_client/remote/AppiumCommandExecutor.java

@@ -20,7 +20,7 @@
 import io.appium.java_client.AppiumClientConfig;
 import io.appium.java_client.internal.ReflectionHelpers;
 import lombok.Getter;
-import org.jspecify.annotations.NonNull;
+import org.jspecify.annotations.NullMarked;
 import org.jspecify.annotations.Nullable;
 import org.openqa.selenium.SessionNotCreatedException;
 import org.openqa.selenium.WebDriverException;
@@ -53,45 +53,43 @@
 import static java.util.Optional.ofNullable;
 import static org.openqa.selenium.remote.DriverCommand.NEW_SESSION;
 
+@NullMarked
 public class AppiumCommandExecutor extends HttpCommandExecutor {
 
     private final Optional<DriverService> serviceOptional;
     @Getter
-    private final HttpClient.Factory httpClientFactory;
-    @Getter
     private final AppiumClientConfig appiumClientConfig;
 
     /**
      * Create an AppiumCommandExecutor instance.
      *
      * @param additionalCommands is the map of Appium commands
      * @param service take a look at {@link DriverService}
-     * @param httpClientFactory take a look at {@link HttpClient.Factory}
+     * @param httpClientFactory take a look at {@link Factory}
      * @param appiumClientConfig take a look at {@link AppiumClientConfig}
      */
     public AppiumCommandExecutor(
-            @NonNull Map<String, CommandInfo> additionalCommands,
+            Map<String, CommandInfo> additionalCommands,
             @Nullable DriverService service,
             @Nullable Factory httpClientFactory,
-            @NonNull AppiumClientConfig appiumClientConfig) {
+            AppiumClientConfig appiumClientConfig) {
         super(additionalCommands,
                 appiumClientConfig,
-                ofNullable(httpClientFactory).orElseGet(AppiumCommandExecutor::getDefaultClientFactory)
+                ofNullable(httpClientFactory).orElseGet(HttpCommandExecutor::getDefaultClientFactory)
         );
         serviceOptional = ofNullable(service);
 
-        this.httpClientFactory = httpClientFactory;
         this.appiumClientConfig = appiumClientConfig;
     }
 
     public AppiumCommandExecutor(Map<String, CommandInfo> additionalCommands, DriverService service,
-                                 HttpClient.Factory httpClientFactory) {
+                                 @Nullable Factory httpClientFactory) {
         this(additionalCommands, requireNonNull(service), httpClientFactory,
                 AppiumClientConfig.defaultConfig().baseUrl(requireNonNull(service).getUrl()));
     }
 
     public AppiumCommandExecutor(Map<String, CommandInfo> additionalCommands, URL addressOfRemoteServer,
-                                 HttpClient.Factory httpClientFactory) {
+                                 @Nullable Factory httpClientFactory) {
         this(additionalCommands, null, httpClientFactory,
                 AppiumClientConfig.defaultConfig().baseUrl(requireNonNull(addressOfRemoteServer)));
     }
@@ -140,6 +138,11 @@ public Map<String, CommandInfo> getAdditionalCommands() {
         return getPrivateFieldValue(HttpCommandExecutor.class, "additionalCommands", Map.class);
     }
 
+    public Factory getHttpClientFactory() {
+        return getPrivateFieldValue(HttpCommandExecutor.class, "httpClientFactory", Factory.class);
+    }
+
+    @Nullable
     protected CommandCodec<HttpRequest> getCommandCodec() {
         return this.commandCodec;
     }
@@ -163,12 +166,8 @@ protected HttpClient getClient() {
      * @param serverUrl A url to override.
      */
     protected void overrideServerUrl(URL serverUrl) {
-        if (this.appiumClientConfig == null) {
-            return;
-        }
-        setPrivateFieldValue(HttpCommandExecutor.class, "client",
-                ofNullable(this.httpClientFactory).orElseGet(AppiumCommandExecutor::getDefaultClientFactory)
-                                .createClient(this.appiumClientConfig.baseUrl(serverUrl)));
+        HttpClient newClient = getHttpClientFactory().createClient(appiumClientConfig.baseUrl(serverUrl));
+        setPrivateFieldValue(HttpCommandExecutor.class, "client", newClient);
     }
 
     private Response createSession(Command command) throws IOException {
@@ -186,7 +185,7 @@ private Response createSession(Command command) throws IOException {
         refreshAdditionalCommands();
         setResponseCodec(dialect.getResponseCodec());
         Response response = result.createResponse();
-        if (this.appiumClientConfig != null && this.appiumClientConfig.isDirectConnectEnabled()) {
+        if (appiumClientConfig.isDirectConnectEnabled()) {
             setDirectConnect(response);
         }
 

src/test/java/io/appium/java_client/remote/AppiumCommandExecutorTest.java

@@ -0,0 +1,41 @@
+package io.appium.java_client.remote;
+
+import io.appium.java_client.AppiumClientConfig;
+import io.appium.java_client.MobileCommand;
+import org.junit.jupiter.api.Test;
+
+import java.net.MalformedURLException;
+import java.net.URL;
+
+import static org.junit.jupiter.api.Assertions.assertDoesNotThrow;
+import static org.junit.jupiter.api.Assertions.assertNotNull;
+
+class AppiumCommandExecutorTest {
+    private static final String APPIUM_URL = "https://appium.example.com";
+
+    private AppiumCommandExecutor createExecutor() {
+        URL baseUrl;
+        try {
+            baseUrl = new URL(APPIUM_URL);
+        } catch (MalformedURLException e) {
+            throw new AssertionError(e);
+        }
+        AppiumClientConfig clientConfig = AppiumClientConfig.defaultConfig().baseUrl(baseUrl);
+        return new AppiumCommandExecutor(MobileCommand.commandRepository, clientConfig);
+    }
+
+    @Test
+    void getAdditionalCommands() {
+        assertNotNull(createExecutor().getAdditionalCommands());
+    }
+
+    @Test
+    void getHttpClientFactory() {
+        assertNotNull(createExecutor().getHttpClientFactory());
+    }
+
+    @Test
+    void overrideServerUrl() {
+        assertDoesNotThrow(() -> createExecutor().overrideServerUrl(new URL("https://direct.example.com")));
+    }
+}