From de6ce53638ac1d6e4feaa1d6c1532e0fc665c6fc Mon Sep 17 00:00:00 2001 From: Justin Sweeney Date: Fri, 1 Sep 2023 08:41:01 -0600 Subject: [PATCH] Handling Query Parsing Exceptions from Lucene queries (#129) * Handling IOExceptions from Lucene queries which are generally exceptions caused by an invalid query so they should be returned as 400s * Updating to handle specific exceptions and adding more tests * Tidying code and fixing imports * Improving error handling in test * Simplifying test code * Fixing indentation and test system property --- .../solr/handler/component/SearchHandler.java | 6 ++ .../handler/component/SearchHandlerTest.java | 87 +++++++++++++++++++ .../solr/cloud/MiniSolrCloudCluster.java | 1 + 3 files changed, 94 insertions(+) diff --git a/solr/core/src/java/org/apache/solr/handler/component/SearchHandler.java b/solr/core/src/java/org/apache/solr/handler/component/SearchHandler.java index 17e1229fc66..89316672cba 100644 --- a/solr/core/src/java/org/apache/solr/handler/component/SearchHandler.java +++ b/solr/core/src/java/org/apache/solr/handler/component/SearchHandler.java @@ -39,6 +39,8 @@ import java.util.concurrent.atomic.AtomicLong; import java.util.stream.StreamSupport; import org.apache.lucene.index.ExitableDirectoryReader; +import org.apache.lucene.queryparser.surround.query.TooManyBasicQueries; +import org.apache.lucene.search.IndexSearcher; import org.apache.lucene.search.TotalHits; import org.apache.solr.client.solrj.SolrServerException; import org.apache.solr.cloud.ZkController; @@ -489,6 +491,10 @@ public void handleRequestBody(SolrQueryRequest req, SolrQueryResponse rsp) throw .getResponseHeader() .asShallowMap() .put(SolrQueryResponse.RESPONSE_HEADER_PARTIAL_RESULTS_KEY, Boolean.TRUE); + } catch (IndexSearcher.TooManyClauses | TooManyBasicQueries ex) { + // An IOException on a non-distributed request is typically an issue parsing the query at + // the lucene level + throw new SolrException(SolrException.ErrorCode.BAD_REQUEST, ex); } finally { SolrQueryTimeoutImpl.reset(); } diff --git a/solr/core/src/test/org/apache/solr/handler/component/SearchHandlerTest.java b/solr/core/src/test/org/apache/solr/handler/component/SearchHandlerTest.java index 0f22a624093..da861b79d99 100644 --- a/solr/core/src/test/org/apache/solr/handler/component/SearchHandlerTest.java +++ b/solr/core/src/test/org/apache/solr/handler/component/SearchHandlerTest.java @@ -21,8 +21,13 @@ import java.util.Collection; import java.util.Iterator; import java.util.List; +import org.apache.lucene.search.TermInSetQuery; +import org.apache.lucene.util.BytesRef; import org.apache.solr.SolrTestCaseJ4; import org.apache.solr.client.solrj.SolrClient; +import org.apache.solr.client.solrj.SolrQuery; +import org.apache.solr.client.solrj.SolrRequest; +import org.apache.solr.client.solrj.impl.BaseHttpSolrClient; import org.apache.solr.client.solrj.impl.CloudSolrClient; import org.apache.solr.client.solrj.impl.HttpSolrClient; import org.apache.solr.client.solrj.request.CollectionAdminRequest; @@ -30,6 +35,7 @@ import org.apache.solr.client.solrj.response.QueryResponse; import org.apache.solr.cloud.MiniSolrCloudCluster; import org.apache.solr.cloud.SolrCloudTestCase; +import org.apache.solr.common.SolrInputDocument; import org.apache.solr.common.cloud.Replica; import org.apache.solr.common.cloud.Slice; import org.apache.solr.common.params.ModifiableSolrParams; @@ -283,6 +289,87 @@ public void testRequireZkConnectedDistrib() throws Exception { } } + @Test + public void testLuceneIOExceptionHandling() throws Exception { + String initialMaxBooleanClauses = System.getProperty("solr.max.booleanClauses"); + System.setProperty("solr.max.booleanClauses", String.valueOf(1)); + MiniSolrCloudCluster miniCluster = + new MiniSolrCloudCluster(1, createTempDir(), buildJettyConfig("/solr")); + + final CloudSolrClient cloudSolrClient = miniCluster.getSolrClient(); + + try { + assertNotNull(miniCluster.getZkServer()); + List jettys = miniCluster.getJettySolrRunners(); + assertEquals(1, jettys.size()); + for (JettySolrRunner jetty : jettys) { + assertTrue(jetty.isRunning()); + } + + // create collection + String collectionName = "testLuceneIOExceptionHandling"; + String configName = collectionName + "Config"; + miniCluster.uploadConfigSet( + SolrTestCaseJ4.TEST_PATH().resolve("collection1/conf"), configName); + + CollectionAdminRequest.createCollection(collectionName, configName, 2, 1) + .setPerReplicaState(SolrCloudTestCase.USE_PER_REPLICA_STATE) + .process(miniCluster.getSolrClient()); + + for (int i = 0; i < 5; i++) { + SolrInputDocument doc = new SolrInputDocument(); + doc.addField("id", i); + doc.addField("title", "test" + i); + cloudSolrClient.add(collectionName, doc); + } + cloudSolrClient.commit(collectionName); + + Collection terms = new ArrayList<>(); + for (int i = 0; i < 10; i++) { + terms.add(newBytesRef("term" + i)); + } + TermInSetQuery termInSetQuery = new TermInSetQuery("name", terms); + SolrQuery solrQuery = new SolrQuery(termInSetQuery.toString()); + final QueryRequest req = new QueryRequest(solrQuery); + req.setMethod(SolrRequest.METHOD.POST); + BaseHttpSolrClient.RemoteSolrException e = + assertThrows( + BaseHttpSolrClient.RemoteSolrException.class, + () -> req.process(cloudSolrClient, collectionName)); + assertEquals(400, e.code()); + assertTrue(e.getMessage().contains("org.apache.solr.search.SyntaxError")); + + solrQuery = new SolrQuery("{!surround maxBasicQueries=1 df=title}99W(test1,test2,test3)"); + final QueryRequest req2 = new QueryRequest(solrQuery); + req2.setMethod(SolrRequest.METHOD.POST); + e = + assertThrows( + BaseHttpSolrClient.RemoteSolrException.class, + () -> req2.process(cloudSolrClient, collectionName)); + assertEquals(400, e.code()); + assertTrue(e.getMessage().contains("Exceeded maximum of 1 basic queries.")); + + solrQuery = new SolrQuery("{!surround maxBasicQueries=1000 df=title}10W(tes*,test*)"); + final QueryRequest req3 = new QueryRequest(solrQuery); + req3.setMethod(SolrRequest.METHOD.POST); + e = + assertThrows( + BaseHttpSolrClient.RemoteSolrException.class, + () -> req3.process(cloudSolrClient, collectionName)); + assertEquals(400, e.code()); + assertTrue( + e.getMessage() + .contains("Query contains too many nested clauses; maxClauseCount is set to 1")); + } finally { + if (initialMaxBooleanClauses != null) { + System.setProperty("solr.max.booleanClauses", initialMaxBooleanClauses); + } else { + System.clearProperty("solr.max.booleanClauses"); + } + miniCluster.shutdown(); + } + } + private static T getRandomEntry(Collection collection) { if (null == collection || collection.isEmpty()) return null; diff --git a/solr/test-framework/src/java/org/apache/solr/cloud/MiniSolrCloudCluster.java b/solr/test-framework/src/java/org/apache/solr/cloud/MiniSolrCloudCluster.java index 704cbfa8589..6e7c28152c5 100644 --- a/solr/test-framework/src/java/org/apache/solr/cloud/MiniSolrCloudCluster.java +++ b/solr/test-framework/src/java/org/apache/solr/cloud/MiniSolrCloudCluster.java @@ -104,6 +104,7 @@ public class MiniSolrCloudCluster { "\n" + "\n" + " ${shareSchema:false}\n" + + " ${solr.max.booleanClauses:1024}\n" + " ${solr.allowPaths:}\n" + " ${configSetBaseDir:configsets}\n" + " ${coreRootDirectory:.}\n"