honeymoose
/
OpenSearch
mirror of https://github.com/honeymoose/OpenSearch.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

[7.x] SQL: fix handling of escaped chars in JDBC connection string (#58429) (#58977) 

SQL: fix handling of escaped chars in JDBC connection string (#58429)

This commit fixes an issue emerging when the connection string URI
contains escaped characters.

The original URI is pre-parsed in order to re-assemble a new URI having
the optional elements filled in with defaults. The new URI has been
using however the unescaped query and fragment parts. So if these
contained any escaped `&` or `=` (such as in the password option value),
the unescaping would reveal them and make them later interfere with the
options parsing.

The commit changes that, so that the new URI be built from the unescaped
"raw" parts of the original URI.

(cherry picked from commit 94eb5a05e79c6e203de548d05b13e00295bd4489)
This commit is contained in:
Bogdan Pintea 2020-07-03 17:03:00 +02:00 committed by GitHub
parent e3fc1638d8
commit 3d96d91efb
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
4 changed files with 82 additions and 42 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

6
docs/reference/sql/endpoints/jdbc.asciidoc
View File

@ -51,9 +51,9 @@ Once registered, the driver understands the following syntax as an URL:
["source","text",subs="attributes"]
----
jdbc:es://[[http|https]://]?[host[:port]]?/[prefix]?[\?[option=value]&]*
jdbc:[es|elasticsearch]://[[http|https]://]?[host[:port]]?/[prefix]?[\?[option=value]&]*
----
`jdbc:es://`:: Prefix. Mandatory.
`jdbc:[es|elasticsearch]://`:: Prefix. Mandatory.
`[[http|https]://]`:: Type of HTTP connection to make. Possible values are
`http` (default) or `https`. Optional.
@ -154,7 +154,7 @@ To put all of it together, the following URL:
jdbc:es://http://server:3456/?timezone=UTC&page.size=250
----
Opens up a {es-sql} connection to `server` on port `3456`, setting the JDBC connection timezone to `UTC` and its pagesize to `250` entries.
opens up a {es-sql} connection to `server` on port `3456`, setting the JDBC connection timezone to `UTC` and its pagesize to `250` entries.
=== API usage

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

@ -10,6 +10,7 @@ import org.elasticsearch.xpack.sql.client.ConnectionConfiguration;
import org.elasticsearch.xpack.sql.client.StringUtils;
import java.net.URI;
import java.net.URLDecoder;
import java.sql.DriverPropertyInfo;
import java.time.ZoneId;
import java.util.ArrayList;
@ -37,6 +38,7 @@ import static org.elasticsearch.xpack.sql.client.UriUtils.removeQuery;
//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 +129,8 @@ public class JdbcConfiguration extends ConnectionConfiguration {
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 +147,10 @@ public class JdbcConfiguration extends ConnectionConfiguration {
if (args.size() != 2) {
throw new JdbcSQLException("Invalid parameter [" + param + "], format needs to be key=value");
}
final String key = URLDecoder.decode(args.get(0), "UTF-8").trim();
final String val = URLDecoder.decode(args.get(1), "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 +214,9 @@ public class JdbcConfiguration extends ConnectionConfiguration {
}
public static boolean canAccept(String url) {
return (StringUtils.hasText(url) && url.trim().startsWith(JdbcConfiguration.URL_PREFIX));
String u = url.trim();
return (StringUtils.hasText(u) &&
(u.startsWith(JdbcConfiguration.URL_PREFIX) || u.startsWith(JdbcConfiguration.URL_FULL_PREFIX)));
}
public DriverPropertyInfo[] driverPropertyInfo() {

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

@ -12,6 +12,7 @@ import org.elasticsearch.xpack.sql.client.SuppressForbidden;
import java.net.URI;
import java.net.URISyntaxException;
import java.net.URLEncoder;
import java.security.AccessController;
import java.security.PrivilegedAction;
import java.sql.DriverManager;
@ -27,6 +28,8 @@ import static org.elasticsearch.xpack.sql.client.ConnectionConfiguration.PAGE_SI
import static org.elasticsearch.xpack.sql.client.ConnectionConfiguration.PAGE_TIMEOUT;
import static org.elasticsearch.xpack.sql.client.ConnectionConfiguration.PROPERTIES_VALIDATION;
import static org.elasticsearch.xpack.sql.client.ConnectionConfiguration.QUERY_TIMEOUT;
import static org.elasticsearch.xpack.sql.jdbc.JdbcConfiguration.URL_FULL_PREFIX;
import static org.elasticsearch.xpack.sql.jdbc.JdbcConfiguration.URL_PREFIX;
import static org.hamcrest.Matchers.equalTo;
import static org.hamcrest.Matchers.is;
@ -43,131 +46,145 @@ public class JdbcConfigurationTests extends ESTestCase {
}
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/"));
}
private static String jdbcPrefix() {
return randomFrom(URL_PREFIX, URL_FULL_PREFIX);
}
public void testHostAndPort() throws Exception {
assertThat(ci("jdbc:es://localhost:1234").baseUri().toString(), is("http://localhost:1234/"));
assertThat(ci(jdbcPrefix() + "localhost:1234").baseUri().toString(), is("http://localhost:1234/"));
}
public void testPropertiesEscaping() throws Exception {
String pass = randomUnicodeOfLengthBetween(1, 500);
String encPass = URLEncoder.encode(pass, "UTF-8").replace("+", "%20");
String url = jdbcPrefix() + "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/"));
assertThat(ci(jdbcPrefix() + "localhost:1234/").baseUri().toString(), is("http://localhost:1234/"));
}
public void testMultiPathSuffix() throws Exception {
assertThat(ci("jdbc:es://a:1/foo/bar/tar").baseUri().toString(), is("http://a:1/foo/bar/tar"));
assertThat(ci(jdbcPrefix() + "a:1/foo/bar/tar").baseUri().toString(), is("http://a:1/foo/bar/tar"));
}
public void testV6Localhost() throws Exception {
assertThat(ci("jdbc:es://[::1]:54161/foo/bar").baseUri().toString(), is("http://[::1]:54161/foo/bar"));
assertThat(ci(jdbcPrefix() + "[::1]:54161/foo/bar").baseUri().toString(), is("http://[::1]:54161/foo/bar"));
}
public void testDebug() throws Exception {
JdbcConfiguration ci = ci("jdbc:es://a:1/?debug=true");
JdbcConfiguration ci = ci(jdbcPrefix() + "a:1/?debug=true");
assertThat(ci.baseUri().toString(), is("http://a:1/"));
assertThat(ci.debug(), is(true));
assertThat(ci.debugOut(), is("err"));
}
public void testDebugOut() throws Exception {
JdbcConfiguration ci = ci("jdbc:es://a:1/?debug=true&debug.output=jdbc.out");
JdbcConfiguration ci = ci(jdbcPrefix() + "a:1/?debug=true&debug.output=jdbc.out");
assertThat(ci.baseUri().toString(), is("http://a:1/"));
assertThat(ci.debug(), is(true));
assertThat(ci.debugOut(), is("jdbc.out"));
}
public void testDebugFlushAlways() throws Exception {
JdbcConfiguration ci = ci("jdbc:es://a:1/?debug=true&debug.flushAlways=false");
JdbcConfiguration ci = ci(jdbcPrefix() + "a:1/?debug=true&debug.flushAlways=false");
assertThat(ci.baseUri().toString(), is("http://a:1/"));
assertThat(ci.debug(), is(true));
assertThat(ci.flushAlways(), is(false));
ci = ci("jdbc:es://a:1/?debug=true&debug.flushAlways=true");
ci = ci(jdbcPrefix() + "a:1/?debug=true&debug.flushAlways=true");
assertThat(ci.baseUri().toString(), is("http://a:1/"));
assertThat(ci.debug(), is(true));
assertThat(ci.flushAlways(), is(true));
ci = ci("jdbc:es://a:1/?debug=true");
ci = ci(jdbcPrefix() + "a:1/?debug=true");
assertThat(ci.baseUri().toString(), is("http://a:1/"));
assertThat(ci.debug(), is(true));
assertThat(ci.flushAlways(), is(false));
}
public void testTypeInParam() throws Exception {
Exception e = expectThrows(JdbcSQLException.class, () -> ci("jdbc:es://a:1/foo/bar/tar?debug=true&debug.out=jdbc.out"));
Exception e = expectThrows(JdbcSQLException.class, () -> ci(jdbcPrefix() + "a:1/foo/bar/tar?debug=true&debug.out=jdbc.out"));
assertEquals("Unknown parameter [debug.out]; did you mean [debug.output]", e.getMessage());
}
public void testDebugOutWithSuffix() throws Exception {
JdbcConfiguration ci = ci("jdbc:es://a:1/foo/bar/tar?debug=true&debug.output=jdbc.out");
JdbcConfiguration ci = ci(jdbcPrefix() + "a:1/foo/bar/tar?debug=true&debug.output=jdbc.out");
assertThat(ci.baseUri().toString(), is("http://a:1/foo/bar/tar"));
assertThat(ci.debug(), is(true));
assertThat(ci.debugOut(), is("jdbc.out"));
}
public void testHttpWithSSLEnabledFromProperty() throws Exception {
JdbcConfiguration ci = ci("jdbc:es://test?ssl=true");
JdbcConfiguration ci = ci(jdbcPrefix() + "test?ssl=true");
assertThat(ci.baseUri().toString(), is("https://test:9200/"));
}
public void testHttpWithSSLEnabledFromPropertyAndDisabledFromProtocol() throws Exception {
JdbcConfiguration ci = ci("jdbc:es://http://test?ssl=true");
JdbcConfiguration ci = ci(jdbcPrefix() + "http://test?ssl=true");
assertThat(ci.baseUri().toString(), is("https://test:9200/"));
}
public void testHttpWithSSLEnabledFromProtocol() throws Exception {
JdbcConfiguration ci = ci("jdbc:es://https://test:9200");
JdbcConfiguration ci = ci(jdbcPrefix() + "https://test:9200");
assertThat(ci.baseUri().toString(), is("https://test:9200/"));
}
public void testHttpWithSSLEnabledFromProtocolAndProperty() throws Exception {
JdbcConfiguration ci = ci("jdbc:es://https://test:9200?ssl=true");
JdbcConfiguration ci = ci(jdbcPrefix() + "https://test:9200?ssl=true");
assertThat(ci.baseUri().toString(), is("https://test:9200/"));
}
public void testHttpWithSSLDisabledFromProperty() throws Exception {
JdbcConfiguration ci = ci("jdbc:es://test?ssl=false");
JdbcConfiguration ci = ci(jdbcPrefix() + "test?ssl=false");
assertThat(ci.baseUri().toString(), is("http://test:9200/"));
}
public void testHttpWithSSLDisabledFromPropertyAndProtocol() throws Exception {
JdbcConfiguration ci = ci("jdbc:es://http://test?ssl=false");
JdbcConfiguration ci = ci(jdbcPrefix() + "http://test?ssl=false");
assertThat(ci.baseUri().toString(), is("http://test:9200/"));
}
public void testHttpWithSSLDisabledFromPropertyAndEnabledFromProtocol() throws Exception {
Exception e = expectThrows(JdbcSQLException.class, () -> ci("jdbc:es://https://test?ssl=false"));
Exception e = expectThrows(JdbcSQLException.class, () -> ci(jdbcPrefix() + "https://test?ssl=false"));
assertEquals("Cannot enable SSL: HTTPS protocol being used in the URL and SSL disabled in properties", e.getMessage());
}
public void testValidatePropertiesDefault() {
Exception e = expectThrows(JdbcSQLException.class, () -> ci("jdbc:es://test:9200?pagee.size=12"));
Exception e = expectThrows(JdbcSQLException.class, () -> ci(jdbcPrefix() + "test:9200?pagee.size=12"));
assertEquals("Unknown parameter [pagee.size]; did you mean [page.size]", e.getMessage());
e = expectThrows(JdbcSQLException.class, () -> ci("jdbc:es://test:9200?foo=bar"));
e = expectThrows(JdbcSQLException.class, () -> ci(jdbcPrefix() + "test:9200?foo=bar"));
assertEquals("Unknown parameter [foo]; did you mean [ssl]", e.getMessage());
}
public void testValidateProperties() {
Exception e = expectThrows(JdbcSQLException.class, () -> ci("jdbc:es://test:9200?pagee.size=12&validate.properties=true"));
Exception e = expectThrows(JdbcSQLException.class, () -> ci(jdbcPrefix() + "test:9200?pagee.size=12&validate.properties=true"));
assertEquals("Unknown parameter [pagee.size]; did you mean [page.size]", e.getMessage());
e = expectThrows(JdbcSQLException.class, () -> ci("jdbc:es://test:9200?&validate.properties=true&something=some_value"));
e = expectThrows(JdbcSQLException.class, () -> ci(jdbcPrefix() + "test:9200?&validate.properties=true&something=some_value"));
assertEquals("Unknown parameter [something]; did you mean []", e.getMessage());
Properties properties = new Properties();
properties.setProperty(PROPERTIES_VALIDATION, "true");
e = expectThrows(JdbcSQLException.class, () -> JdbcConfiguration.create("jdbc:es://test:9200?something=some_value", properties, 0));
e = expectThrows(JdbcSQLException.class,
() -> JdbcConfiguration.create(jdbcPrefix() + "test:9200?something=some_value", properties, 0));
assertEquals("Unknown parameter [something]; did you mean []", e.getMessage());
}
public void testNoPropertiesValidation() throws SQLException {
JdbcConfiguration ci = ci("jdbc:es://test:9200?pagee.size=12&validate.properties=false");
JdbcConfiguration ci = ci(jdbcPrefix() + "test:9200?pagee.size=12&validate.properties=false");
assertEquals(false, ci.validateProperties());
// URL properties test
@ -177,8 +194,9 @@ public class JdbcConfigurationTests extends ESTestCase {
long pageTimeout = randomNonNegativeLong();
int pageSize = randomIntBetween(0, Integer.MAX_VALUE);
ci = ci("jdbc:es://test:9200?validate.properties=false&something=some_value&query.timeout=" + queryTimeout + "&connect.timeout="
+ connectTimeout + "&network.timeout=" + networkTimeout + "&page.timeout=" + pageTimeout + "&page.size=" + pageSize);
ci = ci(jdbcPrefix() + "test:9200?validate.properties=false&something=some_value&query.timeout=" + queryTimeout
+ "&connect.timeout=" + connectTimeout + "&network.timeout=" + networkTimeout + "&page.timeout=" + pageTimeout
+ "&page.size=" + pageSize);
assertEquals(false, ci.validateProperties());
assertEquals(queryTimeout, ci.queryTimeout());
assertEquals(connectTimeout, ci.connectTimeout());
@ -196,7 +214,7 @@ public class JdbcConfigurationTests extends ESTestCase {
properties.put(PAGE_SIZE, Integer.toString(pageSize));
// also putting validate.properties in URL to be overriden by the properties value
ci = JdbcConfiguration.create("jdbc:es://test:9200?validate.properties=true&something=some_value", properties, 0);
ci = JdbcConfiguration.create(jdbcPrefix() + "test:9200?validate.properties=true&something=some_value", properties, 0);
assertEquals(false, ci.validateProperties());
assertEquals(queryTimeout, ci.queryTimeout());
assertEquals(connectTimeout, ci.connectTimeout());
@ -210,7 +228,7 @@ public class JdbcConfigurationTests extends ESTestCase {
properties.setProperty(CONNECT_TIMEOUT, "3"); // Should be overridden
properties.setProperty(PAGE_TIMEOUT, "4");
String url = "jdbc:es://test?connect.timeout=1&page.timeout=2";
String url = jdbcPrefix() + "test?connect.timeout=1&page.timeout=2";
// No properties
JdbcConfiguration ci = JdbcConfiguration.create(url, null, 0);
@ -235,7 +253,7 @@ public class JdbcConfigurationTests extends ESTestCase {
allProps.putAll(urlPropMap);
String sslUrlProps = urlPropMap.entrySet().stream().map(e -> e.getKey() + "=" + e.getValue()).collect(Collectors.joining("&"));
assertSslConfig(allProps, ci("jdbc:es://test?" + sslUrlProps.toString()).sslConfig());
assertSslConfig(allProps, ci(jdbcPrefix() + "test?" + sslUrlProps.toString()).sslConfig());
}
public void testSSLPropertiesInUrlAndProperties() throws Exception {
@ -258,7 +276,7 @@ public class JdbcConfigurationTests extends ESTestCase {
Properties allProps = new Properties();
allProps.putAll(urlPropMap);
allProps.putAll(propMap);
assertSslConfig(allProps, JdbcConfiguration.create("jdbc:es://test?" + sslUrlProps.toString(), props, 0).sslConfig());
assertSslConfig(allProps, JdbcConfiguration.create(jdbcPrefix() + "test?" + sslUrlProps.toString(), props, 0).sslConfig());
}
public void testSSLPropertiesOverride() throws Exception {
@ -295,7 +313,7 @@ public class JdbcConfigurationTests extends ESTestCase {
});
try {
DriverManager.getDriver("jdbc:es://test?" + sslUrlProps);
DriverManager.getDriver(jdbcPrefix() + "test?" + sslUrlProps);
} catch (SQLException sqle) {
fail("Driver registration should have been successful. Error: " + sqle);
}
@ -347,12 +365,12 @@ public class JdbcConfigurationTests extends ESTestCase {
}
private void assertJdbcSqlExceptionFromUrl(String wrongSetting, String correctSetting) {
String url = "jdbc:es://test?" + wrongSetting + "=foo";
String url = jdbcPrefix() + "test?" + wrongSetting + "=foo";
assertJdbcSqlException(wrongSetting, correctSetting, url, null);
}
private void assertJdbcSqlExceptionFromProperties(String wrongSetting, String correctSetting) {
String url = "jdbc:es://test";
String url = jdbcPrefix() + "test";
Properties props = new Properties();
props.put(wrongSetting, correctSetting);
assertJdbcSqlException(wrongSetting, correctSetting, url, props);

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

@ -18,12 +18,26 @@ public final class 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);
}
}