[geode-native] branch develop updated: GEODE-5962: Fix putAll crash with null values (#408)

[bb...@apache.org]

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

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


The following commit(s) were added to refs/heads/develop by this push:
     new 8be03f1  GEODE-5962: Fix putAll crash with null values (#408)
8be03f1 is described below

commit 8be03f13b37d17c7b59e33d255b8144ead5db45f
Author: Michael Martell <mm...@pivotal.io>
AuthorDate: Fri Jan 11 13:08:05 2019 -0800

    GEODE-5962: Fix putAll crash with null values (#408)
    
    - Add nullValue test
    - Update the test to catch the cache server exception.
    - Add TcrMessage:: scope to PUT_DATA_ERROR
    - Replace try/catch in test with EXPECT_THROW
    
    Co-authored-by: Mike Martell <mm...@pivotal.io>
    Co-authored-by: Blake Bender <bb...@pivotal.io>
    Co-authored-by: Michael Oleske <mo...@pivotal.io>
---
 cppcache/integration/test/RegionPutGetAllTest.cpp | 26 +++++++++++++++++++++++
 cppcache/src/TcrMessage.cpp                       | 20 ++++++++++++++++-
 2 files changed, 45 insertions(+), 1 deletion(-)

diff --git a/cppcache/integration/test/RegionPutGetAllTest.cpp b/cppcache/integration/test/RegionPutGetAllTest.cpp
index 16ca0e7..910566d 100644
--- a/cppcache/integration/test/RegionPutGetAllTest.cpp
+++ b/cppcache/integration/test/RegionPutGetAllTest.cpp
@@ -37,6 +37,7 @@ namespace {
 
 using apache::geode::client::Cache;
 using apache::geode::client::CacheableInt32;
+using apache::geode::client::CacheServerException;
 using apache::geode::client::HashMapOfCacheable;
 using apache::geode::client::Region;
 using apache::geode::client::RegionShortcut;
@@ -198,4 +199,29 @@ TEST(RegionPutGetAllTest, variousPdxTypes) {
   assert_eq(putAllMap, getAllMap);
 }
 
+TEST(RegionPutGetAllTest, nullValue) {
+  Cluster cluster{LocatorCount{1}, ServerCount{2}};
+  cluster.getGfsh()
+      .create()
+      .region()
+      .withName("region")
+      .withType("REPLICATE")
+      .execute();
+
+  auto cache = cluster.createCache();
+  auto region = setupRegion(cache);
+
+  setupPdxTypes(cache);
+
+  HashMapOfCacheable map;
+
+  // Add a null value
+  map.emplace(CacheableInt32::create(PdxTypesHelper1::index),
+              std::shared_ptr<PdxTypesHelper1::type>());
+
+  auto keys = to_keys(map);
+
+  ASSERT_THROW(region->putAll(map), CacheServerException);
+}
+
 }  // namespace
diff --git a/cppcache/src/TcrMessage.cpp b/cppcache/src/TcrMessage.cpp
index 81f69d8..eb9ba29 100644
--- a/cppcache/src/TcrMessage.cpp
+++ b/cppcache/src/TcrMessage.cpp
@@ -54,6 +54,8 @@ namespace {
 uint32_t g_headerLen = 17;
 }  // namespace
 
+extern void setThreadLocalExceptionMessage(const char*);
+
 // AtomicInc TcrMessage::m_transactionId = 0;
 uint8_t* TcrMessage::m_keepalive = nullptr;
 const int TcrMessage::m_flag_empty = 0x01;
@@ -839,12 +841,28 @@ void TcrMessage::processChunk(const uint8_t* bytes, int32_t len,
       }
       break;
     }
+    case TcrMessage::PUT_DATA_ERROR: {
+      chunkSecurityHeader(1, bytes, len, isLastChunkAndisSecurityHeader);
+      if (nullptr != bytes) {
+        auto input =
+            m_tcdm->getConnectionManager().getCacheImpl()->createDataInput(
+                bytes, len);
+        auto errorString = readStringPart(input);
+
+        if (!errorString.empty()) {
+          setThreadLocalExceptionMessage(errorString.c_str());
+        }
+
+        _GEODE_SAFE_DELETE_ARRAY(bytes);
+      }
+      break;
+    }
     case TcrMessage::GET_ALL_DATA_ERROR: {
       chunkSecurityHeader(1, bytes, len, isLastChunkAndisSecurityHeader);
       if (bytes != nullptr) {
         _GEODE_SAFE_DELETE_ARRAY(bytes);
       }
-      // nothing else to done since this will be taken care of at higher level
+
       break;
     }
     default: {