SQL: JDBC: fix access to the Manifest for non-entry JAR URLs (#56797)

* JDBC: fix access to the Manifest for non-entry JAR

The JDBC driver will attempt to read its version from the Manifest file
embedded into its JAR. The URL pointing to the JAR can be provided in a
few ways.

So far, accessing the Manfiest was attempted by getting a URLConnection
out of the URL and then getting an input stream out of this connection.
For file JAR URLs, this only works however if the URL points to the
driver as a JAR file entry (i.e. <sub-url>!/jdbc-driver.jar!/). If
that's not the case, the JarURLConnection will throw an IOException.

This commit fixes that: in case the URL points to a JAR entry
(jar:file:<path>/jdbc-driver.jar!/), the manifest is read directly with
JarURLConnection#getManifest().

x-pack/plugin/sql/sql-client/src/main/java/org/elasticsearch/xpack/sql/client/ClientVersion.java

@@ -8,6 +8,7 @@ package org.elasticsearch.xpack.sql.client;
 import org.elasticsearch.xpack.sql.proto.SqlVersion;
 
 import java.io.IOException;
+import java.net.JarURLConnection;
 import java.net.URL;
 import java.net.URLConnection;
 import java.util.Collections;
@@ -71,23 +72,40 @@ public class ClientVersion {
         CURRENT = extractVersion(url);
     }
 
-    static SqlVersion extractVersion(URL url) {
+    // There are three main types of provided URLs:
+    // (1) a file URL: file:<path><FS separator><driver name>.jar
+    // (2) jar file URL pointing to a JAR file: jar:<sub-url><separator><driver name>.jar!/
+    // (3) jar file URL pointing to a JAR file entry (likely a fat JAR, but other types are possible): jar:<sub-url>!/driver name>.jar!/
+    static Manifest getManifest(URL url) throws IOException {
         String urlStr = url.toString();
         if (urlStr.endsWith(".jar") || urlStr.endsWith(".jar!/")) {
-            try {
-                URLConnection conn = url.openConnection();
-                conn.setUseCaches(false);
-
-                try (JarInputStream jar = new JarInputStream(conn.getInputStream())) {
-                    Manifest manifest = jar.getManifest();
-                    String version = manifest.getMainAttributes().getValue("X-Compile-Elasticsearch-Version");
-                    return SqlVersion.fromString(version);
+            URLConnection conn = url.openConnection();
+            // avoid file locking
+            conn.setUseCaches(false);
+            // For a jar protocol, the implementing java.base/sun.net.www.protocol.jar.JarUrlConnection#getInputStream() will only
+            // return a stream (vs. throw an IOException) if the JAR file URL points to a JAR file entry and not a JAR file.
+            if (url.getProtocol().equals("jar")) {
+                JarURLConnection jarConn = (JarURLConnection) conn;
+                if (jarConn.getEntryName() == null) { // the URL points to a JAR file
+                    return jarConn.getManifest(); // in case of a fat JAR, this would return the outermost JAR's manifest
                 }
-            } catch (Exception ex) {
-                throw new IllegalArgumentException("Detected Elasticsearch JDBC jar but cannot retrieve its version", ex);
             }
+            try (JarInputStream jar = new JarInputStream(conn.getInputStream())) {
+                return jar.getManifest();
+            }
+        }
+        return null;
+    }
+
+    static SqlVersion extractVersion(URL url) {
+        Manifest manifest = null;
+        try {
+            manifest = getManifest(url);
+        } catch (IOException ex) {
+            throw new IllegalArgumentException("Detected an Elasticsearch JDBC jar but cannot retrieve its version", ex);
         }
-        return new SqlVersion(0, 0, 0);
+        String version = manifest != null ? manifest.getMainAttributes().getValue("X-Compile-Elasticsearch-Version") : null;
+        return version != null ? SqlVersion.fromString(version) : new SqlVersion(0, 0, 0);
     }
 
     // This function helps ensure that a client won't attempt to communicate to a server with less features than its own. Since this check

x-pack/plugin/sql/sql-client/src/test/java/org/elasticsearch/xpack/sql/client/VersionTests.java

@@ -31,27 +31,80 @@ public class VersionTests extends ESTestCase {
         assertEquals("Invalid version format [7.1]", err.getMessage());
     }
 
-    public void testVersionFromJarInJar() throws IOException {
+
+    private static final String JAR_PATH_SEPARATOR = "!/";
+
+    private static String versionString(byte[] parts) {
+        StringBuffer version = new StringBuffer();
+        for (byte part : parts) {
+            version.append(".");
+            version.append(part);
+        }
+        return version.substring(1);
+    }
+
+    private static byte[] randomVersion() {
+        byte[] parts = new byte[3];
+        for (int i = 0; i < parts.length; i ++) {
+            parts[i] = (byte) randomIntBetween(0, SqlVersion.REVISION_MULTIPLIER);
+        }
+        return parts;
+    }
+
+    private static Path createDriverJar(byte[] parts) throws IOException {
         final String JDBC_JAR_NAME = "es-sql-jdbc.jar";
-        final String JAR_PATH_SEPARATOR = "!/";
 
         Path dir = createTempDir();
-        Path jarPath = dir.resolve("uberjar.jar");          // simulated uberjar containing the jdbc driver
-        Path innerJarPath = dir.resolve(JDBC_JAR_NAME); // simulated ES JDBC driver file
+        Path jarPath = dir.resolve(JDBC_JAR_NAME); // simulated ES JDBC driver file
 
         Manifest jdbcJarManifest = new Manifest();
         Attributes attributes = jdbcJarManifest.getMainAttributes();
         attributes.put(Attributes.Name.MANIFEST_VERSION, "1.0.0");
         attributes.put(new Attributes.Name("Change"), "abc");
-        attributes.put(new Attributes.Name("X-Compile-Elasticsearch-Version"), "1.2.3");
+        attributes.put(new Attributes.Name("X-Compile-Elasticsearch-Version"), versionString(parts));
 
         // create the jdbc driver file
-        try (JarOutputStream jdbc = new JarOutputStream(Files.newOutputStream(innerJarPath, StandardOpenOption.CREATE), jdbcJarManifest)) {}
+        try (JarOutputStream __ = new JarOutputStream(Files.newOutputStream(jarPath, StandardOpenOption.CREATE), jdbcJarManifest)) {}
+
+        return jarPath;
+    }
+
+    public void testVersionFromFileJar() throws IOException {
+        byte[] parts = randomVersion();
+        Path jarPath = createDriverJar(parts);
+
+        URL fileUrl = new URL(jarPath.toUri().toURL().toString());
+        SqlVersion version = ClientVersion.extractVersion(fileUrl);
+
+        assertEquals(parts[0], version.major);
+        assertEquals(parts[1], version.minor);
+        assertEquals(parts[2], version.revision);
+        assertEquals(versionString(parts), version.version);
+    }
+
+    public void testVersionFromJar() throws IOException {
+        byte[] parts = randomVersion();
+        Path jarPath = createDriverJar(parts);
+
+        URL jarUrl = new URL("jar:" + jarPath.toUri().toURL().toString() + JAR_PATH_SEPARATOR);
+        SqlVersion version = ClientVersion.extractVersion(jarUrl);
+
+        assertEquals(parts[0], version.major);
+        assertEquals(parts[1], version.minor);
+        assertEquals(parts[2], version.revision);
+        assertEquals(versionString(parts), version.version);
+    }
+
+    public void testVersionFromJarInJar() throws IOException {
+        byte[] parts = randomVersion();
+        Path dir = createTempDir();
+        Path jarPath = dir.resolve("uberjar.jar");          // simulated uberjar containing the jdbc driver
+        Path innerJarPath = createDriverJar(parts);
 
         // create the uberjar and embed the jdbc driver one into it
         try (BufferedInputStream in = new BufferedInputStream(Files.newInputStream(innerJarPath));
                 JarOutputStream out = new JarOutputStream(Files.newOutputStream(jarPath, StandardOpenOption.CREATE), new Manifest())) {
-            JarEntry entry = new JarEntry(JDBC_JAR_NAME + JAR_PATH_SEPARATOR);
+            JarEntry entry = new JarEntry(innerJarPath.getFileName() + JAR_PATH_SEPARATOR);
             out.putNextEntry(entry);
 
             byte[] buffer = new byte[1024];
@@ -63,13 +116,14 @@ public class VersionTests extends ESTestCase {
                 out.write(buffer, 0, count);
             }
         }
-        
-        URL jarInJar = new URL("jar:" + jarPath.toUri().toURL().toString() + JAR_PATH_SEPARATOR + JDBC_JAR_NAME + JAR_PATH_SEPARATOR);
+
+        URL jarInJar = new URL("jar:" + jarPath.toUri().toURL().toString() + JAR_PATH_SEPARATOR + innerJarPath.getFileName() +
+            JAR_PATH_SEPARATOR);
 
         SqlVersion version = ClientVersion.extractVersion(jarInJar);
-        assertEquals(1, version.major);
-        assertEquals(2, version.minor);
-        assertEquals(3, version.revision);
-        assertEquals("1.2.3", version.version);
+        assertEquals(parts[0], version.major);
+        assertEquals(parts[1], version.minor);
+        assertEquals(parts[2], version.revision);
+        assertEquals(versionString(parts), version.version);
     }
 }