From 9f72a776bd7a2e2ad9b33e50c21c2d379da5e411 Mon Sep 17 00:00:00 2001 From: "Chris M. Hostetter" Date: Thu, 1 Aug 2013 18:18:10 +0000 Subject: [PATCH] SOLR-4953: Make XML Configuration parsing fail if an xpath matches multiple nodes when only a single value is expected. git-svn-id: https://svn.apache.org/repos/asf/lucene/dev/trunk@1509359 13f79535-47bb-0310-9956-ffa450edef68 --- solr/CHANGES.txt | 12 +++++++ .../src/java/org/apache/solr/core/Config.java | 17 +++++---- .../java/org/apache/solr/core/SolrConfig.java | 4 +-- .../apache/solr/update/SolrIndexConfig.java | 8 +++-- .../conf/bad-solrconfig-multiple-cfs.xml | 30 ++++++++++++++++ .../bad-solrconfig-multiple-indexconfigs.xml | 35 +++++++++++++++++++ .../org/apache/solr/core/TestBadConfig.java | 9 +++++ 7 files changed, 104 insertions(+), 11 deletions(-) create mode 100644 solr/core/src/test-files/solr/collection1/conf/bad-solrconfig-multiple-cfs.xml create mode 100644 solr/core/src/test-files/solr/collection1/conf/bad-solrconfig-multiple-indexconfigs.xml diff --git a/solr/CHANGES.txt b/solr/CHANGES.txt index bfbf025d6ec..9f841896bb6 100644 --- a/solr/CHANGES.txt +++ b/solr/CHANGES.txt @@ -61,6 +61,15 @@ Apache ZooKeeper 3.4.5 Upgrading from Solr 4.4.0 ---------------------- +* XML configuration parsing is now more strict about situations where a single + setting is allowed but multiple values are found. In the past, one value + would be chosen arbitrarily and silently. Starting with 4.5, configuration + parsing will fail with an error in situations like this. If you see error + messages such as "solrconfig.xml contains more than one value for config path: + indexConfig/infoStream" check your solrconfig.xml file for multiple occurrences + of "infoStream" and delete the one that you do not wish to use. See SOLR-4953 + for more details. + Detailed Change List ---------------------- @@ -107,6 +116,9 @@ Other Changes * SOLR-4951: Better randomization of MergePolicy in Solr tests (hossman) +* SOLR-4953: Make XML Configuration parsing fail if an xpath matches multiple + nodes when only a single value is expected. (hossman) + ================== 4.4.0 ================== Versions of Major Components diff --git a/solr/core/src/java/org/apache/solr/core/Config.java b/solr/core/src/java/org/apache/solr/core/Config.java index 7c855206bcf..82c044fa0e8 100644 --- a/solr/core/src/java/org/apache/solr/core/Config.java +++ b/solr/core/src/java/org/apache/solr/core/Config.java @@ -233,14 +233,13 @@ public class Config { } public Node getNode(String path, Document doc, boolean errIfMissing) { - XPath xpath = xpathFactory.newXPath(); - Node nd = null; - String xstr = normalize(path); + XPath xpath = xpathFactory.newXPath(); + String xstr = normalize(path); try { - nd = (Node)xpath.evaluate(xstr, doc, XPathConstants.NODE); - - if (nd==null) { + NodeList nodes = (NodeList)xpath.evaluate(xstr, doc, + XPathConstants.NODESET); + if (nodes==null || 0 == nodes.getLength() ) { if (errIfMissing) { throw new RuntimeException(name + " missing "+path); } else { @@ -248,7 +247,11 @@ public class Config { return null; } } - + if ( 1 < nodes.getLength() ) { + throw new SolrException( SolrException.ErrorCode.SERVER_ERROR, + name + " contains more than one value for config path: " + path); + } + Node nd = nodes.item(0); log.trace(name + ":" + path + "=" + nd); return nd; diff --git a/solr/core/src/java/org/apache/solr/core/SolrConfig.java b/solr/core/src/java/org/apache/solr/core/SolrConfig.java index fd887b9afd9..4ddb9050fc5 100644 --- a/solr/core/src/java/org/apache/solr/core/SolrConfig.java +++ b/solr/core/src/java/org/apache/solr/core/SolrConfig.java @@ -124,8 +124,8 @@ public class SolrConfig extends Config { // Old indexDefaults and mainIndex sections are deprecated and fails fast for luceneMatchVersion=>LUCENE_40. // For older solrconfig.xml's we allow the old sections, but never mixed with the new - boolean hasDeprecatedIndexConfig = get("indexDefaults/text()", null) != null || get("mainIndex/text()", null) != null; - boolean hasNewIndexConfig = get("indexConfig/text()", null) != null; + boolean hasDeprecatedIndexConfig = (getNode("indexDefaults", false) != null) || (getNode("mainIndex", false) != null); + boolean hasNewIndexConfig = getNode("indexConfig", false) != null; if(hasDeprecatedIndexConfig){ if(luceneMatchVersion.onOrAfter(Version.LUCENE_40)) { throw new SolrException(ErrorCode.FORBIDDEN, " and configuration sections are discontinued. Use instead."); diff --git a/solr/core/src/java/org/apache/solr/update/SolrIndexConfig.java b/solr/core/src/java/org/apache/solr/update/SolrIndexConfig.java index f7d592e1c88..8b7b1506425 100644 --- a/solr/core/src/java/org/apache/solr/update/SolrIndexConfig.java +++ b/solr/core/src/java/org/apache/solr/update/SolrIndexConfig.java @@ -117,15 +117,19 @@ public class SolrIndexConfig { def = new SolrIndexConfig(solrConfig); } + // sanity check: this will throw an error for us if there is more then one + // config section + Object unused = solrConfig.getNode(prefix, false); + luceneVersion = solrConfig.luceneMatchVersion; // Assert that end-of-life parameters or syntax is not in our config. // Warn for luceneMatchVersion's before LUCENE_36, fail fast above assertWarnOrFail("The myclass syntax is no longer supported in solrconfig.xml. Please use syntax instead.", - !((solrConfig.get(prefix+"/mergeScheduler/text()",null) != null) && (solrConfig.get(prefix+"/mergeScheduler/@class",null) == null)), + !((solrConfig.getNode(prefix+"/mergeScheduler",false) != null) && (solrConfig.get(prefix+"/mergeScheduler/@class",null) == null)), true); assertWarnOrFail("The myclass syntax is no longer supported in solrconfig.xml. Please use syntax instead.", - !((solrConfig.get(prefix+"/mergePolicy/text()",null) != null) && (solrConfig.get(prefix+"/mergePolicy/@class",null) == null)), + !((solrConfig.getNode(prefix+"/mergePolicy",false) != null) && (solrConfig.get(prefix+"/mergePolicy/@class",null) == null)), true); assertWarnOrFail("The true|false parameter is no longer valid in solrconfig.xml.", solrConfig.get(prefix+"/luceneAutoCommit", null) == null, diff --git a/solr/core/src/test-files/solr/collection1/conf/bad-solrconfig-multiple-cfs.xml b/solr/core/src/test-files/solr/collection1/conf/bad-solrconfig-multiple-cfs.xml new file mode 100644 index 00000000000..f13acb3f6b0 --- /dev/null +++ b/solr/core/src/test-files/solr/collection1/conf/bad-solrconfig-multiple-cfs.xml @@ -0,0 +1,30 @@ + + + + + + + ${tests.luceneMatchVersion:LUCENE_CURRENT} + + + + true + false + + + diff --git a/solr/core/src/test-files/solr/collection1/conf/bad-solrconfig-multiple-indexconfigs.xml b/solr/core/src/test-files/solr/collection1/conf/bad-solrconfig-multiple-indexconfigs.xml new file mode 100644 index 00000000000..00dd08c36fe --- /dev/null +++ b/solr/core/src/test-files/solr/collection1/conf/bad-solrconfig-multiple-indexconfigs.xml @@ -0,0 +1,35 @@ + + + + + + + ${tests.luceneMatchVersion:LUCENE_CURRENT} + + + true + false + + + + ${useCompoundFile:false} + true + + + + diff --git a/solr/core/src/test/org/apache/solr/core/TestBadConfig.java b/solr/core/src/test/org/apache/solr/core/TestBadConfig.java index 98c8e40453a..efae0f0ab34 100644 --- a/solr/core/src/test/org/apache/solr/core/TestBadConfig.java +++ b/solr/core/src/test/org/apache/solr/core/TestBadConfig.java @@ -27,6 +27,15 @@ public class TestBadConfig extends AbstractBadConfigTestBase { assertConfigs("bad_solrconfig.xml","schema.xml","unset.sys.property"); } + public void testMultipleIndexConfigs() throws Exception { + assertConfigs("bad-solrconfig-multiple-indexconfigs.xml", "schema12.xml", + "indexConfig"); + } + public void testMultipleCFS() throws Exception { + assertConfigs("bad-solrconfig-multiple-cfs.xml", "schema12.xml", + "useCompoundFile"); + } + public void testUpdateLogButNoVersionField() throws Exception { System.setProperty("enable.update.log", "true");