You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@lucene.apache.org by sh...@apache.org on 2013/01/28 12:31:42 UTC

svn commit: r1439350 - in /lucene/dev/trunk/lucene: ./ facet/src/java/org/apache/lucene/facet/taxonomy/ facet/src/test/org/apache/lucene/facet/taxonomy/ facet/src/test/org/apache/lucene/facet/taxonomy/writercache/cl2o/

Author: shaie
Date: Mon Jan 28 11:31:42 2013
New Revision: 1439350

URL: http://svn.apache.org/viewvc?rev=1439350&view=rev
Log:
LUCENE-4724: disallow empty or null strings as components

Modified:
    lucene/dev/trunk/lucene/CHANGES.txt
    lucene/dev/trunk/lucene/facet/src/java/org/apache/lucene/facet/taxonomy/CategoryPath.java
    lucene/dev/trunk/lucene/facet/src/test/org/apache/lucene/facet/taxonomy/TestCategoryPath.java
    lucene/dev/trunk/lucene/facet/src/test/org/apache/lucene/facet/taxonomy/writercache/cl2o/TestCompactLabelToOrdinal.java

Modified: lucene/dev/trunk/lucene/CHANGES.txt
URL: http://svn.apache.org/viewvc/lucene/dev/trunk/lucene/CHANGES.txt?rev=1439350&r1=1439349&r2=1439350&view=diff
==============================================================================
--- lucene/dev/trunk/lucene/CHANGES.txt (original)
+++ lucene/dev/trunk/lucene/CHANGES.txt Mon Jan 28 11:31:42 2013
@@ -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

Modified: lucene/dev/trunk/lucene/facet/src/java/org/apache/lucene/facet/taxonomy/CategoryPath.java
URL: http://svn.apache.org/viewvc/lucene/dev/trunk/lucene/facet/src/java/org/apache/lucene/facet/taxonomy/CategoryPath.java?rev=1439350&r1=1439349&r2=1439350&view=diff
==============================================================================
--- lucene/dev/trunk/lucene/facet/src/java/org/apache/lucene/facet/taxonomy/CategoryPath.java (original)
+++ lucene/dev/trunk/lucene/facet/src/java/org/apache/lucene/facet/taxonomy/CategoryPath.java Mon Jan 28 11:31:42 2013
@@ -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 Com
   /** 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 Com
       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;
     }

Modified: lucene/dev/trunk/lucene/facet/src/test/org/apache/lucene/facet/taxonomy/TestCategoryPath.java
URL: http://svn.apache.org/viewvc/lucene/dev/trunk/lucene/facet/src/test/org/apache/lucene/facet/taxonomy/TestCategoryPath.java?rev=1439350&r1=1439349&r2=1439350&view=diff
==============================================================================
--- lucene/dev/trunk/lucene/facet/src/test/org/apache/lucene/facet/taxonomy/TestCategoryPath.java (original)
+++ lucene/dev/trunk/lucene/facet/src/test/org/apache/lucene/facet/taxonomy/TestCategoryPath.java Mon Jan 28 11:31:42 2013
@@ -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 Fa
     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/", '/'));
+    
+  }
+  
 }

Modified: lucene/dev/trunk/lucene/facet/src/test/org/apache/lucene/facet/taxonomy/writercache/cl2o/TestCompactLabelToOrdinal.java
URL: http://svn.apache.org/viewvc/lucene/dev/trunk/lucene/facet/src/test/org/apache/lucene/facet/taxonomy/writercache/cl2o/TestCompactLabelToOrdinal.java?rev=1439350&r1=1439349&r2=1439350&view=diff
==============================================================================
--- lucene/dev/trunk/lucene/facet/src/test/org/apache/lucene/facet/taxonomy/writercache/cl2o/TestCompactLabelToOrdinal.java (original)
+++ lucene/dev/trunk/lucene/facet/src/test/org/apache/lucene/facet/taxonomy/writercache/cl2o/TestCompactLabelToOrdinal.java Mon Jan 28 11:31:42 2013
@@ -56,6 +56,12 @@ public class TestCompactLabelToOrdinal e
           .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++;
       }