You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@lucene.apache.org by ge...@apache.org on 2020/07/20 13:51:48 UTC
[lucene-solr] branch master updated: SOLR-13205: Improve
empty-string handling in SolrQueryParserBase
This is an automated email from the ASF dual-hosted git repository.
gerlowskija pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/lucene-solr.git
The following commit(s) were added to refs/heads/master by this push:
new 48e92ba SOLR-13205: Improve empty-string handling in SolrQueryParserBase
48e92ba is described below
commit 48e92ba9c74ebf204308793d9a00bc48b2780136
Author: Jason Gerlowski <ja...@lucidworks.com>
AuthorDate: Fri Jul 17 19:38:53 2020 -0400
SOLR-13205: Improve empty-string handling in SolrQueryParserBase
Contributed-By: pramodkumar9
---
solr/CHANGES.txt | 3 +
.../apache/solr/parser/SolrQueryParserBase.java | 27 ++--
.../solr/parser/SolrQueryParserBaseTest.java | 137 +++++++++++++++++++++
3 files changed, 156 insertions(+), 11 deletions(-)
diff --git a/solr/CHANGES.txt b/solr/CHANGES.txt
index ee5afa1..063334b 100644
--- a/solr/CHANGES.txt
+++ b/solr/CHANGES.txt
@@ -128,6 +128,9 @@ Improvements
* SOLR-11262: Implement writeMap and writeIterator in XMLWriter (yonik, Munendra S N)
+* SOLR-13205: Prevent StringIndexOutOfBoundsException when parsing field names in SolrQueryParserBase
+ (pramodkumar9 via Jason Gerlowski)
+
Optimizations
---------------------
diff --git a/solr/core/src/java/org/apache/solr/parser/SolrQueryParserBase.java b/solr/core/src/java/org/apache/solr/parser/SolrQueryParserBase.java
index a4084d1..c05c758 100644
--- a/solr/core/src/java/org/apache/solr/parser/SolrQueryParserBase.java
+++ b/solr/core/src/java/org/apache/solr/parser/SolrQueryParserBase.java
@@ -27,6 +27,7 @@ import java.util.List;
import java.util.Map;
import java.util.stream.Collectors;
+import com.google.common.base.Strings;
import org.apache.lucene.analysis.Analyzer;
import org.apache.lucene.analysis.reverse.ReverseStringFilter;
import org.apache.lucene.analysis.util.TokenFilterFactory;
@@ -231,17 +232,21 @@ public abstract class SolrQueryParserBase extends QueryBuilder {
public void init(String defaultField, QParser parser) {
+ if ((parser == null) || (parser.getReq() == null) || (parser.getReq().getSchema() == null)) {
+ throw new SolrException
+ (SolrException.ErrorCode.BAD_REQUEST,
+ "query parser is null or invalid");
+ }
+ if ((defaultField != null) && (defaultField.isEmpty())) {
+ throw new SolrException
+ (SolrException.ErrorCode.BAD_REQUEST,
+ "default field name is empty");
+ }
this.schema = parser.getReq().getSchema();
this.parser = parser;
this.flags = parser.getFlags();
this.defaultField = defaultField;
setAnalyzer(schema.getQueryAnalyzer());
- // TODO in 8.0(?) remove this. Prior to 7.2 we defaulted to allowing sub-query parsing by default
- /*
- if (!parser.getReq().getCore().getSolrConfig().luceneMatchVersion.onOrAfter(Version.LUCENE_7_2_0)) {
- setAllowSubQueryParsing(true);
- } // otherwise defaults to false
- */
}
// Turn on the "filter" bit and return the previous flags for the caller to save
@@ -284,7 +289,7 @@ public abstract class SolrQueryParserBase extends QueryBuilder {
/** Handles the default field if null is passed */
public String getField(String fieldName) {
explicitField = fieldName;
- return fieldName != null ? fieldName : this.defaultField;
+ return !Strings.isNullOrEmpty(fieldName) ? fieldName : this.defaultField;
}
/** For a fielded query, returns the actual field specified (i.e. null if default is being used)
@@ -852,7 +857,7 @@ public abstract class SolrQueryParserBase extends QueryBuilder {
if (boost == null || boost.image.length()==0 || q == null) {
return q;
}
- if (boost.image.charAt(0) == '=') {
+ if (boost.image.startsWith("=")) {
// syntax looks like foo:x^=3
float val = Float.parseFloat(boost.image.substring(1));
Query newQ = q;
@@ -1003,7 +1008,7 @@ public abstract class SolrQueryParserBase extends QueryBuilder {
private void checkNullField(String field) throws SolrException {
- if (field == null && defaultField == null) {
+ if (Strings.isNullOrEmpty(field) && Strings.isNullOrEmpty(defaultField)) {
throw new SolrException
(SolrException.ErrorCode.BAD_REQUEST,
"no field name specified in query and no default specified via 'df' param");
@@ -1068,7 +1073,7 @@ public abstract class SolrQueryParserBase extends QueryBuilder {
} else {
// intercept magic field name of "_" to use as a hook for our
// own functions.
- if (allowSubQueryParsing && field.charAt(0) == '_' && parser != null) {
+ if (allowSubQueryParsing && field.startsWith("_") && parser != null) {
MagicFieldName magic = MagicFieldName.get(field);
if (null != magic) {
subQParser = parser.subQuery(queryText, magic.subParser);
@@ -1116,7 +1121,7 @@ public abstract class SolrQueryParserBase extends QueryBuilder {
} else {
// intercept magic field name of "_" to use as a hook for our
// own functions.
- if (allowSubQueryParsing && field.charAt(0) == '_' && parser != null) {
+ if (allowSubQueryParsing && field.startsWith("_") && parser != null) {
MagicFieldName magic = MagicFieldName.get(field);
if (null != magic) {
subQParser = parser.subQuery(String.join(" ", queryTerms), magic.subParser);
diff --git a/solr/core/src/test/org/apache/solr/parser/SolrQueryParserBaseTest.java b/solr/core/src/test/org/apache/solr/parser/SolrQueryParserBaseTest.java
new file mode 100644
index 0000000..68dd38c
--- /dev/null
+++ b/solr/core/src/test/org/apache/solr/parser/SolrQueryParserBaseTest.java
@@ -0,0 +1,137 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.solr.parser;
+
+import org.apache.lucene.search.Query;
+import org.apache.solr.common.SolrException;
+import org.apache.solr.request.SolrQueryRequest;
+import org.apache.solr.schema.IndexSchema;
+import org.apache.solr.search.QParser;
+import org.junit.Before;
+import org.junit.BeforeClass;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.mockito.Mock;
+import org.mockito.junit.MockitoJUnitRunner;
+
+import java.util.Arrays;
+import java.util.List;
+
+import static org.apache.solr.SolrTestCaseJ4.assumeWorkingMockito;
+import static org.junit.Assert.assertEquals;
+import static org.mockito.ArgumentMatchers.anyString;
+import static org.mockito.ArgumentMatchers.eq;
+import static org.mockito.Mockito.doReturn;
+
+@RunWith(MockitoJUnitRunner.class)
+public class SolrQueryParserBaseTest {
+
+ @BeforeClass
+ public static void setUpClass() {
+ assumeWorkingMockito();
+ }
+
+ private static final String DEFAULT_FIELD_NAME = "TestDefaultFieldname";
+
+ private static class MockSolrQueryParser extends SolrQueryParserBase {
+ public void ReInit(CharStream stream) {
+ }
+
+ public Query TopLevelQuery(String field) {
+ return null;
+ }
+ }
+
+ @Mock
+ private QParser qParser;
+ @Mock
+ private QParser subQParser;
+ @Mock
+ private SolrQueryRequest solrQueryRequest;
+ @Mock
+ private Query query;
+ @Mock
+ private IndexSchema indexSchema;
+
+ private MockSolrQueryParser solrQueryParser;
+
+ @Before
+ public void setUp() throws Exception {
+ solrQueryParser = new MockSolrQueryParser();
+ }
+
+ private void initQParser() {
+ doReturn(indexSchema).when(solrQueryRequest).getSchema();
+ doReturn(solrQueryRequest).when(qParser).getReq();
+ }
+
+ @Test
+ public void testInitHappyCases() {
+ initQParser();
+ solrQueryParser.init(null, qParser);
+ solrQueryParser.init(DEFAULT_FIELD_NAME, qParser);
+ }
+
+ @Test(expected = SolrException.class)
+ public void testInitBadDefaultField() {
+ solrQueryParser.init("", qParser);
+ }
+
+ @Test(expected = SolrException.class)
+ public void testInitNullQParser() {
+ solrQueryParser.init(DEFAULT_FIELD_NAME, null);
+ }
+
+ @Test(expected = SolrException.class)
+ public void testInitNullQParserReq() {
+ solrQueryParser.init(DEFAULT_FIELD_NAME, qParser);
+ }
+
+ @Test(expected = SolrException.class)
+ public void testInitNullQParserReqSchema() {
+ doReturn(solrQueryRequest).when(qParser).getReq();
+ solrQueryParser.init(DEFAULT_FIELD_NAME, qParser);
+ }
+
+ @Test
+ public void testGetField() {
+ initQParser();
+ solrQueryParser.init(DEFAULT_FIELD_NAME, qParser);
+ assertEquals(DEFAULT_FIELD_NAME, solrQueryParser.getField(null));
+ assertEquals(DEFAULT_FIELD_NAME, solrQueryParser.getField(""));
+ final String nonNullFieldName = "testFieldName";
+ assertEquals(nonNullFieldName, solrQueryParser.getField(nonNullFieldName));
+ }
+
+ @Test
+ public void testGetMagicFieldQuery() throws Exception {
+ String magicField = "_val_";
+ String magicFieldSubParser = SolrQueryParserBase.MagicFieldName.get(magicField).subParser;
+ initQParser();
+ solrQueryParser.init(DEFAULT_FIELD_NAME, qParser);
+ solrQueryParser.setAllowSubQueryParsing(true);
+ doReturn(query).when(subQParser).getQuery();
+ doReturn(subQParser).when(qParser).subQuery(anyString(), eq(magicFieldSubParser));
+
+ String queryText = "queryText";
+ List<String> queryTerms = Arrays.asList("query", "terms");
+ boolean quoted = true; //value doesn't matter for this test
+ boolean raw = true; //value doesn't matter for this test
+ assertEquals(query, solrQueryParser.getFieldQuery(magicField, queryText, quoted, raw));
+ assertEquals(query, solrQueryParser.getFieldQuery(magicField, queryTerms, raw));
+ }
+}