LUCENE-6055: PayloadAttribute.clone() should deep clone its BytesRef

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

lucene/CHANGES.txt

@@ -226,6 +226,9 @@ Bug Fixes
   the only docs in it had fields that hit non-aborting exceptions
   during indexing but also had doc values.  (Mike McCandless)
 
+* LUCENE-6055: PayloadAttribute.clone() now does a deep clone of the underlying
+  bytes. (Shai Erera)
+  
 Documentation
 
 * LUCENE-5392: Add/improve analysis package documentation to reflect

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

@@ -88,7 +88,6 @@ public class CharTermAttributeImpl extends AttributeImpl implements CharTermAttr
   @Override
   public void fillBytesRef() {
     bytes.copyChars(termBuffer, 0, termLength);
-    bytes.get();
   }
 
   @Override

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

@@ -55,7 +55,7 @@ public class PayloadAttributeImpl extends AttributeImpl implements PayloadAttrib
   public PayloadAttributeImpl clone()  {
     PayloadAttributeImpl clone = (PayloadAttributeImpl) super.clone();
     if (payload != null) {
-      clone.payload = payload.clone();
+      clone.payload = BytesRef.deepCopyOf(payload);
     }
     return clone;
   }
@@ -86,7 +86,7 @@ public class PayloadAttributeImpl extends AttributeImpl implements PayloadAttrib
   @Override
   public void copyTo(AttributeImpl target) {
     PayloadAttribute t = (PayloadAttribute) target;
-    t.setPayload((payload == null) ? null : payload.clone());
+    t.setPayload((payload == null) ? null : BytesRef.deepCopyOf(payload));
   }  
 
   

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

@@ -116,10 +116,10 @@ public abstract class AttributeImpl implements Cloneable, Attribute {
    * Attributes this implementation supports.
    */
   public abstract void copyTo(AttributeImpl target);
-    
+
   /**
-   * Shallow clone. Subclasses must override this if they 
-   * need to clone any members deeply,
+   * In most cases the clone is, and should be, deep in order to be able to
+   * properly capture the state of all attributes.
    */
   @Override
   public AttributeImpl clone() {

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

@@ -159,4 +159,21 @@ public class TestAttributeSource extends LuceneTestCase {
     assertTrue("The hashCode is identical, so the captured state was preserved.", hash1 != src1.hashCode());
     assertEquals(src2.hashCode(), src1.hashCode());
   }
+  
+  public void testClonePayloadAttribute() throws Exception {
+    // LUCENE-6055: verify that PayloadAttribute.clone() does deep cloning.
+    PayloadAttributeImpl src = new PayloadAttributeImpl(new BytesRef(new byte[] { 1, 2, 3 }));
+    
+    // test clone()
+    PayloadAttributeImpl clone = src.clone();
+    clone.getPayload().bytes[0] = 10; // modify one byte, srcBytes shouldn't change
+    assertEquals("clone() wasn't deep", 1, src.getPayload().bytes[0]);
+    
+    // test copyTo()
+    clone = new PayloadAttributeImpl();
+    src.copyTo(clone);
+    clone.getPayload().bytes[0] = 10; // modify one byte, srcBytes shouldn't change
+    assertEquals("clone() wasn't deep", 1, src.getPayload().bytes[0]);
+  }
+  
 }