LUCENE-6345: add null checking for query parameters

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

lucene/CHANGES.txt

@@ -102,6 +102,9 @@ Bug Fixes
 * LUCENE-6426: Fix FieldType's copy constructor to also copy over the numeric
   precision step. (Adrien Grand)
 
+* LUCENE-6345: Null check terms/fields in Lucene queries (Lee
+  Hinman via Mike McCandless)
+
 API Changes
 
 * LUCENE-6377: SearcherFactory#newSearcher now accepts the previous reader

lucene/core/src/java/org/apache/lucene/search/AutomatonQuery.java

@@ -18,6 +18,7 @@ package org.apache.lucene.search;
  */
 
 import java.io.IOException;
+import java.util.Objects;
 
 import org.apache.lucene.index.Term;
 import org.apache.lucene.index.Terms;

lucene/core/src/java/org/apache/lucene/search/BitsFilteredDocIdSet.java

@@ -17,6 +17,8 @@ package org.apache.lucene.search;
  * limitations under the License.
  */
 
+import java.util.Objects;
+
 import org.apache.lucene.util.Bits;
 
 /**
@@ -50,9 +52,7 @@ public final class BitsFilteredDocIdSet extends FilteredDocIdSet {
    */
   public BitsFilteredDocIdSet(DocIdSet innerSet, Bits acceptDocs) {
     super(innerSet);
-    if (acceptDocs == null)
-      throw new NullPointerException("acceptDocs is null");
-    this.acceptDocs = acceptDocs;
+    this.acceptDocs = Objects.requireNonNull(acceptDocs, "Bits must not be null");
   }
 
   @Override

lucene/core/src/java/org/apache/lucene/search/BooleanClause.java

@@ -17,6 +17,8 @@ package org.apache.lucene.search;
  * limitations under the License.
  */
 
