Avoid forwarding unrelated request parameters to Calcite from the /sql handler

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

solr/modules/sql/src/java/org/apache/solr/handler/sql/SQLHandler.java

@@ -22,10 +22,10 @@
 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.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 void inform(SolrCore core) {
 
     if (coreContainer.isZooKeeperAware()) {
       defaultZkhost = coreContainer.getZkController().getZkServerAddress();
-      defaultWorkerCollection = core.getCoreDescriptor().getCollectionName();
       isCloud = true;
     }
 
@@ -85,8 +102,6 @@ public void handleRequestBody(SolrQueryRequest req, SolrQueryResponse rsp) throw
     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 void handleRequestBody(SolrQueryRequest req, SolrQueryResponse rsp) throw
       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 Tuple read() throws IOException {
     }
   }
 
+  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);

solr/modules/sql/src/test/org/apache/solr/handler/sql/TestSQLHandler.java

@@ -452,6 +452,61 @@ public void testBasicSelect() throws Exception {
     tuples = getTuples(sParams, baseUrl);
 
     assertEquals(0, tuples.size());
+
+    // Verify that unrelated request parameters are ignored
+    sParams =
+        params(
+            CommonParams.QT,
+            "/sql",
+            "someOtherParam",
+            "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"));
+  }
+
+  @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

solr/solr-ref-guide/modules/configuration-guide/pages/solr-properties.adoc

@@ -110,6 +110,8 @@ NOTE: Properties marked with "!" indicate inverted meaning between pre Solr 10 a
 
 |solr.solrj.http.jetty.customizer|solr.httpclient.builder.factory||A class loaded to customize HttpJettySolrClient upon creation.
 
+|solr.sql.connection.params.allowed||caseSensitive, quoting, quotedCasing, unquotedCasing, conformance, fun,typeCoercion, lenientOperatorLookup, defaultNullCollation, timeZone, locale|A comma separated list of request parameters to forward to Apache Calcite as connection properties.
+
 |solr.streamingexpressions.facet.tiered.enabled|solr.facet.stream.tiered|true|Controls whether tiered faceting is enabled for streaming expressions.
 
 |solr.streamingexpressions.macros.enabled|StreamingExpressionMacros|false|Controls whether to expand URL parameters inside of the `expr` parameter.