fix: check for 0 length array values and key strings

Author: bduranleau-nr

agent/fw_drupal8.c

@@ -605,6 +605,16 @@ NR_PHP_WRAPPER(nr_drupal94_invoke_all_with_clean) {
 NR_PHP_WRAPPER_END
 #endif  // OAPI
 
+static bool nr_validate_key_val_arr(nr_php_string_hash_key_t* key, zval* val) {
+  if (NULL == key || 0 == ZEND_STRING_LEN(key)
+      || 0 == nr_php_is_zval_valid_array(val)
+      || 0 == zend_hash_num_elements(Z_ARRVAL_P(val))) {
+    return true;
+  } else {
+    return false;
+  }
+}
+
 /*
  * Purpose: Instrument Drupal Attribute Hooks for Drupal 11.1+
  *
@@ -636,23 +646,23 @@ static bool nr_drupal_hook_attribute_instrument(zval* module_handler) {
 
   ZEND_HASH_FOREACH_STR_KEY_VAL(Z_ARRVAL_P(hook_implementation_map), hook_key,
                                 hook_val) {
-    if ((NULL == hook_key) || (0 == nr_php_is_zval_valid_array(hook_val))) {
+    if (nr_validate_key_val_arr(hook_key, hook_val)) {
       nrl_warning(NRL_FRAMEWORK,
                   "hookImplementationsMap[hook]: invalid key or value");
       return false;
     }
 
     ZEND_HASH_FOREACH_STR_KEY_VAL(Z_ARRVAL_P(hook_val), class_key, class_val) {
-      if ((NULL == class_key) || (0 == nr_php_is_zval_valid_array(class_val))) {
+      if (nr_validate_key_val_arr(class_key, class_val)) {
         nrl_warning(NRL_FRAMEWORK,
                     "hookImplementationsMap[class]: invalid key or value");
         return false;
       }
 
       ZEND_HASH_FOREACH_STR_KEY_VAL(Z_ARRVAL_P(class_val), method_key,
                                     module_val) {
-        if ((NULL == method_key)
-            || (0 == nr_php_is_zval_valid_string(module_val))) {
+        if (NULL == method_key
+            || 0 == nr_php_is_zval_valid_string(module_val)) {
           nrl_warning(NRL_FRAMEWORK,
                       "hookImplementationsMap[method]: invalid key or value");
           return false;

tests/integration/frameworks/drupal/mock_module_handler_empty_array.php

@@ -0,0 +1,34 @@
+<?php
+/*
+ * Copyright 2020 New Relic Corporation. All rights reserved.
+ * SPDX-License-Identifier: Apache-2.0
+ */
+
+/* Verify agent behavior when key value is an empty array */
+
+namespace Drupal\Core\Extension {
+  interface ModuleHandlerInterface
+  {
+    public function invokeAllWith($hook_str, $callback);
+  }
+  class ModuleHandler implements ModuleHandlerInterface
+  {
+    protected array $hookImplementationsMap = array(
+      'hookname' => array('classname' => array('methodname' => 'modulename')),
+      'hookname_b' => array(),
+      'hookname_c' => array('classname_c' => array('methodname_c' => 'modulename_c')),
+    );
+
+    // to avoid editor warnings
+    public function invokeAllWith($hook_str, $callback)
+    {
+      return null;
+    }
+
+    // for debugging purposes
+    public function dump()
+    {
+      var_dump($this->hookImplementationsMap);
+    }
+  }
+}

tests/integration/frameworks/drupal/mock_module_handler_empty_key.php

@@ -0,0 +1,34 @@
+<?php
+/*
+ * Copyright 2020 New Relic Corporation. All rights reserved.
+ * SPDX-License-Identifier: Apache-2.0
+ */
+
+/* Verify agent behavior when key is an empty string */
+
+namespace Drupal\Core\Extension {
+  interface ModuleHandlerInterface
+  {
+    public function invokeAllWith($hook_str, $callback);
+  }
+  class ModuleHandler implements ModuleHandlerInterface
+  {
+    protected array $hookImplementationsMap = array(
+      'hookname' => array('classname' => array('methodname' => 'modulename')),
+      'hookname_b' => array('' => array('methodname_b' => 'modulename_b')),
+      'hookname_c' => array('classname_c' => array('methodname_c' => 'modulename_c')),
+    );
+
+    // to avoid editor warnings
+    public function invokeAllWith($hook_str, $callback)
+    {
+      return null;
+    }
+
+    // for debugging purposes
+    public function dump()
+    {
+      var_dump($this->hookImplementationsMap);
+    }
+  }
+}

tests/integration/frameworks/drupal/test_hook_implementations_map_empty_array.php

@@ -0,0 +1,35 @@
+<?php
+/*
+ * Copyright 2020 New Relic Corporation. All rights reserved.
+ * SPDX-License-Identifier: Apache-2.0
+ */
+
+/*DESCRIPTION
+Verify agent behavior when key value is an empty array
+*/
+
+/*INI
+newrelic.framework = drupal8
+*/
+
+/*EXPECT_TRACED_ERRORS null */
+
+/*EXPECT_ERROR_EVENTS null */
+
+/*EXPECT
+*/
+
+require_once __DIR__ . '/mock_module_handler_empty_array.php';
+
+// This specific API is needed for us to instrument the ModuleHandler
+class Drupal
+{
+  public function moduleHandler()
+  {
+    return new Drupal\Core\Extension\ModuleHandler();
+  }
+}
+
+// Create module handler
+$drupal = new Drupal();
+$handler = $drupal->moduleHandler();

tests/integration/frameworks/drupal/test_hook_implementations_map_empty_key.php

@@ -0,0 +1,35 @@
+<?php
+/*
+ * Copyright 2020 New Relic Corporation. All rights reserved.
+ * SPDX-License-Identifier: Apache-2.0
+ */
+
+/*DESCRIPTION
+Verify agent behavior when key is an empty string
+*/
+
+/*INI
+newrelic.framework = drupal8
+*/
+
+/*EXPECT_TRACED_ERRORS null */
+
+/*EXPECT_ERROR_EVENTS null */
+
+/*EXPECT
+*/
+
+require_once __DIR__ . '/mock_module_handler_empty_key.php';
+
+// This specific API is needed for us to instrument the ModuleHandler
+class Drupal
+{
+  public function moduleHandler()
+  {
+    return new Drupal\Core\Extension\ModuleHandler();
+  }
+}
+
+// Create module handler
+$drupal = new Drupal();
+$handler = $drupal->moduleHandler();