+import java.util.Objects;
+
 /** A clause in a BooleanQuery. */
 public class BooleanClause {
   
@@ -55,8 +57,8 @@ public class BooleanClause {
   /** Constructs a BooleanClause.
   */ 
   public BooleanClause(Query query, Occur occur) {
-    this.query = query;
-    this.occur = occur;
+    this.query = Objects.requireNonNull(query, "Query must not be null");
+    this.occur = Objects.requireNonNull(occur, "Occur must not be null");
     
   }
 
@@ -65,7 +67,7 @@ public class BooleanClause {
   }
 
   public void setOccur(Occur occur) {
-    this.occur = occur;
+    this.occur = Objects.requireNonNull(occur, "Occur must not be null");
 
   }
 
@@ -74,7 +76,7 @@ public class BooleanClause {
   }
 
   public void setQuery(Query query) {
-    this.query = query;
+    this.query = Objects.requireNonNull(query, "Query must not be null");
   }
   
   public boolean isProhibited() {

lucene/core/src/java/org/apache/lucene/search/BooleanQuery.java

@@ -21,6 +21,7 @@ import java.io.IOException;
 import java.util.ArrayList;
 import java.util.Iterator;
 import java.util.List;
+import java.util.Objects;
 import java.util.Set;
 
 import org.apache.lucene.index.IndexReader;
@@ -137,6 +138,7 @@ public class BooleanQuery extends Query implements Iterable<BooleanClause> {
    * @see #getMaxClauseCount()
    */
   public void add(BooleanClause clause) {
+    Objects.requireNonNull(clause, "BooleanClause must not be null");
     if (clauses.size() >= maxClauseCount) {
       throw new TooManyClauses();
     }

lucene/core/src/java/org/apache/lucene/search/CachingWrapperQuery.java

@@ -23,6 +23,7 @@ import java.util.Collection;
 import java.util.Collections;
 import java.util.List;
 import java.util.Map;
+import java.util.Objects;
 import java.util.Set;
 import java.util.WeakHashMap;
 
@@ -50,8 +51,8 @@ public class CachingWrapperQuery extends Query implements Accountable {
    * @param policy policy defining which filters should be cached on which segments
    */
   public CachingWrapperQuery(Query query, QueryCachingPolicy policy) {
-    this.query = query;
-    this.policy = policy;
+    this.query = Objects.requireNonNull(query, "Query must not be null");
+    this.policy = Objects.requireNonNull(policy, "QueryCachingPolicy must not be null");
   }
 
   /** Same as {@link CachingWrapperQuery#CachingWrapperQuery(Query, QueryCachingPolicy)}

lucene/core/src/java/org/apache/lucene/search/ConstantScoreQuery.java

@@ -20,6 +20,7 @@ package org.apache.lucene.search;
 import java.io.IOException;
 import java.util.Collection;
 import java.util.Collections;
+import java.util.Objects;
 import java.util.Set;
 
 import org.apache.lucene.index.IndexReader;
@@ -39,7 +40,7 @@ public class ConstantScoreQuery extends Query {
   /** Strips off scores from the passed in Query. The hits will get a constant score
    * dependent on the boost factor of this query. */
   public ConstantScoreQuery(Query query) {
-    this.query = query;
+    this.query = Objects.requireNonNull(query, "Query must not be null");
   }
 
   /** Returns the encapsulated query. */

lucene/core/src/java/org/apache/lucene/search/DisjunctionMaxQuery.java

@@ -21,6 +21,7 @@ import java.util.ArrayList;
 import java.util.Collection;
 import java.util.Iterator;
 import java.util.List;
+import java.util.Objects;
 import java.util.Set;
 
 import org.apache.lucene.index.LeafReaderContext;
@@ -66,6 +67,7 @@ public class DisjunctionMaxQuery extends Query implements Iterable<Query> {
    * @param tieBreakerMultiplier   the weight to give to each matching non-maximum disjunct
    */
   public DisjunctionMaxQuery(Collection<Query> disjuncts, float tieBreakerMultiplier) {
+    Objects.requireNonNull(disjuncts, "Collection of Querys must not be null");
     this.tieBreakerMultiplier = tieBreakerMultiplier;
     add(disjuncts);
   }
@@ -74,7 +76,7 @@ public class DisjunctionMaxQuery extends Query implements Iterable<Query> {
    * @param query the disjunct added
    */
   public void add(Query query) {
-    disjuncts.add(query);
+    disjuncts.add(Objects.requireNonNull(query, "Query must not be null"));
   }
 
   /** Add a collection of disjuncts to this disjunction
@@ -82,7 +84,7 @@ public class DisjunctionMaxQuery extends Query implements Iterable<Query> {
    * @param disjuncts a collection of queries to add as disjuncts.
    */
   public void add(Collection<Query> disjuncts) {
-    this.disjuncts.addAll(disjuncts);
+    this.disjuncts.addAll(Objects.requireNonNull(disjuncts, "Query connection must not be null"));
   }
 
   /** @return An {@code Iterator<Query>} over the disjuncts */

lucene/core/src/java/org/apache/lucene/search/DocValuesTermsQuery.java

@@ -90,6 +90,7 @@ public class DocValuesTermsQuery extends Query {
 
   public DocValuesTermsQuery(String field, Collection<BytesRef> terms) {
     this.field = Objects.requireNonNull(field);
+    Objects.requireNonNull(terms, "Collection of terms must not be null");
     this.terms = terms.toArray(new BytesRef[terms.size()]);
     ArrayUtil.timSort(this.terms, BytesRef.getUTF8SortedAsUnicodeComparator());
   }

lucene/core/src/java/org/apache/lucene/search/FilteredQuery.java

@@ -21,6 +21,7 @@ import java.io.IOException;
 import java.util.Arrays;
 import java.util.Collection;
 import java.util.Collections;
+import java.util.Objects;
 import java.util.Set;
 
 import org.apache.lucene.index.IndexReader;
@@ -66,18 +67,9 @@ public class FilteredQuery extends Query {
    * @see FilterStrategy
    */
   public FilteredQuery(Query query, Filter filter, FilterStrategy strategy) {
-    if (query == null) {
-      throw new IllegalArgumentException("Query must not be be null.");
-    }
-    if (filter == null) {
-      throw new IllegalArgumentException("Filter must not be be null.");
-    }
-    if (strategy == null) {
-      throw new IllegalArgumentException("FilterStrategy must not be null");
-    }
-    this.strategy = strategy;
-    this.query = query;
-    this.filter = filter;
+    this.strategy = Objects.requireNonNull(strategy, "FilterStrategy must not be null");
+    this.query = Objects.requireNonNull(query, "Query must not be null");
+    this.filter = Objects.requireNonNull(filter, "Filter must not be null");
   }
   
   /**

lucene/core/src/java/org/apache/lucene/search/MultiPhraseQuery.java

@@ -93,6 +93,7 @@ public class MultiPhraseQuery extends Query {
    * @see PhraseQuery#add(Term, int)
    */
   public void add(Term[] terms, int position) {
+    Objects.requireNonNull(terms, "Term array must not be null");
     if (termArrays.size() == 0)
       field = terms[0].field();
 

lucene/core/src/java/org/apache/lucene/search/MultiTermQuery.java

@@ -18,6 +18,7 @@ package org.apache.lucene.search;
  */
 
 import java.io.IOException;
+import java.util.Objects;
 
 import org.apache.lucene.index.FilteredTermsEnum; // javadocs
 import org.apache.lucene.index.IndexReader;
@@ -214,10 +215,7 @@ public abstract class MultiTermQuery extends Query {
    * Term.
    */
   public MultiTermQuery(final String field) {
-    if (field == null) {
-      throw new IllegalArgumentException("field must not be null");
-    }
-    this.field = field;
+    this.field = Objects.requireNonNull(field, "field must not be null");
   }
 
   /** Returns the field name for this query */

lucene/core/src/java/org/apache/lucene/search/NumericRangeQuery.java

@@ -19,6 +19,7 @@ package org.apache.lucene.search;
 
 import java.io.IOException;
 import java.util.LinkedList;
+import java.util.Objects;
 
 import org.apache.lucene.analysis.NumericTokenStream; // for javadocs
 import org.apache.lucene.document.DoubleField; // for javadocs
@@ -172,7 +173,7 @@ public final class NumericRangeQuery<T extends Number> extends MultiTermQuery {
     if (precisionStep < 1)
       throw new IllegalArgumentException("precisionStep must be >=1");
     this.precisionStep = precisionStep;
-    this.dataType = dataType;
+    this.dataType = Objects.requireNonNull(dataType, "NumericType must not be null");
     this.min = min;
     this.max = max;
     this.minInclusive = minInclusive;

lucene/core/src/java/org/apache/lucene/search/PhraseQuery.java

@@ -20,6 +20,7 @@ package org.apache.lucene.search;
 import java.io.IOException;
 import java.util.ArrayList;
 import java.util.Arrays;
+import java.util.Objects;
 import java.util.Set;
 
 import org.apache.lucene.index.PostingsEnum;
@@ -110,6 +111,7 @@ public class PhraseQuery extends Query {
    * 
    */
   public void add(Term term, int position) {
+    Objects.requireNonNull(term, "Term must not be null");
     if (positions.size() > 0) {
       final int previousPosition = positions.get(positions.size()-1);
       if (position < previousPosition) {

lucene/core/src/java/org/apache/lucene/util/IOUtils.java

@@ -481,7 +481,8 @@ public final class IOUtils {
     }
     
     // get block device name
-    String devName = getBlockDevice(store);
+    String devName = store.name();
+
     // not a device (e.g. NFS server)
     if (!devName.startsWith("/")) {
       return true;
@@ -519,28 +520,38 @@ public final class IOUtils {
     FileStore store = Files.getFileStore(path);
     String mount = getMountPoint(store);
 
-    // find the "matching" FileStore from system list, it's the one we want.
+    // find the "matching" FileStore from system list, it's the one we want, but only return
+    // that if it's unambiguous (only one matching):
+    FileStore sameMountPoint = null;
     for (FileStore fs : path.getFileSystem().getFileStores()) {
       if (mount.equals(getMountPoint(fs))) {
-        return fs;
+        if (sameMountPoint == null) {
+          sameMountPoint = fs;
+        } else {
+          // more than one filesystem has the same mount point; something is wrong!
+          // fall back to crappy one we got from Files.getFileStore
+          return store;
+        }
       }
     }
 
-    // fall back to crappy one we got from Files.getFileStore
-    return store;    
+    if (sameMountPoint != null) {
+      // ok, we found only one, use it:
+      return sameMountPoint;
+    } else {
+      // fall back to crappy one we got from Files.getFileStore
+      return store;    
+    }
   }
   
-  // these are hacks that are not guaranteed
+  // these are hacks that are not guaranteed, may change across JVM versions, etc.
   static String getMountPoint(FileStore store) {
     String desc = store.toString();
-    return desc.substring(0, desc.lastIndexOf('(') - 1);
-  }
-  
-  // these are hacks that are not guaranteed
-  static String getBlockDevice(FileStore store) {
-    String desc = store.toString();
-    int start = desc.lastIndexOf('(');
-    int end = desc.indexOf(')', start);
-    return desc.substring(start+1, end);
+    int index = desc.lastIndexOf(" (");
+    if (index != -1) {
+      return desc.substring(0, index);
+    } else {
+      return desc;
+    }
   }
 }

lucene/core/src/test/org/apache/lucene/search/TestFilteredQuery.java

@@ -362,20 +362,20 @@ public class TestFilteredQuery extends LuceneTestCase {
   public void testInvalidArguments() throws Exception {
     try {
       new FilteredQuery(null, null);
-      fail("Should throw IllegalArgumentException");
-    } catch (IllegalArgumentException iae) {
+      fail("Should throw NullPointerException");
+    } catch (NullPointerException npe) {
       // pass
     }
     try {
       new FilteredQuery(new TermQuery(new Term("field", "one")), null);
-      fail("Should throw IllegalArgumentException");
-    } catch (IllegalArgumentException iae) {
+      fail("Should throw NullPointerException");
+    } catch (NullPointerException npe) {
       // pass
     }
     try {
       new FilteredQuery(null, new QueryWrapperFilter(new PrefixQuery(new Term("field", "o"))));
-      fail("Should throw IllegalArgumentException");
-    } catch (IllegalArgumentException iae) {
+      fail("Should throw NullPointerException");
+    } catch (NullPointerException npe) {
       // pass
     }
   }

lucene/core/src/test/org/apache/lucene/util/TestIOUtils.java

@@ -25,13 +25,14 @@ import java.nio.file.FileSystem;
 import java.nio.file.Files;
 import java.nio.file.LinkOption;
 import java.nio.file.Path;
+import java.nio.file.attribute.FileAttributeView;
+import java.nio.file.attribute.FileStoreAttributeView;
 import java.util.Collection;
 import java.util.Collections;
 import java.util.HashMap;
 import java.util.HashSet;
 import java.util.Map;
 
-import org.apache.lucene.mockfile.FilterFileStore;
 import org.apache.lucene.mockfile.FilterFileSystem;
 import org.apache.lucene.mockfile.FilterFileSystemProvider;
 import org.apache.lucene.mockfile.FilterPath;
@@ -115,18 +116,15 @@ public class TestIOUtils extends LuceneTestCase {
   }
   
   // fake up a filestore to test some underlying methods
-  static class MockFileStore extends FilterFileStore {
+  static class MockFileStore extends FileStore {
     final String description;
     final String type;
+    final String name;
     
-    MockFileStore(FileStore delegate, String description) {
-      this(delegate, description, "mockfs");
-    }
-    
-    MockFileStore(FileStore delegate, String description, String type) {
-      super(delegate, "justafake://");
+    MockFileStore(String description, String type, String name) {
       this.description = description;
       this.type = type;
+      this.name = name;
     }
 
     @Override
@@ -134,28 +132,64 @@ public class TestIOUtils extends LuceneTestCase {
       return type;
     }
 
+    @Override
+    public String name() {
+      return name;
+    }
+
     @Override
     public String toString() {
       return description;
     }
-  }
-  
-  public void testGetBlockDevice() throws Exception {
-    Path dir = createTempDir();
-    FileStore actual = Files.getFileStore(dir);
 
-    assertEquals("/dev/sda1", IOUtils.getBlockDevice(new MockFileStore(actual, "/ (/dev/sda1)")));
-    assertEquals("/dev/sda1", IOUtils.getBlockDevice(new MockFileStore(actual, "/test/ space(((trash)))/ (/dev/sda1)")));
-    assertEquals("notreal", IOUtils.getBlockDevice(new MockFileStore(actual, "/ (notreal)")));
+
+    // TODO: we can enable mocking of these when we need them later:
+
+    @Override
+    public boolean isReadOnly() {
+      return false;
+    }
+
+    @Override
+    public long getTotalSpace() throws IOException {
+      return 1000;
+    }
+
+    @Override
+    public long getUsableSpace() throws IOException {
+      return 800;
+    }
+
+    @Override
+    public long getUnallocatedSpace() throws IOException {
+      return 1000;
+    }
+
+    @Override
+    public boolean supportsFileAttributeView(Class<? extends FileAttributeView> type) {
+      return false;
+    }
+
+    @Override
+    public boolean supportsFileAttributeView(String name) {
+      return false;
+    }
+
+    @Override
+    public <V extends FileStoreAttributeView> V getFileStoreAttributeView(Class<V> type) {
+      return null;
+    }
+
+    @Override
+    public Object getAttribute(String attribute) throws IOException {
+      return null;
+    }
   }
   
   public void testGetMountPoint() throws Exception {
-    Path dir = createTempDir();
-    FileStore actual = Files.getFileStore(dir);
-
-    assertEquals("/", IOUtils.getMountPoint(new MockFileStore(actual, "/ (/dev/sda1)")));
-    assertEquals("/test/ space(((trash)))/", IOUtils.getMountPoint(new MockFileStore(actual, "/test/ space(((trash)))/ (/dev/sda1)")));
-    assertEquals("/", IOUtils.getMountPoint(new MockFileStore(actual, "/ (notreal)")));
+    assertEquals("/", IOUtils.getMountPoint(new MockFileStore("/ (/dev/sda1)", "ext4", "/dev/sda1")));
+    assertEquals("/test/ space(((trash)))/", IOUtils.getMountPoint(new MockFileStore("/test/ space(((trash)))/ (/dev/sda1)", "ext3", "/dev/sda1")));
+    assertEquals("/", IOUtils.getMountPoint(new MockFileStore("/ (notreal)", "ext2", "notreal")));
   }
   
   /** mock linux that takes mappings of test files, to their associated filesystems.
@@ -193,7 +227,7 @@ public class TestIOUtils extends LuceneTestCase {
       }
       // act like the linux fs provider here, return a crazy rootfs one
       if (ret.toString().startsWith(root + " (")) {
-        return new MockFileStore(ret, root + " (rootfs)", "rootfs");
+        return new MockFileStore(root + " (rootfs)", "rootfs", "rootfs");
       }
 
       return ret;
@@ -240,8 +274,8 @@ public class TestIOUtils extends LuceneTestCase {
     dir = FilterPath.unwrap(dir).toRealPath();
     
     // now we can create some fake mount points:
-    FileStore root = new MockFileStore(Files.getFileStore(dir), dir.toString() + " (/dev/sda1)");
-    FileStore usr = new MockFileStore(Files.getFileStore(dir), dir.resolve("usr").toString() + " (/dev/sda2)");
+    FileStore root = new MockFileStore(dir.toString() + " (/dev/sda1)", "ntfs", "/dev/sda1");
+    FileStore usr = new MockFileStore(dir.resolve("usr").toString() + " (/dev/sda2)", "xfs", "/dev/sda2");
 
     // associate some preset files to these
     Map<String,FileStore> mappings = new HashMap<>();
@@ -274,7 +308,7 @@ public class TestIOUtils extends LuceneTestCase {
     dir = FilterPath.unwrap(dir).toRealPath();
     
     // fake tmpfs
-    FileStore root = new MockFileStore(Files.getFileStore(dir), dir.toString() + " (/dev/sda1)", "tmpfs");
+    FileStore root = new MockFileStore(dir.toString() + " (/dev/sda1)", "tmpfs", "/dev/sda1");
     Map<String,FileStore> mappings = Collections.singletonMap(dir.toString(), root);
     FileSystem mockLinux = new MockLinuxFileSystemProvider(dir.getFileSystem(), mappings, dir).getFileSystem(null);
     
@@ -287,7 +321,7 @@ public class TestIOUtils extends LuceneTestCase {
     dir = FilterPath.unwrap(dir).toRealPath();
     
     // fake nfs
-    FileStore root = new MockFileStore(Files.getFileStore(dir), dir.toString() + " (somenfsserver:/some/mount)", "nfs");
+    FileStore root = new MockFileStore(dir.toString() + " (somenfsserver:/some/mount)", "nfs", "somenfsserver:/some/mount");
     Map<String,FileStore> mappings = Collections.singletonMap(dir.toString(), root);
     FileSystem mockLinux = new MockLinuxFileSystemProvider(dir.getFileSystem(), mappings, dir).getFileSystem(null);
     
@@ -301,7 +335,7 @@ public class TestIOUtils extends LuceneTestCase {
     dir = FilterPath.unwrap(dir).toRealPath();
     
     // fake ssd
-    FileStore root = new MockFileStore(Files.getFileStore(dir), dir.toString() + " (/dev/zzz1)");
+    FileStore root = new MockFileStore(dir.toString() + " (/dev/zzz1)", "btrfs", "/dev/zzz1");
     // make a fake /dev/zzz1 for it
     Path devdir = dir.resolve("dev");
     Files.createDirectories(devdir);
@@ -325,7 +359,7 @@ public class TestIOUtils extends LuceneTestCase {
     dir = FilterPath.unwrap(dir).toRealPath();
     
     // fake ssd
-    FileStore root = new MockFileStore(Files.getFileStore(dir), dir.toString() + " (/dev/zzz1)");
+    FileStore root = new MockFileStore(dir.toString() + " (/dev/zzz1)", "reiser4", "/dev/zzz1");
     // make a fake /dev/zzz1 for it
     Path devdir = dir.resolve("dev");
     Files.createDirectories(devdir);
@@ -349,7 +383,7 @@ public class TestIOUtils extends LuceneTestCase {
     dir = FilterPath.unwrap(dir).toRealPath();
     
     // fake ssd
-    FileStore root = new MockFileStore(Files.getFileStore(dir), dir.toString() + " (/dev/zzz12)");
+    FileStore root = new MockFileStore(dir.toString() + " (/dev/zzz12)", "zfs", "/dev/zzz12");
     // make a fake /dev/zzz11 for it
     Path devdir = dir.resolve("dev");
     Files.createDirectories(devdir);