diff --git a/solr/CHANGES.txt b/solr/CHANGES.txt index ee5afa17669e..063334b33703 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 a4084d1509de..c05c7582ca71 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.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 @@ protected SolrQueryParserBase() { 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 String getDefaultField() { /** 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 @@ Query handleBoost(Query q, Token boost) { 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 @@ protected ReversedWildcardFilterFactory getReversedWildcardFilterFactory(FieldTy 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 @@ protected Query getFieldQuery(String field, String queryText, boolean quoted, bo } 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 @@ protected Query getFieldQuery(String field, List queryTerms, boolean raw } 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 000000000000..68dd38c10f91 --- /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 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)); + } +}