eliminate session_create leaks and strengthen regression coverage

Author: pavankumar464

source/jst_session.c

@@ -164,6 +164,12 @@ static duk_ret_t session_create(duk_context *ctx)
   char* session_id = NULL;
 
   session_id = (char*)malloc(SESSION_ID_BYTES_LENGTH+1);
+  if(!session_id)
+  {
+    CosaPhpExtLog("Failed to allocate session_id!\n");
+    RETURN_FALSE;
+  }
+
   n = syscall(SYS_getrandom, bytes, SESSION_ID_BYTES_LENGTH, 0);
   if(n != SESSION_ID_BYTES_LENGTH)
   {
@@ -177,16 +183,24 @@ static duk_ret_t session_create(duk_context *ctx)
     session_id[i] = BYTE_TO_PRINTABLE_HEX_CODE(bytes[i]);
   }
 
+  if(session_identifier)
+  {
+    free(session_identifier);
+    session_identifier = NULL;
+  }
+
   session_identifier = (char*)malloc(SESSION_ID_LENGTH+1);
   if(!session_identifier)
   {
     CosaPhpExtLog("Failed to allocate session_identifier!\n");
+    free(session_id);
     RETURN_FALSE;
   }
   memset(session_identifier, 0, SESSION_ID_LENGTH+1);
 
   session_id[SESSION_ID_BYTES_LENGTH] = '\0';
   snprintf(session_identifier, SESSION_ID_LENGTH+1, "%s%s", SESSION_PREFIX, session_id);
+  free(session_id);
 
   RETURN_TRUE;
   return 1;

tests/CMakeLists.txt

@@ -73,6 +73,7 @@ add_executable(
   parser_test
   ../tests/parser_test.cpp 
   ../source/jst_parser.c 
+  ../source/jst_session.c
   ../source/jst_internal.c
   ../source/duktape/duktape.c)
 target_link_libraries(parser_test libgtest libgmock -pthread)

tests/parser_test.cpp

@@ -26,6 +26,10 @@
 #include <sys/stat.h>
 #include <unistd.h>
 
+extern "C" {
+  duk_ret_t ccsp_session_module_open(duk_context *ctx);
+}
+
 using namespace std;
 
 class BufferFreer
@@ -106,6 +110,81 @@ TEST(general, parser) {
   }
 }
 
+TEST(general, session_create_multiple_calls_succeed)
+{
+  duk_context* ctx = duk_create_heap_default();
+  ASSERT_NE(ctx, nullptr);
+
+  duk_push_c_function(ctx, ccsp_session_module_open, 0);
+  duk_call(ctx, 0);
+  duk_put_global_string(ctx, "ccsp_session");
+
+  duk_get_global_string(ctx, "ccsp_session");
+  duk_get_prop_string(ctx, -1, "create");
+  ASSERT_EQ(duk_pcall(ctx, 0), DUK_EXEC_SUCCESS);
+  EXPECT_TRUE(duk_get_boolean(ctx, -1));
+  duk_pop_2(ctx);
+
+  duk_get_global_string(ctx, "ccsp_session");
+  duk_get_prop_string(ctx, -1, "create");
+  ASSERT_EQ(duk_pcall(ctx, 0), DUK_EXEC_SUCCESS);
+  EXPECT_TRUE(duk_get_boolean(ctx, -1));
+  duk_pop_2(ctx);
+
+  duk_get_global_string(ctx, "ccsp_session");
+  duk_get_prop_string(ctx, -1, "destroy");
+  ASSERT_EQ(duk_pcall(ctx, 0), DUK_EXEC_SUCCESS);
+  EXPECT_TRUE(duk_get_boolean(ctx, -1));
+  duk_pop_2(ctx);
+
+  duk_destroy_heap(ctx);
+}
+
+TEST(general, session_create_destroy_cycle_and_id_format)
+{
+  duk_context* ctx = duk_create_heap_default();
+  ASSERT_NE(ctx, nullptr);
+
+  duk_push_c_function(ctx, ccsp_session_module_open, 0);
+  duk_call(ctx, 0);
+  duk_put_global_string(ctx, "ccsp_session");
+
+  duk_get_global_string(ctx, "ccsp_session");
+  duk_get_prop_string(ctx, -1, "create");
+  ASSERT_EQ(duk_pcall(ctx, 0), DUK_EXEC_SUCCESS);
+  EXPECT_TRUE(duk_get_boolean(ctx, -1));
+  duk_pop_2(ctx);
+
+  duk_get_global_string(ctx, "ccsp_session");
+  duk_get_prop_string(ctx, -1, "getId");
+  ASSERT_EQ(duk_pcall(ctx, 0), DUK_EXEC_SUCCESS);
+  const char* first_id = duk_get_string(ctx, -1);
+  ASSERT_NE(first_id, nullptr);
+  EXPECT_EQ(strlen(first_id), 40u);
+  EXPECT_EQ(strncmp(first_id, "jst_sess", 8), 0);
+  duk_pop_2(ctx);
+
+  duk_get_global_string(ctx, "ccsp_session");
+  duk_get_prop_string(ctx, -1, "destroy");
+  ASSERT_EQ(duk_pcall(ctx, 0), DUK_EXEC_SUCCESS);
+  EXPECT_TRUE(duk_get_boolean(ctx, -1));
+  duk_pop_2(ctx);
+
+  duk_get_global_string(ctx, "ccsp_session");
+  duk_get_prop_string(ctx, -1, "create");
+  ASSERT_EQ(duk_pcall(ctx, 0), DUK_EXEC_SUCCESS);
+  EXPECT_TRUE(duk_get_boolean(ctx, -1));
+  duk_pop_2(ctx);
+
+  duk_get_global_string(ctx, "ccsp_session");
+  duk_get_prop_string(ctx, -1, "destroy");
+  ASSERT_EQ(duk_pcall(ctx, 0), DUK_EXEC_SUCCESS);
+  EXPECT_TRUE(duk_get_boolean(ctx, -1));
+  duk_pop_2(ctx);
+
+  duk_destroy_heap(ctx);
+}
+
 int main(int argc, char* argv[])
 {
   ::testing::InitGoogleTest(&argc, argv);