SQL: fix handling of escaped chars in JDBC connection string

x-pack/plugin/sql/jdbc/src/main/java/org/elasticsearch/xpack/sql/jdbc/JdbcConfiguration.java

@@ -10,6 +10,8 @@
 import org.elasticsearch.xpack.sql.client.StringUtils;
 
 import java.net.URI;
+import java.net.URLDecoder;
+import java.nio.charset.StandardCharsets;
 import java.sql.DriverPropertyInfo;
 import java.time.ZoneId;
 import java.util.ArrayList;
@@ -37,6 +39,7 @@
 //TODO: beef this up for Security/SSL
 public class JdbcConfiguration extends ConnectionConfiguration {
     static final String URL_PREFIX = "jdbc:es://";
+    static final String URL_FULL_PREFIX = "jdbc:elasticsearch://";
     public static URI DEFAULT_URI = URI.create("http://localhost:9200/");
 
 
@@ -127,6 +130,8 @@ private static URI parseUrl(String u) throws JdbcSQLException {
     private static String removeJdbcPrefix(String connectionString) throws JdbcSQLException {
         if (connectionString.startsWith(URL_PREFIX)) {
             return connectionString.substring(URL_PREFIX.length());
+        } else if (connectionString.startsWith(URL_FULL_PREFIX)) {
+            return connectionString.substring(URL_FULL_PREFIX.length());
         } else {
             throw new JdbcSQLException("Expected [" + URL_PREFIX + "] url, received [" + connectionString + "]");
         }
@@ -143,8 +148,10 @@ private static Properties parseProperties(URI uri, String u) throws JdbcSQLExcep
                     if (args.size() != 2) {
                         throw new JdbcSQLException("Invalid parameter [" + param + "], format needs to be key=value");
                     }
+                    final String key = URLDecoder.decode(args.get(0), StandardCharsets.UTF_8).trim();
+                    final String val = URLDecoder.decode(args.get(1), StandardCharsets.UTF_8);
                     // further validation happens in the constructor (since extra properties might be specified either way)
-                    props.setProperty(args.get(0).trim(), args.get(1).trim());
+                    props.setProperty(key, val);
                 }
             }
         } catch (JdbcSQLException e) {
@@ -208,7 +215,8 @@ public boolean indexIncludeFrozen() {
     }
 
     public static boolean canAccept(String url) {
-        return (StringUtils.hasText(url) && url.trim().startsWith(JdbcConfiguration.URL_PREFIX));
+        return (StringUtils.hasText(url) &&
+            (url.trim().startsWith(JdbcConfiguration.URL_PREFIX) || url.trim().startsWith(JdbcConfiguration.URL_FULL_PREFIX)));
     }
 
     public DriverPropertyInfo[] driverPropertyInfo() {

x-pack/plugin/sql/jdbc/src/test/java/org/elasticsearch/xpack/sql/jdbc/JdbcConfigurationTests.java

@@ -12,6 +12,8 @@
 
 import java.net.URI;
 import java.net.URISyntaxException;
+import java.net.URLEncoder;
+import java.nio.charset.StandardCharsets;
 import java.security.AccessController;
 import java.security.PrivilegedAction;
 import java.sql.DriverManager;
@@ -43,18 +45,27 @@ public void testInvalidUrl() {
     }
 
     public void testJustThePrefix() throws Exception {
-       Exception e = expectThrows(JdbcSQLException.class, () -> ci("jdbc:es:"));
-       assertEquals("Expected [jdbc:es://] url, received [jdbc:es:]", e.getMessage());
+        Exception e = expectThrows(JdbcSQLException.class, () -> ci("jdbc:es:"));
+        assertEquals("Expected [jdbc:es://] url, received [jdbc:es:]", e.getMessage());
     }
 
     public void testJustTheHost() throws Exception {
         assertThat(ci("jdbc:es://localhost").baseUri().toString(), is("http://localhost:9200/"));
+        assertThat(ci("jdbc:elasticsearch://localhost").baseUri().toString(), is("http://localhost:9200/"));
     }
 
     public void testHostAndPort() throws Exception {
         assertThat(ci("jdbc:es://localhost:1234").baseUri().toString(), is("http://localhost:1234/"));
     }
 
+    public void testPropertiesEscaping() throws Exception {
+        String pass = randomUnicodeOfLengthBetween(1, 500);
+        String encPass = URLEncoder.encode(pass, StandardCharsets.UTF_8).replace("+", "%20");
+        String url = "jdbc:es://test?password=" + encPass;
+        JdbcConfiguration ci = ci(url);
+        assertEquals(pass, ci.authPass());
+    }
+
     public void testTrailingSlashForHost() throws Exception {
         assertThat(ci("jdbc:es://localhost:1234/").baseUri().toString(), is("http://localhost:1234/"));
     }

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

@@ -18,12 +18,26 @@ private UriUtils() {
      */
     public static URI parseURI(String connectionString, URI defaultURI) {
         final URI uri = parseWithNoScheme(connectionString);
+        // Repack the connection string with provided default elements - where missing from the original string - and reparse into a URI.
         final String path = "".equals(uri.getPath()) ? defaultURI.getPath() : uri.getPath();
-        final String query = uri.getQuery() == null ? defaultURI.getQuery() : uri.getQuery();
+        final String rawQuery = uri.getQuery() == null ? defaultURI.getRawQuery() : uri.getRawQuery();
+        final String rawFragment = uri.getFragment() == null ? defaultURI.getRawFragment() : uri.getRawFragment();
         final int port = uri.getPort() < 0 ? defaultURI.getPort() : uri.getPort();
         try {
-            return new URI(uri.getScheme(), uri.getUserInfo(), uri.getHost(), port, path, query, defaultURI.getFragment());
+            // The query part is attached in original "raw" format, to preserve the escaping of characters. This is needed since any
+            // escaped query structure characters (`&` and `=`) wouldn't remain escaped when passed back through the URI constructor
+            // (since they are legal in the query part), and that would later interfere with correctly parsing the attributes.
+            // And same with escaped `#` chars in the fragment part.
+            String connStr = new URI(uri.getScheme(), uri.getUserInfo(), uri.getHost(), port, path, null, null).toString();
+            if (StringUtils.hasLength(rawQuery)) {
+                connStr += "?" + rawQuery;
+            }
+            if (StringUtils.hasLength(rawFragment)) {
+                connStr += "#" + rawFragment;
+            }
+            return new URI(connStr);
         } catch (URISyntaxException e) {
+            // should only happen if the defaultURI is malformed
             throw new IllegalArgumentException("Invalid connection configuration [" + connectionString + "]: " + e.getMessage(), e);
         }
     }