Skip to content

Commit

Permalink
refactor: suggestion to #765 (#789)
Browse files Browse the repository at this point in the history
A couple of suggestions to #765:
1. Save time by using pre-calculated string lengths vs calling strlen
whenever possible
2. Move `nr_file_basename` as `nr_wordpress_strip_php_suffix` back to
`fw_wordpress.c`
  • Loading branch information
lavarou authored Dec 4, 2023
1 parent 52ce225 commit a53c360
Show file tree
Hide file tree
Showing 6 changed files with 139 additions and 68 deletions.
51 changes: 38 additions & 13 deletions agent/fw_wordpress.c
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,27 @@ static nr_matcher_t* create_matcher_for_constant(const char* constant,
return NULL;
}

/*
* Purpose : Strip the ".php" file extension from a file name
*
* Params : 1. The string filename
* 2. The filename length
*
* Returns : A newly allocated string stripped of the .php extension
*
*/
static inline char* nr_wordpress_strip_php_suffix(char* filename, int filename_len) {
char* retval = NULL;

if (!nr_striendswith(filename, filename_len, NR_PSTR(".php"))) {
return filename;
}

retval = nr_strndup(filename, filename_len - (sizeof(".php") - 1));
nr_free(filename);
return retval;
}

static nr_matcher_t* nr_wordpress_core_matcher() {
nr_matcher_t* matcher = NULL;

Expand Down Expand Up @@ -149,24 +170,27 @@ static nr_matcher_t* nr_wordpress_theme_matcher() {

char* nr_php_wordpress_plugin_match_matcher(const char* filename) {
char* plugin = NULL;
plugin = nr_matcher_match(nr_wordpress_plugin_matcher(), filename);
plugin = nr_file_basename(plugin, nr_strlen(plugin));
int plugin_len;
plugin = nr_matcher_match_ex(nr_wordpress_plugin_matcher(), filename, nr_strlen(filename), &plugin_len);
plugin = nr_wordpress_strip_php_suffix(plugin, plugin_len);
nr_matcher_destroy(&NRPRG(wordpress_plugin_matcher));
return plugin;
}

char* nr_php_wordpress_theme_match_matcher(const char* filename) {
char* theme = NULL;
theme = nr_matcher_match(nr_wordpress_theme_matcher(), filename);
theme = nr_file_basename(theme, nr_strlen(theme));
int plugin_len;
theme = nr_matcher_match_ex(nr_wordpress_theme_matcher(), filename, nr_strlen(filename), &plugin_len);
theme = nr_wordpress_strip_php_suffix(theme, plugin_len);
nr_matcher_destroy(&NRPRG(wordpress_theme_matcher));
return theme;
}

char* nr_php_wordpress_core_match_matcher(const char* filename) {
char* core = NULL;
core = nr_matcher_match_core(nr_wordpress_core_matcher(), filename);
core = nr_file_basename(core, nr_strlen(core));
int plugin_len;
core = nr_matcher_match_core_ex(nr_wordpress_core_matcher(), filename, nr_strlen(filename), &plugin_len);
core = nr_wordpress_strip_php_suffix(core, plugin_len);
nr_matcher_destroy(&NRPRG(wordpress_core_matcher));
return core;
}
Expand All @@ -193,6 +217,7 @@ static char* nr_wordpress_plugin_from_function(zend_function* func TSRMLS_DC) {
const char* filename = NULL;
size_t filename_len;
char* plugin = NULL;
int plugin_len;

if (NULL == func) {
return NULL;
Expand All @@ -206,7 +231,7 @@ static char* nr_wordpress_plugin_from_function(zend_function* func TSRMLS_DC) {
NRP_PHP(NRPRG(wordpress_tag)));
return NULL;
}
filename_len = nr_strlen(filename);
filename_len = nr_php_function_filename_len(func);

if (NRPRG(wordpress_file_metadata)) {
if (nr_hashmap_get_into(NRPRG(wordpress_file_metadata), filename,
Expand All @@ -224,20 +249,20 @@ static char* nr_wordpress_plugin_from_function(zend_function* func TSRMLS_DC) {
"Wordpress: NOT found in cache: "
"filename=" NRP_FMT,
NRP_FILENAME(filename));
plugin = nr_matcher_match(nr_wordpress_plugin_matcher(), filename);
plugin = nr_file_basename(plugin, nr_strlen(plugin));
plugin = nr_matcher_match_ex(nr_wordpress_plugin_matcher(), filename, filename_len, &plugin_len);
plugin = nr_wordpress_strip_php_suffix(plugin, plugin_len);
if (plugin) {
goto cache_and_return;
}

plugin = nr_matcher_match(nr_wordpress_theme_matcher(), filename);
plugin = nr_file_basename(plugin, nr_strlen(plugin));
plugin = nr_matcher_match_ex(nr_wordpress_theme_matcher(), filename, filename_len, &plugin_len);
plugin = nr_wordpress_strip_php_suffix(plugin, plugin_len);
if (plugin) {
goto cache_and_return;
}

plugin = nr_matcher_match_core(nr_wordpress_core_matcher(), filename);
plugin = nr_file_basename(plugin, nr_strlen(plugin));
plugin = nr_matcher_match_core_ex(nr_wordpress_core_matcher(), filename, filename_len, &plugin_len);
plugin = nr_wordpress_strip_php_suffix(plugin, plugin_len);
if (plugin) {
/*
* The core wordpress functions are anonymized, so we don't need to return
Expand Down
5 changes: 5 additions & 0 deletions agent/php_agent.h
Original file line number Diff line number Diff line change
Expand Up @@ -702,6 +702,11 @@ nr_php_op_array_scope_name(const zend_op_array* op_array) {
return NULL;
}

static inline nr_string_len_t NRPURE
nr_php_function_filename_len(zend_function* func) {
return func ? nr_php_op_array_file_name_len(&func->op_array) : 0;
}

static inline const char* NRPURE
nr_php_ini_entry_name(const zend_ini_entry* entry) {
#if ZEND_MODULE_API_NO >= ZEND_7_0_X_API_NO /* PHP 7.0+ */
Expand Down
40 changes: 40 additions & 0 deletions axiom/tests/test_matcher.c
Original file line number Diff line number Diff line change
Expand Up @@ -63,9 +63,49 @@ static void test_match_single(void) {
nr_matcher_destroy(&matcher);
}

static void test_match_ex(void) {
char* found;
int len;
nr_matcher_t* matcher = nr_matcher_create();

nr_matcher_add_prefix(matcher, "/foo/bar");
found = nr_matcher_match_ex(matcher, NR_PSTR(""), &len);
tlib_pass_if_null("needle not matched", found);
tlib_pass_if_equal("needle match len", 0, len, int, "%d")
nr_free(found);

found = nr_matcher_match_ex(matcher, NR_PSTR("foo"), &len);
tlib_pass_if_null("needle not matched", found);
tlib_pass_if_equal("needle match len", 0, len, int, "%d")
nr_free(found);

found = nr_matcher_match_ex(matcher, NR_PSTR("/bar"), &len);
tlib_pass_if_null("needle not matched", found);
tlib_pass_if_equal("needle match len", 0, len, int, "%d")
nr_free(found);

found = nr_matcher_match_ex(matcher, NR_PSTR("/foo/bar/quux"), &len);
tlib_pass_if_str_equal("needle match", "quux", found);
tlib_pass_if_equal("needle match len", 4, len, int, "%d")
nr_free(found);

found = nr_matcher_match_ex(matcher, NR_PSTR("/foo/bar//quux"), &len);
tlib_pass_if_str_equal("needle match", "", found);
tlib_pass_if_equal("needle match len", 0, len, int, "%d")
nr_free(found);

found = nr_matcher_match_ex(matcher, NR_PSTR("/foo/bar/quux/baz"), &len);
tlib_pass_if_str_equal("needle match", "quux", found);
tlib_pass_if_equal("needle match len", 4, len, int, "%d")
nr_free(found);

nr_matcher_destroy(&matcher);
}

tlib_parallel_info_t parallel_info = {.suggested_nthreads = 2, .state_size = 0};

void test_main(void* p NRUNUSED) {
test_match_multiple();
test_match_single();
test_match_ex();
}
77 changes: 54 additions & 23 deletions axiom/util_matcher.c
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,15 @@
#include "util_memory.h"
#include "util_strings.h"

static void nr_matcher_prefix_dtor(void* prefix, void* userdata NRUNUSED) {
nr_free(prefix);
typedef struct {
char *cp;
int len;
} matcher_prefix;

static void nr_matcher_prefix_dtor(void* _p, void* userdata NRUNUSED) {
matcher_prefix* p = (matcher_prefix *)_p;
nr_free(p->cp);
nr_free(p);
}

nr_matcher_t* nr_matcher_create(void) {
Expand All @@ -31,38 +38,51 @@ void nr_matcher_destroy(nr_matcher_t** matcher_ptr) {
nr_realfree((void**)matcher_ptr);
}

bool nr_matcher_add_prefix(nr_matcher_t* matcher, const char* prefix) {
size_t i;
char* prefix_lc;
size_t prefix_len;
bool nr_matcher_add_prefix(nr_matcher_t* matcher, const char* str) {
int i;
matcher_prefix* prefix;

if (NULL == matcher || NULL == prefix) {
if (NULL == matcher || NULL == str) {
return false;
}

prefix_len = nr_strlen(prefix);
while (prefix_len > 0 && '/' == prefix[prefix_len - 1]) {
prefix_len--;
if (NULL == (prefix = nr_calloc(1, sizeof(matcher_prefix)))) {
return false;
}
prefix->len = nr_strlen(str);
while (prefix->len > 0 && '/' == str[prefix->len - 1]) {
prefix->len--;
}

prefix_lc = nr_malloc(prefix_len + 2);
for (i = 0; i < prefix_len; i++) {
prefix_lc[i] = nr_tolower(prefix[i]);
prefix->len += 1; // +1 for the trailing '/'
if (NULL == (prefix->cp = nr_malloc(prefix->len+1))) { // +1 for the '\0'
nr_matcher_prefix_dtor(prefix, NULL);
return false;
}
for (i = 0; i < prefix->len; i++) {
prefix->cp[i] = nr_tolower(str[i]);
}
prefix_lc[prefix_len] = '/';
prefix_lc[prefix_len + 1] = '\0';
prefix->cp[prefix->len-1] = '/';
prefix->cp[prefix->len] = '\0';

return nr_vector_push_back(&matcher->prefixes, prefix_lc);
return nr_vector_push_back(&matcher->prefixes, prefix);
}

static char* nr_matcher_match_internal(nr_matcher_t* matcher,
#define SET_SAFE(p, v) do {\
if (NULL != p) *p = (v); \
} while(0);
static char* nr_matcher_match_internal(nr_matcher_t* matcher,
const char* input,
int input_len,
int* match_len,
bool core) {
size_t i;
char* input_lc;
char* match = NULL;
size_t num_prefixes;

SET_SAFE(match_len, 0);

if (NULL == matcher || NULL == input) {
return NULL;
}
Expand All @@ -72,26 +92,29 @@ static char* nr_matcher_match_internal(nr_matcher_t* matcher,

for (i = 0; i < num_prefixes; i++) {
const char* found;
const char* prefix = nr_vector_get(&matcher->prefixes, i);
const matcher_prefix* prefix = nr_vector_get(&matcher->prefixes, i);

found = nr_strstr(input, prefix);
found = nr_strstr(input, prefix->cp);
if (found) {
const char* slash;

found += nr_strlen(prefix);
found += prefix->len;
if (true == core) {
slash = nr_strrchr(found, '/');
} else {
slash = nr_strchr(found, '/');
}
if (NULL == slash) {
match = nr_strdup(found);
SET_SAFE(match_len, input_len - (found-input));
} else {
if (true == core) {
const char* offset = input + nr_strlen(input);
const char* offset = input + input_len;
match = nr_strndup(slash + 1, offset - slash);
SET_SAFE(match_len, offset - (slash+1));
} else {
match = nr_strndup(found, slash - found);
SET_SAFE(match_len, slash - found);
}
}
break;
Expand All @@ -102,10 +125,18 @@ static char* nr_matcher_match_internal(nr_matcher_t* matcher,
return match;
}

char* nr_matcher_match_ex(nr_matcher_t* matcher, const char* input, int input_len, int *match_len) {
return nr_matcher_match_internal(matcher, input, input_len, match_len, false);
}

char* nr_matcher_match(nr_matcher_t* matcher, const char* input) {
return nr_matcher_match_internal(matcher, input, false);
return nr_matcher_match_internal(matcher, input, nr_strlen(input), NULL, false);
}

char* nr_matcher_match_core_ex(nr_matcher_t* matcher, const char* input, int input_len, int *match_len) {
return nr_matcher_match_internal(matcher, input, input_len, match_len, true);
}

char* nr_matcher_match_core(nr_matcher_t* matcher, const char* input) {
return nr_matcher_match_internal(matcher, input, true);
return nr_matcher_match_internal(matcher, input, nr_strlen(input), NULL, true);
}
2 changes: 2 additions & 0 deletions axiom/util_matcher.h
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,10 @@ extern void nr_matcher_destroy(nr_matcher_t** matcher_ptr);

extern bool nr_matcher_add_prefix(nr_matcher_t* matcher, const char* prefix);

extern char* nr_matcher_match_ex(nr_matcher_t* matcher, const char* input, int input_len, int* match_len);
extern char* nr_matcher_match(nr_matcher_t* matcher, const char* input);

extern char* nr_matcher_match_core_ex(nr_matcher_t* matcher, const char* input, int input_len, int* match_len);
extern char* nr_matcher_match_core(nr_matcher_t* matcher, const char* input);

#endif
32 changes: 0 additions & 32 deletions axiom/util_strings.h
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@
#include <string.h>

#include "util_object.h"
#include "util_memory.h"

/*
* Purpose : Convert a string to lower case, following USASCII rules, returning
Expand Down Expand Up @@ -473,35 +472,4 @@ static inline bool nr_striendswith(const char* s,
return 0 == nr_stricmp(suffix, pattern);
}

/*
* Purpose : Strip the ".php" file extension from a file name
*
* Params : 1. The string filename
* 2. The filename length
*
* Returns : A newly allocated string stripped of the .php extension
*
*/
static inline char* nr_file_basename(char* filename, int filename_len) {
char* retval = NULL;

if (NULL == filename || 0 >= filename_len) {
return NULL;
}

if (4 >= filename_len) {
/* if filename_len <= 4, there can't be a ".php" substring to remove. Assume
* the filename does not contain ".php" and return the original filename. */
return filename;
}

if (!nr_striendswith(filename, filename_len, NR_PSTR(".php"))) {
return filename;
}

retval = nr_strndup(filename, filename_len - (sizeof(".php") - 1));
nr_free(filename);
return retval;
}

#endif /* UTIL_STRINGS_HDR */

0 comments on commit a53c360

Please sign in to comment.