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

@@ -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

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();
+    XPath xpath = xpathFactory.newXPath();
-   Node nd = null;
+    String xstr = normalize(path);
-   String xstr = normalize(path);
 
     try {
-      nd = (Node)xpath.evaluate(xstr, doc, XPathConstants.NODE);
+      NodeList nodes = (NodeList)xpath.evaluate(xstr, doc, 
-
+                                                XPathConstants.NODESET);
-      if (nd==null) {
+      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;
 

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 <indexConfig>
-    boolean hasDeprecatedIndexConfig = get("indexDefaults/text()", null) != null || get("mainIndex/text()", null) != null;
+    boolean hasDeprecatedIndexConfig = (getNode("indexDefaults", false) != null) || (getNode("mainIndex", false) != null);
-    boolean hasNewIndexConfig = get("indexConfig/text()", null) != null; 
+    boolean hasNewIndexConfig = getNode("indexConfig", false) != null; 
     if(hasDeprecatedIndexConfig){
       if(luceneMatchVersion.onOrAfter(Version.LUCENE_40)) {
         throw new SolrException(ErrorCode.FORBIDDEN, "<indexDefaults> and <mainIndex> configuration sections are discontinued. Use <indexConfig> instead.");

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 <mergeScheduler>myclass</mergeScheduler> syntax is no longer supported in solrconfig.xml. Please use syntax <mergeScheduler class=\"myclass\"/> 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 <mergePolicy>myclass</mergePolicy> syntax is no longer supported in solrconfig.xml. Please use syntax <mergePolicy class=\"myclass\"/> 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 <luceneAutoCommit>true|false</luceneAutoCommit> parameter is no longer valid in solrconfig.xml.",
         solrConfig.get(prefix+"/luceneAutoCommit", null) == null,

solr/core/src/test-files/solr/collection1/conf/bad-solrconfig-multiple-cfs.xml

@@ -0,0 +1,30 @@
+<?xml version="1.0" ?>
+
+<!--
+ 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.
+-->
+
+<config>
+
+  <luceneMatchVersion>${tests.luceneMatchVersion:LUCENE_CURRENT}</luceneMatchVersion>
+
+  <indexConfig>
+    <!-- BEGIN BAD: multiple useCompoundFile -->
+    <useCompoundFile>true</useCompoundFile>
+    <useCompoundFile>false</useCompoundFile>
+  </indexConfig>
+
+</config>

solr/core/src/test-files/solr/collection1/conf/bad-solrconfig-multiple-indexconfigs.xml

@@ -0,0 +1,35 @@
+<?xml version="1.0" ?>
+
+<!--
+ 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.
+-->
+
+<config>
+
+  <luceneMatchVersion>${tests.luceneMatchVersion:LUCENE_CURRENT}</luceneMatchVersion>
+
+  <indexConfig>
+    <useCompoundFile>true</useCompoundFile>
+    <unlockOnStartup>false</unlockOnStartup>
+  </indexConfig>
+  <!-- BEGIN BAD: multiple indexConfig sections -->
+  <indexConfig>
+    <useCompoundFile>${useCompoundFile:false}</useCompoundFile>
+    <unlockOnStartup>true</unlockOnStartup>
+  </indexConfig>
+  <!-- END BAD -->
+
+</config>

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");