You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@lucene.apache.org by ds...@apache.org on 2018/09/27 19:53:44 UTC
lucene-solr:master: SOLR-5163: edismax now throws an exception when
qf refers to a nonexistent field
Repository: lucene-solr
Updated Branches:
refs/heads/master 044bc2a48 -> 9481c1f62
SOLR-5163: edismax now throws an exception when qf refers to a nonexistent field
Project: http://git-wip-us.apache.org/repos/asf/lucene-solr/repo
Commit: http://git-wip-us.apache.org/repos/asf/lucene-solr/commit/9481c1f6
Tree: http://git-wip-us.apache.org/repos/asf/lucene-solr/tree/9481c1f6
Diff: http://git-wip-us.apache.org/repos/asf/lucene-solr/diff/9481c1f6
Branch: refs/heads/master
Commit: 9481c1f623b77214a2a14ad18efc59fb406ed765
Parents: 044bc2a
Author: Charles Sanders <sa...@gmail.com>
Authored: Thu Sep 27 15:53:26 2018 -0400
Committer: David Smiley <ds...@apache.org>
Committed: Thu Sep 27 15:53:26 2018 -0400
----------------------------------------------------------------------
solr/CHANGES.txt | 2 +
.../solr/search/ExtendedDismaxQParser.java | 89 +++++++++++++++++++-
.../solr/search/TestExtendedDismaxParser.java | 33 +++++++-
3 files changed, 120 insertions(+), 4 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/9481c1f6/solr/CHANGES.txt
----------------------------------------------------------------------
diff --git a/solr/CHANGES.txt b/solr/CHANGES.txt
index 9ef4145..408ab53 100644
--- a/solr/CHANGES.txt
+++ b/solr/CHANGES.txt
@@ -70,6 +70,8 @@ Other Changes
* SOLR-12586: Upgrade ParseDateFieldUpdateProcessorFactory (present in "schemaless mode") to use Java 8's
java.time.DateTimeFormatter instead of Joda time (see upgrade notes). "Lenient" is enabled. Removed Joda Time dependency.
(David Smiley, Bar Rotstein)
+
+* SOLR-5163: edismax now throws an exception when qf refers to a nonexistent field (Charles Sanders, David Smiley)
* SOLR-12805: Store previous term (generation) of replica when start recovery process (Cao Manh Dat)
http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/9481c1f6/solr/core/src/java/org/apache/solr/search/ExtendedDismaxQParser.java
----------------------------------------------------------------------
diff --git a/solr/core/src/java/org/apache/solr/search/ExtendedDismaxQParser.java b/solr/core/src/java/org/apache/solr/search/ExtendedDismaxQParser.java
index 9d6cd59..3e99d76 100644
--- a/solr/core/src/java/org/apache/solr/search/ExtendedDismaxQParser.java
+++ b/solr/core/src/java/org/apache/solr/search/ExtendedDismaxQParser.java
@@ -48,6 +48,7 @@ import org.apache.lucene.search.PhraseQuery;
import org.apache.lucene.search.Query;
import org.apache.lucene.util.Version;
import org.apache.solr.analysis.TokenizerChain;
+import org.apache.solr.common.SolrException;
import org.apache.solr.common.params.DisMaxParams;
import org.apache.solr.common.params.SolrParams;
import org.apache.solr.common.util.NamedList;
@@ -55,6 +56,8 @@ import org.apache.solr.parser.QueryParser;
import org.apache.solr.parser.SolrQueryParserBase.MagicFieldName;
import org.apache.solr.request.SolrQueryRequest;
import org.apache.solr.schema.FieldType;
+import org.apache.solr.schema.IndexSchema;
+import org.apache.solr.search.ExtendedDismaxQParser.ExtendedSolrQueryParser.Alias;
import org.apache.solr.util.SolrPluginUtils;
import com.google.common.collect.Multimap;
@@ -144,6 +147,7 @@ public class ExtendedDismaxQParser extends QParser {
ExtendedSolrQueryParser up = createEdismaxQueryParser(this, IMPOSSIBLE_FIELD_NAME);
up.addAlias(IMPOSSIBLE_FIELD_NAME, config.tiebreaker, config.queryFields);
addAliasesFromRequest(up, config.tiebreaker);
+ validateQueryFields(up);
up.setPhraseSlop(config.qslop); // slop for explicit user phrase queries
up.setAllowLeadingWildcard(true);
up.setAllowSubQueryParsing(config.userFields.isAllowed(MagicFieldName.QUERY.field));
@@ -206,6 +210,84 @@ public class ExtendedDismaxQParser extends QParser {
}
/**
+ * Validate query field names. Must be explicitly defined in the schema or match a dynamic field pattern.
+ * Checks source field(s) represented by a field alias
+ *
+ * @param up parser used
+ * @throws SyntaxError for invalid field name
+ */
+ protected void validateQueryFields(ExtendedSolrQueryParser up) throws SyntaxError {
+ List<String> flds = new ArrayList<>(config.queryFields.keySet().size());
+ for (String fieldName : config.queryFields.keySet()) {
+ buildQueryFieldList(fieldName, up.getAlias(fieldName), flds, up);
+ }
+
+ checkFieldsInSchema(flds);
+ }
+
+ /**
+ * Build list of source (non-alias) query field names. Recursive through aliases.
+ *
+ * @param fieldName query field name
+ * @param alias field alias
+ * @param flds list of query field names
+ * @param up parser used
+ * @throws SyntaxError for invalid field name
+ */
+ private void buildQueryFieldList(String fieldName, Alias alias, List<String> flds, ExtendedSolrQueryParser up) throws SyntaxError {
+ if (null == alias) {
+ flds.add(fieldName);
+ return;
+ }
+
+ up.validateCyclicAliasing(fieldName);
+ flds.addAll(getFieldsFromAlias(up, alias));
+ }
+
+ /**
+ * Return list of source (non-alias) field names from an alias
+ *
+ * @param up parser used
+ * @param a field alias
+ * @return list of source fields
+ * @throws SyntaxError for invalid field name
+ */
+ private List<String> getFieldsFromAlias(ExtendedSolrQueryParser up, Alias a) throws SyntaxError {
+ List<String> lst = new ArrayList<>();
+ for (String s : a.fields.keySet()) {
+ buildQueryFieldList(s, up.getAlias(s), lst, up);
+ }
+
+ return lst;
+ }
+
+ /**
+ * Verify field name exists in schema, explicit or dynamic field pattern
+ *
+ * @param fieldName source field name to verify
+ * @throws SyntaxError for invalid field name
+ */
+ private void checkFieldInSchema(String fieldName) throws SyntaxError {
+ try {
+ config.schema.getField(fieldName);
+ } catch (SolrException se) {
+ throw new SyntaxError("Query Field '" + fieldName + "' is not a valid field name", se);
+ }
+ }
+
+ /**
+ * Verify list of source field names
+ *
+ * @param flds list of source field names to verify
+ * @throws SyntaxError for invalid field name
+ */
+ private void checkFieldsInSchema(List<String> flds) throws SyntaxError {
+ for (String fieldName : flds) {
+ checkFieldInSchema(fieldName);
+ }
+ }
+
+ /**
* Adds shingled phrase queries to all the fields specified in the pf, pf2 anf pf3 parameters
*
*/
@@ -1597,15 +1679,18 @@ public class ExtendedDismaxQParser extends QParser {
protected String[] boostFuncs;
protected boolean splitOnWhitespace;
+
+ protected IndexSchema schema;
public ExtendedDismaxConfiguration(SolrParams localParams,
SolrParams params, SolrQueryRequest req) {
solrParams = SolrParams.wrapDefaults(localParams, params);
- minShouldMatch = DisMaxQParser.parseMinShouldMatch(req.getSchema(), solrParams); // req.getSearcher() here causes searcher refcount imbalance
+ schema = req.getSchema();
+ minShouldMatch = DisMaxQParser.parseMinShouldMatch(schema, solrParams); // req.getSearcher() here causes searcher refcount imbalance
final boolean forbidSubQueryByDefault = req.getCore().getSolrConfig().luceneMatchVersion.onOrAfter(Version.LUCENE_7_2_0);
userFields = new UserFields(U.parseFieldBoosts(solrParams.getParams(DMP.UF)), forbidSubQueryByDefault);
try {
- queryFields = DisMaxQParser.parseQueryFields(req.getSchema(), solrParams); // req.getSearcher() here causes searcher refcount imbalance
+ queryFields = DisMaxQParser.parseQueryFields(schema, solrParams); // req.getSearcher() here causes searcher refcount imbalance
} catch (SyntaxError e) {
throw new RuntimeException(e);
}
http://git-wip-us.apache.org/repos/asf/lucene-solr/blob/9481c1f6/solr/core/src/test/org/apache/solr/search/TestExtendedDismaxParser.java
----------------------------------------------------------------------
diff --git a/solr/core/src/test/org/apache/solr/search/TestExtendedDismaxParser.java b/solr/core/src/test/org/apache/solr/search/TestExtendedDismaxParser.java
index f77baaf..ff9a2c4 100644
--- a/solr/core/src/test/org/apache/solr/search/TestExtendedDismaxParser.java
+++ b/solr/core/src/test/org/apache/solr/search/TestExtendedDismaxParser.java
@@ -38,6 +38,7 @@ import org.apache.solr.common.params.ModifiableSolrParams;
import org.apache.solr.common.params.SolrParams;
import org.apache.solr.request.SolrQueryRequest;
import org.apache.solr.util.SolrPluginUtils;
+import org.junit.Assert;
import org.junit.BeforeClass;
import org.junit.Test;
import org.noggit.ObjectBuilder;
@@ -672,7 +673,8 @@ public class TestExtendedDismaxParser extends SolrTestCaseJ4 {
try {
h.query(req("defType","edismax", "q","blarg", "qf","field1", "f.field1.qf","field2 field3","f.field2.qf","field4 field5", "f.field4.qf","field5", "f.field5.qf","field6", "f.field3.qf","field6"));
} catch (SolrException e) {
- fail("This is not cyclic alising");
+ assertFalse("This is not cyclic alising", e.getCause().getMessage().contains("Field aliases lead to a cycle"));
+ assertTrue(e.getCause().getMessage().contains("not a valid field name"));
}
try {
@@ -683,7 +685,7 @@ public class TestExtendedDismaxParser extends SolrTestCaseJ4 {
}
try {
- h.query(req("defType","edismax", "q","who:(Zapp Pig)", "qf","field1", "f.who.qf","name","f.name.qf","myalias", "f.myalias.qf","who"));
+ h.query(req("defType","edismax", "q","who:(Zapp Pig)", "qf","text", "f.who.qf","name","f.name.qf","myalias", "f.myalias.qf","who"));
fail("Cyclic alising not detected");
} catch (SolrException e) {
assertTrue(e.getCause().getMessage().contains("Field aliases lead to a cycle"));
@@ -2090,5 +2092,32 @@ public class TestExtendedDismaxParser extends SolrTestCaseJ4 {
public void killInfiniteRecursionParse() throws Exception {
assertJQ(req("defType", "edismax", "q", "*", "qq", "{!edismax v=something}", "bq", "{!edismax v=$qq}"));
}
+
+ /** SOLR-5163 */
+ @Test
+ public void testValidateQueryFields() throws Exception {
+ // field aliasing covered by test - testAliasing
+ ModifiableSolrParams params = new ModifiableSolrParams();
+ params.add("defType", "edismax");
+ params.add("df", "text");
+ params.add("q", "olive AND other");
+ params.add("qf", "subject^3 title");
+ params.add("debugQuery", "true");
+
+ // test valid field names
+ try (SolrQueryRequest req = req(params)) {
+ String response = h.query(req);
+ response.contains("+DisjunctionMaxQuery((title:olive | (subject:oliv)^3.0)) +DisjunctionMaxQuery((title:other | (subject:other)^3.0))");
+ }
+
+ // test invalid field name
+ params.set("qf", "subject^3 nosuchfield");
+ try (SolrQueryRequest req = req(params)) {
+ h.query(req);
+ } catch (Exception e) {
+ Assert.assertEquals("org.apache.solr.search.SyntaxError: Query Field 'nosuchfield' is not a valid field name", e.getMessage());
+ }
+
+ }
}