[CALCITE-5218] Verify HTTP client class before instantiating it

apache · Jul 26, 2022 · 0c097b6 · 0c097b6
1 parent aad227f
commit 0c097b6
Show file tree

Hide file tree

Showing 2 changed files with 27 additions and 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
@@ -138,14 +138,23 @@ public static AvaticaHttpClientFactoryImpl getInstance() {
   }
 
   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
@@ -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