This is an automated email from the ASF dual-hosted git repository. dsmiley pushed a commit to branch branch_9x in repository https://gitbox.apache.org/repos/asf/solr.git
commit 7e935fa1926bd67d3573c258db55e0390ccb1435 Author: Jan Høydahl <[email protected]> AuthorDate: Thu Jul 30 14:15:02 2026 +0200 Avoid forwarding unrelated request parameters to Calcite from the /sql handler (#4607) (cherry picked from commit 34eb9e3c998b6835969f12d75379607aea9220a2) --- changelog/unreleased/calcite-sql-params.yml | 7 +++ .../org/apache/solr/handler/sql/SQLHandler.java | 45 +++++++++++---- .../apache/solr/handler/sql/TestSQLHandler.java | 67 ++++++++++++++++++++++ 3 files changed, 109 insertions(+), 10 deletions(-) diff --git a/changelog/unreleased/calcite-sql-params.yml b/changelog/unreleased/calcite-sql-params.yml new file mode 100644 index 00000000000..a178dbe6488 --- /dev/null +++ b/changelog/unreleased/calcite-sql-params.yml @@ -0,0 +1,7 @@ +title: The /sql handler request params forwarded to Calcite is now configurable +type: changed +authors: + - name: Jan Høydahl +links: + - name: PR#4607 + url: https://github.com/apache/solr/pull/4607 diff --git a/solr/modules/sql/src/java/org/apache/solr/handler/sql/SQLHandler.java b/solr/modules/sql/src/java/org/apache/solr/handler/sql/SQLHandler.java index 1f3cfe6b05d..1aa7ace83ef 100644 --- a/solr/modules/sql/src/java/org/apache/solr/handler/sql/SQLHandler.java +++ b/solr/modules/sql/src/java/org/apache/solr/handler/sql/SQLHandler.java @@ -22,10 +22,10 @@ import java.sql.ResultSetMetaData; import java.sql.SQLException; import java.util.ArrayList; import java.util.HashMap; -import java.util.Iterator; import java.util.List; import java.util.Map; import java.util.Properties; +import java.util.Set; import org.apache.calcite.config.Lex; import org.apache.solr.client.solrj.io.Tuple; import org.apache.solr.client.solrj.io.comp.StreamComparator; @@ -34,6 +34,7 @@ import org.apache.solr.client.solrj.io.stream.TupleStream; import org.apache.solr.common.params.CommonParams; import org.apache.solr.common.params.ModifiableSolrParams; import org.apache.solr.common.params.SolrParams; +import org.apache.solr.common.util.EnvUtils; import org.apache.solr.core.CoreContainer; import org.apache.solr.core.SolrCore; import org.apache.solr.handler.RequestHandlerBase; @@ -52,10 +53,27 @@ public class SQLHandler extends RequestHandlerBase private static final Logger log = LoggerFactory.getLogger(MethodHandles.lookup().lookupClass()); private static String defaultZkhost = null; - private static String defaultWorkerCollection = null; static final String sqlNonCloudErrorMsg = "/sql handler only works in Solr Cloud mode"; + /** System property to override the set of request parameters forwarded to Calcite. */ + static final String ALLOWED_CONNECTION_PARAMS_PROP = "solr.sql.connection.params.allowed"; + + /** Calcite configuration parameters forwarded as connection properties. */ + static final Set<String> DEFAULT_CONNECTION_PARAMS = + Set.of( + "caseSensitive", + "quoting", + "quotedCasing", + "unquotedCasing", + "conformance", + "fun", + "typeCoercion", + "lenientOperatorLookup", + "defaultNullCollation", + "timeZone", + "locale"); + private boolean isCloud = false; @Override @@ -64,7 +82,6 @@ public class SQLHandler extends RequestHandlerBase if (coreContainer.isZooKeeperAware()) { defaultZkhost = coreContainer.getZkController().getZkServerAddress(); - defaultWorkerCollection = core.getCoreDescriptor().getCollectionName(); isCloud = true; } @@ -85,8 +102,6 @@ public class SQLHandler extends RequestHandlerBase String sql = params.get("stmt"); // Set defaults for parameters params.set("numWorkers", params.getInt("numWorkers", 1)); - params.set("workerCollection", params.get("workerCollection", defaultWorkerCollection)); - params.set("workerZkhost", params.get("workerZkhost", defaultZkhost)); params.set("aggregationMode", params.get("aggregationMode", "facet")); TupleStream tupleStream = null; @@ -103,13 +118,18 @@ public class SQLHandler extends RequestHandlerBase String url = CalciteSolrDriver.CONNECT_STRING_PREFIX; Properties properties = new Properties(); - // Add all query parameters - Iterator<String> parameterNamesIterator = params.getParameterNamesIterator(); - while (parameterNamesIterator.hasNext()) { - String param = parameterNamesIterator.next(); - properties.setProperty(param, params.get(param)); + // Forward only the configured Calcite configuration parameters + for (String param : allowedConnectionParams()) { + String value = params.get(param); + if (value != null) { + properties.setProperty(param, value); + } } + // Solr-specific parameters + properties.setProperty("aggregationMode", params.get("aggregationMode")); + properties.setProperty("numWorkers", params.get("numWorkers")); + // Set these last to ensure that they are set properly properties.setProperty("lex", Lex.MYSQL.toString()); properties.setProperty("zk", defaultZkhost); @@ -203,6 +223,11 @@ public class SQLHandler extends RequestHandlerBase } } + static Set<String> allowedConnectionParams() { + List<String> override = EnvUtils.getPropertyAsList(ALLOWED_CONNECTION_PARAMS_PROP); + return override != null ? Set.copyOf(override) : DEFAULT_CONNECTION_PARAMS; + } + private ModifiableSolrParams adjustParams(SolrParams params) { ModifiableSolrParams adjustedParams = new ModifiableSolrParams(); adjustedParams.add(params); diff --git a/solr/modules/sql/src/test/org/apache/solr/handler/sql/TestSQLHandler.java b/solr/modules/sql/src/test/org/apache/solr/handler/sql/TestSQLHandler.java index 1283ca7e372..aa3886f0949 100644 --- a/solr/modules/sql/src/test/org/apache/solr/handler/sql/TestSQLHandler.java +++ b/solr/modules/sql/src/test/org/apache/solr/handler/sql/TestSQLHandler.java @@ -453,6 +453,73 @@ public class TestSQLHandler extends SolrCloudTestCase { tuples = getTuples(sParams, baseUrl); assertEquals(0, tuples.size()); + + // Verify that request parameters not on the allowlist are not forwarded to Calcite. + // 'schemaType' is a real Calcite connection property which would crash the request + sParams = + params( + CommonParams.QT, + "/sql", + "schemaType", + "someValue", + "stmt", + "select id, field_i, str_s from collection1 where text_t='XXXX' order by field_i desc limit 1"); + + tuples = getTuples(sParams, baseUrl); + + assertEquals(1, tuples.size()); + tuple = tuples.get(0); + assertEquals(8, tuple.getLong("id").longValue()); + assertEquals(60, tuple.getLong("field_i").longValue()); + assertEquals("c", tuple.get("str_s")); + + // Sanity check: with 'schemaType' added to the allowlist it IS forwarded, and the same + // request now fails on the invalid value, proving the allowlist controls forwarding + System.setProperty(SQLHandler.ALLOWED_CONNECTION_PARAMS_PROP, "schemaType"); + try { + final SolrParams forwardedParams = sParams; + IOException e = expectThrows(IOException.class, () -> getTuples(forwardedParams, baseUrl)); + assertTrue(e.getMessage().contains("not valid")); + } finally { + System.clearProperty(SQLHandler.ALLOWED_CONNECTION_PARAMS_PROP); + } + } + + @Test + public void testConnectionParamsAllowlistOverride() throws Exception { + + new UpdateRequest() + .add("id", "1", "text_t", "XXXX XXXX", "str_s", "a", "field_i", "7") + .commit(cluster.getSolrClient(), COLLECTIONORALIAS); + + String baseUrl = + cluster.getJettySolrRunners().get(0).getBaseUrl().toString() + "/" + COLLECTIONORALIAS; + + SolrParams sParams = + params( + CommonParams.QT, + "/sql", + "caseSensitive", + "true", + "stmt", + "select id, FIELD_I from collection1 limit 1"); + + // caseSensitive is forwarded to Calcite by default, making column names case-sensitive, + // unlike the default MySQL dialect behavior + SolrStream solrStream = new SolrStream(baseUrl, sParams); + Tuple tuple = getTuple(new ExceptionStream(solrStream)); + assertTrue(tuple.EOF); + assertTrue(tuple.EXCEPTION); + assertTrue(tuple.getException().contains("Column 'FIELD_I' not found")); + + // Restrict the allowlist so that caseSensitive is no longer forwarded and the same query + // succeeds with case-insensitive column resolution + System.setProperty(SQLHandler.ALLOWED_CONNECTION_PARAMS_PROP, ""); + try { + assertEquals(1, getTuples(sParams, baseUrl).size()); + } finally { + System.clearProperty(SQLHandler.ALLOWED_CONNECTION_PARAMS_PROP); + } } @Test
