LUCENE-4724: disallow empty or null strings as components

git-svn-id: https://svn.apache.org/repos/asf/lucene/dev/trunk@1439350 13f79535-47bb-0310-9956-ffa450edef68

lucene/CHANGES.txt

@@ -107,6 +107,11 @@ Bug Fixes
 * LUCENE-4704: Make join queries override hashcode and equals methods.
   (Martijn van Groningen)
 
+* LUCENE-4724: Fix bug in CategoryPath which allowed passing null or empty
+  string components. This is forbidden now (throws an exception). Note that if
+  you have a taxonomy index created with such strings, you should rebuild it.
+  (Michael McCandless, Shai Erera)
+
 ======================= Lucene 4.1.0 =======================
 
 Changes in backwards compatibility policy

lucene/facet/src/java/org/apache/lucene/facet/taxonomy/CategoryPath.java

@@ -1,6 +1,6 @@
 package org.apache.lucene.facet.taxonomy;
 
-
+import java.util.Arrays;
 
 /*
  * Licensed to the Apache Software Foundation (ASF) under one or more
@@ -62,6 +62,11 @@ public class CategoryPath implements Comparable<CategoryPath> {
   /** Construct from the given path components. */
   public CategoryPath(final String... components) {
     assert components.length > 0 : "use CategoryPath.EMPTY to create an empty path";
+    for (String comp : components) {
+      if (comp == null || comp.isEmpty()) {
+        throw new IllegalArgumentException("empty or null components not allowed: " + Arrays.toString(components));
+      }
+    }
     this.components = components;
     length = components.length;
   }
@@ -73,6 +78,11 @@ public class CategoryPath implements Comparable<CategoryPath> {
       components = null;
       length = 0;
     } else {
+      for (String comp : comps) {
+        if (comp == null || comp.isEmpty()) {
+          throw new IllegalArgumentException("empty or null components not allowed: " + Arrays.toString(comps));
+        }
+      }
       components = comps;
       length = components.length;
     }

lucene/facet/src/test/org/apache/lucene/facet/taxonomy/TestCategoryPath.java

@@ -1,5 +1,7 @@
 package org.apache.lucene.facet.taxonomy;
 
+import java.util.Arrays;
+
 import org.apache.lucene.facet.FacetTestCase;
 import org.junit.Test;
 
@@ -173,9 +175,46 @@ public class TestCategoryPath extends FacetTestCase {
     pother = new CategoryPath("a/b/c/e", '/');
     assertTrue(pother.compareTo(p) > 0);
     assertTrue(p.compareTo(pother) < 0);
-    pother = new CategoryPath("a/b/c//e", '/');
-    assertTrue(pother.compareTo(p) < 0);
-    assertTrue(p.compareTo(pother) > 0);
   }
 
+  @Test
+  public void testEmptyNullComponents() throws Exception {
+    // LUCENE-4724: CategoryPath should not allow empty or null components
+    String[][] components_tests = new String[][] {
+      new String[] { "", "test" }, // empty in the beginning
+      new String[] { "test", "" }, // empty in the end
+      new String[] { "test", "", "foo" }, // empty in the middle
+      new String[] { null, "test" }, // null at the beginning
+      new String[] { "test", null }, // null in the end
+      new String[] { "test", null, "foo" }, // null in the middle
+    };
+
+    for (String[] components : components_tests) {
+      try {
+        assertNotNull(new CategoryPath(components));
+        fail("empty or null components should not be allowed: " + Arrays.toString(components));
+      } catch (IllegalArgumentException e) {
+        // ok
+      }
+    }
+    
+    String[] path_tests = new String[] {
+        "/test", // empty in the beginning
+        "test//foo", // empty in the middle
+    };
+    
+    for (String path : path_tests) {
+      try {
+        assertNotNull(new CategoryPath(path, '/'));
+        fail("empty or null components should not be allowed: " + path);
+      } catch (IllegalArgumentException e) {
+        // ok
+      }
+    }
+
+    // a trailing path separator is produces only one component
+    assertNotNull(new CategoryPath("test/", '/'));
+    
+  }
+  
 }

lucene/facet/src/test/org/apache/lucene/facet/taxonomy/writercache/cl2o/TestCompactLabelToOrdinal.java

@@ -56,6 +56,12 @@ public class TestCompactLabelToOrdinal extends FacetTestCase {
           .onUnmappableCharacter(CodingErrorAction.REPLACE)
           .onMalformedInput(CodingErrorAction.REPLACE);
       uniqueValues[i] = decoder.decode(ByteBuffer.wrap(buffer, 0, size)).toString();
+      // we cannot have empty path components, so eliminate all prefix as well
+      // as middle consecuive delimiter chars.
+      uniqueValues[i] = uniqueValues[i].replaceAll("/+", "/");
+      if (uniqueValues[i].startsWith("/")) {
+        uniqueValues[i] = uniqueValues[i].substring(1);
+      }
       if (uniqueValues[i].indexOf(CompactLabelToOrdinal.TERMINATOR_CHAR) == -1) {
         i++;
       }