[calcite-avatica] branch main updated: [CALCITE-5218] Verify HTTP client class before instantiating it

[rubenql]

This is an automated email from the ASF dual-hosted git repository.

rubenql pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/calcite-avatica.git


The following commit(s) were added to refs/heads/main by this push:
     new 0c097b6a6 [CALCITE-5218] Verify HTTP client class before instantiating 
it
0c097b6a6 is described below

commit 0c097b6a685fc1f97f151505a219976f15ed0c4c
Author: rubenada <rube...@gmail.com>
AuthorDate: Tue Jul 26 13:21:48 2022 +0100

    [CALCITE-5218] Verify HTTP client class before instantiating it
---
 .../avatica/remote/AvaticaHttpClientFactoryImpl.java  | 19 ++++++++++++++-----
 .../avatica/remote/AvaticaHttpClientFactoryTest.java  | 14 +++++++++++++-
 2 files changed, 27 insertions(+), 6 deletions(-)

diff --git 
a/core/src/main/java/org/apache/calcite/avatica/remote/AvaticaHttpClientFactoryImpl.java
 
b/core/src/main/java/org/apache/calcite/avatica/remote/AvaticaHttpClientFactoryImpl.java
index 420da0e63..e0f3446ae 100644
--- 
a/core/src/main/java/org/apache/calcite/avatica/remote/AvaticaHttpClientFactoryImpl.java
+++ 
b/core/src/main/java/org/apache/calcite/avatica/remote/AvaticaHttpClientFactoryImpl.java
@@ -138,14 +138,23 @@ public class AvaticaHttpClientFactoryImpl implements 
AvaticaHttpClientFactory {
   }
 
   private AvaticaHttpClient instantiateClient(String className, URL url) {
+    AvaticaHttpClient client = null;
+    Exception clientCreationException = null;
     try {
-      Class<?> clz = Class.forName(className);
-      Constructor<?> constructor = clz.getConstructor(URL.class);
-      Object instance = constructor.newInstance(Objects.requireNonNull(url));
-      return AvaticaHttpClient.class.cast(instance);
+      // Ensure that the given class is actually a subclass of 
AvaticaHttpClient
+      Class<? extends AvaticaHttpClient> clz =
+          Class.forName(className).asSubclass(AvaticaHttpClient.class);
+      Constructor<? extends AvaticaHttpClient> constructor = 
clz.getConstructor(URL.class);
+      client = constructor.newInstance(Objects.requireNonNull(url));
     } catch (Exception e) {
+      clientCreationException = e;
+    }
+
+    if (client == null) {
       throw new RuntimeException("Failed to construct AvaticaHttpClient 
implementation "
-          + className, e);
+          + className, clientCreationException);
+    } else {
+      return client;
     }
   }
 
diff --git 
a/core/src/test/java/org/apache/calcite/avatica/remote/AvaticaHttpClientFactoryTest.java
 
b/core/src/test/java/org/apache/calcite/avatica/remote/AvaticaHttpClientFactoryTest.java
index 8e1397c14..27c9bcbd6 100644
--- 
a/core/src/test/java/org/apache/calcite/avatica/remote/AvaticaHttpClientFactoryTest.java
+++ 
b/core/src/test/java/org/apache/calcite/avatica/remote/AvaticaHttpClientFactoryTest.java
@@ -43,7 +43,7 @@ public class AvaticaHttpClientFactoryTest {
         client instanceof AvaticaCommonsHttpClientImpl);
   }
 
-  @Test public void testOverridenHttpClient() throws Exception {
+  @Test public void testOverriddenHttpClient() throws Exception {
     Properties props = new Properties();
     props.setProperty(BuiltInConnectionProperty.HTTP_CLIENT_IMPL.name(),
         AvaticaHttpClientImpl.class.getName());
@@ -55,6 +55,18 @@ public class AvaticaHttpClientFactoryTest {
     assertTrue("Client was an instance of " + client.getClass(),
         client instanceof AvaticaHttpClientImpl);
   }
+
+  @Test(expected = RuntimeException.class) public void testInvalidHttpClient() 
throws Exception {
+    Properties props = new Properties();
+    props.setProperty(BuiltInConnectionProperty.HTTP_CLIENT_IMPL.name(),
+        Properties.class.getName()); // Properties is intentionally *not* a 
valid class
+    URL url = new URL("http://localhost:8765";);
+    ConnectionConfig config = new ConnectionConfigImpl(props);
+    AvaticaHttpClientFactory httpClientFactory = new 
AvaticaHttpClientFactoryImpl();
+
+    // This should throw since the Properties class is invalid
+    httpClientFactory.getClient(url, config, null);
+  }
 }
 
 // End AvaticaHttpClientFactoryTest.java