[geode] branch develop updated: GEODE-6692: canonicalize deserialization of empty string (#3513)

[ds...@apache.org]

This is an automated email from the ASF dual-hosted git repository.

dschneider pushed a commit to branch develop
in repository https://gitbox.apache.org/repos/asf/geode.git


The following commit(s) were added to refs/heads/develop by this push:
     new 59c76ee  GEODE-6692: canonicalize deserialization of empty string (#3513)
59c76ee is described below

commit 59c76ee235ef5ab3681cfd1d5b3c4884c3aac370
Author: Darrel Schneider <ds...@pivotal.io>
AuthorDate: Tue Apr 30 10:02:41 2019 -0700

    GEODE-6692: canonicalize deserialization of empty string (#3513)
    
    * added unit test to confirm that DataSerializer.readString
    canonicalizes the empty string.
    
    * deserialization of an empty string no longer allocates a new String instance
---
 .../apache/geode/internal/InternalDataSerializer.java  |  3 +++
 .../test/java/org/apache/geode/DataSerializerTest.java | 18 ++++++++++++++++++
 2 files changed, 21 insertions(+)

diff --git a/geode-core/src/main/java/org/apache/geode/internal/InternalDataSerializer.java b/geode-core/src/main/java/org/apache/geode/internal/InternalDataSerializer.java
index ce98894..4be75b4 100644
--- a/geode-core/src/main/java/org/apache/geode/internal/InternalDataSerializer.java
+++ b/geode-core/src/main/java/org/apache/geode/internal/InternalDataSerializer.java
@@ -2602,6 +2602,9 @@ public abstract class InternalDataSerializer extends DataSerializer {
     if (logger.isTraceEnabled(LogMarker.SERIALIZER_VERBOSE)) {
       logger.trace(LogMarker.SERIALIZER_VERBOSE, "Reading STRING_BYTES of len={}", len);
     }
+    if (len == 0) {
+      return "";
+    }
     byte[] buf = threadLocalByteArrayCache.get(len);
     dataInput.readFully(buf, 0, len);
     return new String(buf, 0, 0, len); // intentionally using deprecated constructor
diff --git a/geode-core/src/test/java/org/apache/geode/DataSerializerTest.java b/geode-core/src/test/java/org/apache/geode/DataSerializerTest.java
index bab9d10..eaa085d 100644
--- a/geode-core/src/test/java/org/apache/geode/DataSerializerTest.java
+++ b/geode-core/src/test/java/org/apache/geode/DataSerializerTest.java
@@ -20,11 +20,15 @@ import static org.mockito.Mockito.times;
 import static org.mockito.Mockito.verify;
 import static org.mockito.Mockito.when;
 
+import java.io.IOException;
+
 import org.junit.Test;
 import org.junit.experimental.categories.Category;
 
+import org.apache.geode.internal.ByteArrayDataInput;
 import org.apache.geode.internal.cache.EventID;
 import org.apache.geode.internal.cache.tier.sockets.ClientProxyMembershipID;
+import org.apache.geode.internal.util.BlobHelper;
 import org.apache.geode.test.junit.categories.SerializationTest;
 
 @Category({SerializationTest.class})
@@ -48,4 +52,18 @@ public class DataSerializerTest {
     assertThat(mockDataSerializer.getEventId()).isSameAs(mockEventID);
     assertThat(mockDataSerializer.getContext()).isSameAs(mockClientProxyMembershipID);
   }
+
+  @Test
+  public void readStringShouldReturnCanonicalEmptyString() throws IOException {
+    byte[] serializedEmptyStringBytes = BlobHelper.serializeToBlob("");
+    ByteArrayDataInput dataInput1 = new ByteArrayDataInput();
+    dataInput1.initialize(serializedEmptyStringBytes, null);
+    ByteArrayDataInput dataInput2 = new ByteArrayDataInput();
+    dataInput2.initialize(serializedEmptyStringBytes, null);
+
+    String firstRead = DataSerializer.readString(dataInput1);
+    String secondRead = DataSerializer.readString(dataInput2);
+
+    assertThat(secondRead).isSameAs(firstRead);
+  }
 }