From e12039a377c4639f30aad8b31fb39964754d6084 Mon Sep 17 00:00:00 2001 From: Robert Muir Date: Fri, 9 May 2014 16:36:25 +0000 Subject: [PATCH] LUCENE-5662: add missing null checks to Field to avoid aborting exceptions git-svn-id: https://svn.apache.org/repos/asf/lucene/dev/trunk@1593577 13f79535-47bb-0310-9956-ffa450edef68 --- .../org/apache/lucene/document/Field.java | 9 ++ .../index/TestIndexWriterExceptions.java | 144 ++++++++++++++++++ 2 files changed, 153 insertions(+) diff --git a/lucene/core/src/java/org/apache/lucene/document/Field.java b/lucene/core/src/java/org/apache/lucene/document/Field.java index 7140a677a2c..1fd440cacf0 100644 --- a/lucene/core/src/java/org/apache/lucene/document/Field.java +++ b/lucene/core/src/java/org/apache/lucene/document/Field.java @@ -211,6 +211,9 @@ public class Field implements IndexableField, StorableField { if (name == null) { throw new IllegalArgumentException("name cannot be null"); } + if (bytes == null) { + throw new IllegalArgumentException("bytes cannot be null"); + } if (type.indexed()) { throw new IllegalArgumentException("Fields with BytesRef values cannot be indexed"); } @@ -299,6 +302,9 @@ public class Field implements IndexableField, StorableField { if (!(fieldsData instanceof String)) { throw new IllegalArgumentException("cannot change value type from " + fieldsData.getClass().getSimpleName() + " to String"); } + if (value == null) { + throw new IllegalArgumentException("value cannot be null"); + } fieldsData = value; } @@ -335,6 +341,9 @@ public class Field implements IndexableField, StorableField { if (type.indexed()) { throw new IllegalArgumentException("cannot set a BytesRef value on an indexed field"); } + if (value == null) { + throw new IllegalArgumentException("value cannot be null"); + } fieldsData = value; } diff --git a/lucene/core/src/test/org/apache/lucene/index/TestIndexWriterExceptions.java b/lucene/core/src/test/org/apache/lucene/index/TestIndexWriterExceptions.java index 9e35b090e56..310219047c0 100644 --- a/lucene/core/src/test/org/apache/lucene/index/TestIndexWriterExceptions.java +++ b/lucene/core/src/test/org/apache/lucene/index/TestIndexWriterExceptions.java @@ -42,6 +42,7 @@ import org.apache.lucene.document.FieldType; import org.apache.lucene.document.NumericDocValuesField; import org.apache.lucene.document.SortedDocValuesField; import org.apache.lucene.document.SortedSetDocValuesField; +import org.apache.lucene.document.StoredField; import org.apache.lucene.document.StringField; import org.apache.lucene.document.TextField; import org.apache.lucene.index.IndexWriterConfig.OpenMode; @@ -1488,6 +1489,149 @@ public class TestIndexWriterExceptions extends LuceneTestCase { dir.close(); } + /** test a null string value doesn't abort the entire segment */ + public void testNullStoredField() throws Exception { + Directory dir = newDirectory(); + Analyzer analyzer = new MockAnalyzer(random()); + IndexWriter iw = new IndexWriter(dir, new IndexWriterConfig(TEST_VERSION_CURRENT, analyzer)); + // add good document + Document doc = new Document(); + iw.addDocument(doc); + try { + // set to null value + String value = null; + doc.add(new StoredField("foo", value)); + iw.addDocument(doc); + } catch (IllegalArgumentException expected) {} + iw.shutdown(); + // make sure we see our good doc + DirectoryReader r = DirectoryReader.open(dir); + assertEquals(1, r.numDocs()); + r.close(); + dir.close(); + } + + /** test a null string value doesn't abort the entire segment */ + public void testNullStoredFieldReuse() throws Exception { + Directory dir = newDirectory(); + Analyzer analyzer = new MockAnalyzer(random()); + IndexWriter iw = new IndexWriter(dir, new IndexWriterConfig(TEST_VERSION_CURRENT, analyzer)); + // add good document + Document doc = new Document(); + Field theField = new StoredField("foo", "hello", StoredField.TYPE); + doc.add(theField); + iw.addDocument(doc); + try { + // set to null value + theField.setStringValue(null); + iw.addDocument(doc); + } catch (IllegalArgumentException expected) {} + iw.shutdown(); + // make sure we see our good doc + DirectoryReader r = DirectoryReader.open(dir); + assertEquals(1, r.numDocs()); + r.close(); + dir.close(); + } + + /** test a null byte[] value doesn't abort the entire segment */ + public void testNullStoredBytesField() throws Exception { + Directory dir = newDirectory(); + Analyzer analyzer = new MockAnalyzer(random()); + IndexWriter iw = new IndexWriter(dir, new IndexWriterConfig(TEST_VERSION_CURRENT, analyzer)); + // add good document + Document doc = new Document(); + iw.addDocument(doc); + + try { + // set to null value + byte v[] = null; + Field theField = new StoredField("foo", v); + doc.add(theField); + iw.addDocument(doc); + } catch (NullPointerException expected) {} + iw.shutdown(); + // make sure we see our good doc + DirectoryReader r = DirectoryReader.open(dir); + assertEquals(1, r.numDocs()); + r.close(); + dir.close(); + } + + /** test a null byte[] value doesn't abort the entire segment */ + public void testNullStoredBytesFieldReuse() throws Exception { + Directory dir = newDirectory(); + Analyzer analyzer = new MockAnalyzer(random()); + IndexWriter iw = new IndexWriter(dir, new IndexWriterConfig(TEST_VERSION_CURRENT, analyzer)); + // add good document + Document doc = new Document(); + Field theField = new StoredField("foo", new BytesRef("hello").bytes); + doc.add(theField); + iw.addDocument(doc); + try { + // set to null value + byte v[] = null; + theField.setBytesValue(v); + iw.addDocument(doc); + } catch (NullPointerException expected) {} + iw.shutdown(); + // make sure we see our good doc + DirectoryReader r = DirectoryReader.open(dir); + assertEquals(1, r.numDocs()); + r.close(); + dir.close(); + } + + /** test a null bytesref value doesn't abort the entire segment */ + public void testNullStoredBytesRefField() throws Exception { + Directory dir = newDirectory(); + Analyzer analyzer = new MockAnalyzer(random()); + IndexWriter iw = new IndexWriter(dir, new IndexWriterConfig(TEST_VERSION_CURRENT, analyzer)); + // add good document + Document doc = new Document(); + iw.addDocument(doc); + + try { + // set to null value + BytesRef v = null; + Field theField = new StoredField("foo", v); + doc.add(theField); + iw.addDocument(doc); + } catch (IllegalArgumentException expected) {} + iw.shutdown(); + // make sure we see our good doc + DirectoryReader r = DirectoryReader.open(dir); + assertEquals(1, r.numDocs()); + r.close(); + dir.close(); + } + + /** test a null bytesref value doesn't abort the entire segment */ + public void testNullStoredBytesRefFieldReuse() throws Exception { + Directory dir = newDirectory(); + Analyzer analyzer = new MockAnalyzer(random()); + IndexWriter iw = new IndexWriter(dir, new IndexWriterConfig(TEST_VERSION_CURRENT, analyzer)); + // add good document + Document doc = new Document(); + Field theField = new StoredField("foo", new BytesRef("hello")); + doc.add(theField); + iw.addDocument(doc); + try { + // set to null value + BytesRef v = null; + theField.setBytesValue(v); + iw.addDocument(doc); + } catch (IllegalArgumentException expected) {} + iw.shutdown(); + // make sure we see our good doc + DirectoryReader r = DirectoryReader.open(dir); + assertEquals(1, r.numDocs()); + r.close(); + dir.close(); + } + + // TODO: we could also check isValid, to catch "broken" bytesref values, might be too much? + static class UOEDirectory extends RAMDirectory { boolean doFail = false;