LUCENE-6976 SOLR-8541: BytesTermAttributeImpl.copyTo could NPE.

Could be triggered by trying to highlight a spatial RPT field.

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

lucene/CHANGES.txt

@@ -180,6 +180,11 @@ Optimizations
 * LUCENE-6940: MUST_NOT clauses execute faster, especially when they are sparse.
   (Adrien Grand)
 
+Bug Fixes
+
+* LUCENE-6976: BytesRefTermAttributeImpl.copyTo NPE'ed if BytesRef was null.
+  Added equals & hashCode, and a new test for these things. (David Smiley)
+
 Other
 
 * LUCENE-6924: Upgrade randomizedtesting to 2.3.2. (Dawid Weiss)

lucene/core/src/java/org/apache/lucene/analysis/tokenattributes/BytesTermAttributeImpl.java

@@ -17,6 +17,8 @@ package org.apache.lucene.analysis.tokenattributes;
  * limitations under the License.
  */
 
+import java.util.Objects;
+
 import org.apache.lucene.util.AttributeImpl;
 import org.apache.lucene.util.AttributeReflector;
 import org.apache.lucene.util.BytesRef;
@@ -48,7 +50,7 @@ public class BytesTermAttributeImpl extends AttributeImpl implements BytesTermAt
   @Override
   public void copyTo(AttributeImpl target) {
     BytesTermAttributeImpl other = (BytesTermAttributeImpl) target;
-    other.bytes = BytesRef.deepCopyOf(bytes);
+    other.bytes = bytes == null ? null : BytesRef.deepCopyOf(bytes);
   }
 
   @Override
@@ -62,4 +64,17 @@ public class BytesTermAttributeImpl extends AttributeImpl implements BytesTermAt
   public void reflectWith(AttributeReflector reflector) {
     reflector.reflect(TermToBytesRefAttribute.class, "bytes", bytes);
   }
+
+  @Override
+  public boolean equals(Object o) {
+    if (this == o) return true;
+    if (!(o instanceof BytesTermAttributeImpl)) return false;
+    BytesTermAttributeImpl that = (BytesTermAttributeImpl) o;
+    return Objects.equals(bytes, that.bytes);
+  }
+
+  @Override
+  public int hashCode() {
+    return Objects.hash(bytes);
+  }
 }

lucene/core/src/test/org/apache/lucene/analysis/tokenattributes/TestBytesRefAttImpl.java

@@ -0,0 +1,48 @@
+package org.apache.lucene.analysis.tokenattributes;
+
+/*
+ * 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.
+ */
+
+import org.apache.lucene.util.AttributeImpl;
+import org.apache.lucene.util.BytesRef;
+import org.apache.lucene.util.LuceneTestCase;
+
+public class TestBytesRefAttImpl extends LuceneTestCase {
+
+  public void testCopyTo() throws Exception {
+    BytesTermAttributeImpl t = new BytesTermAttributeImpl();
+    BytesTermAttributeImpl copy = assertCopyIsEqual(t);
+
+    // first do empty
+    assertEquals(t.getBytesRef(), copy.getBytesRef());
+    assertNull(copy.getBytesRef());
+    // now after setting it
+    t.setBytesRef(new BytesRef("hello"));
+    copy = assertCopyIsEqual(t);
+    assertEquals(t.getBytesRef(), copy.getBytesRef());
+    assertNotSame(t.getBytesRef(), copy.getBytesRef());
+  }
+
+  public static <T extends AttributeImpl> T assertCopyIsEqual(T att) throws Exception {
+    @SuppressWarnings("unchecked")
+    T copy = (T) att.getClass().newInstance();
+    att.copyTo(copy);
+    assertEquals("Copied instance must be equal", att, copy);
+    assertEquals("Copied instance's hashcode must be equal", att.hashCode(), copy.hashCode());
+    return copy;
+  }
+}

solr/CHANGES.txt

@@ -399,6 +399,9 @@ Bug Fixes
 * SOLR-2798: Fixed local params to work correctly with multivalued params
   (Demian Katz via hossman)
 
+* SOLR-8541: Highlighting a geo RPT field would throw an NPE instead of doing nothing.
+  (Pawel Rog via David Smiley)
+
 Optimizations
 ----------------------
 

solr/core/src/test/org/apache/solr/search/TestSolr4Spatial2.java

@@ -17,9 +17,13 @@ package org.apache.solr.search;
  * limitations under the License.
  */
 
+import org.apache.lucene.analysis.CachingTokenFilter;
+import org.apache.lucene.analysis.TokenStream;
+import org.apache.lucene.index.memory.MemoryIndex;
 import org.apache.solr.SolrTestCaseJ4;
 import org.apache.solr.common.SolrException;
 import org.apache.solr.common.params.FacetParams;
+import org.apache.solr.common.params.ModifiableSolrParams;
 import org.apache.solr.request.SolrQueryRequest;
 import org.junit.Before;
 import org.junit.BeforeClass;
@@ -160,4 +164,17 @@ public class TestSolr4Spatial2 extends SolrTestCaseJ4 {
     return getSearcher().getRawReader().leaves().get(0).reader().getCoreCacheKey();
   }
 
+  @Test// SOLR-8541
+  public void testConstantScoreQueryWithFilterPartOnly() {
+    final String[] doc1 = {"id", "1", "srptgeom", "56.9485,24.0980"};
+    assertU(adoc(doc1));
+    assertU(commit());
+
+    ModifiableSolrParams params = new ModifiableSolrParams();
+    params.add("q", "{!geofilt sfield=\"srptgeom\" pt=\"56.9484,24.0981\" d=100}");
+    params.add("hl", "true");
+    params.add("hl.fl", "srptgeom");
+    assertQ(req(params), "*[count(//doc)=1]", "count(//lst[@name='highlighting']/*)=1");
+  }
+
 }