Skip to content

Commit

Permalink
SOLR-13205: Improve empty-string handling in SolrQueryParserBase
Browse files Browse the repository at this point in the history
Contributed-By: pramodkumar9
  • Loading branch information
Jason Gerlowski committed Jul 20, 2020
1 parent b46321e commit 48e92ba
Show file tree
Hide file tree
Showing 3 changed files with 156 additions and 11 deletions.
3 changes: 3 additions & 0 deletions solr/CHANGES.txt
Original file line number Diff line number Diff line change
Expand Up @@ -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
---------------------

Expand Down
27 changes: 16 additions & 11 deletions solr/core/src/java/org/apache/solr/parser/SolrQueryParserBase.java
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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");
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -1116,7 +1121,7 @@ protected Query getFieldQuery(String field, List<String> 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);
Expand Down
137 changes: 137 additions & 0 deletions solr/core/src/test/org/apache/solr/parser/SolrQueryParserBaseTest.java
Original file line number Diff line number Diff line change
@@ -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));
}
}

0 comments on commit 48e92ba

Please sign in to comment.