Correcting issue with multiple edges

When `r3_node_find_common_prefix()` searches for the common prefix it
selects the first matched edge, which might not be the best match.

This issue gives memoryleaks which can be viewed in legacy
testcase `check_tree::test_insert_pathl()` by building using:
`CFLAGS="-fno-omit-frame-pointer -fsanitize=leak" cmake ..`
See testcase procedures:
    ret = r3_tree_insert_path(n, "/foo/{id}",  NULL);
    ..
    ret = r3_tree_insert_path(n, "/foo/{idx}/{idy}",  NULL);
    ..
    ret = r3_tree_insert_path(n, "/foo/{idx}/{idh}",  NULL); <-- leaks

Also added a testcase that triggers the problem including a leak for
reproduction on baseline.
This commit is contained in:
Björn Svensson 2021-09-20 16:27:12 +02:00
parent e20e48a5ce
commit ff1ef2c148
2 changed files with 59 additions and 5 deletions

View file

@ -613,17 +613,18 @@ R3Route * r3_tree_insert_routel_ex(R3Node *tree, int method, const char *path, i
*/ */
R3Edge * r3_node_find_common_prefix(R3Node *n, const char *path, int path_len, int *prefix_len, char **errstr) { R3Edge * r3_node_find_common_prefix(R3Node *n, const char *path, int path_len, int *prefix_len, char **errstr) {
unsigned int i = 0; unsigned int i = 0;
int prefix = 0; int edge_prefix, prefix = 0;
*prefix_len = 0; *prefix_len = 0;
R3Edge *e = NULL; R3Edge *e = NULL;
for(i = 0 ; i < n->edges.size ; i++ ) { // Check all edges to find the most common prefix
for(i = 0; i < n->edges.size; i++) {
// ignore all edges with slug // ignore all edges with slug
prefix = strndiff( (char*) path, n->edges.entries[i].pattern.base, n->edges.entries[i].pattern.len); edge_prefix = strndiff( (char*) path, n->edges.entries[i].pattern.base, n->edges.entries[i].pattern.len);
// no common, consider insert a new edge // no common, consider insert a new edge
if ( prefix > 0 ) { if (edge_prefix > prefix) {
prefix = edge_prefix;
e = n->edges.entries + i; e = n->edges.entries + i;
break;
} }
} }

View file

@ -227,7 +227,59 @@ START_TEST (test_find_common_prefix_same_pattern2)
} }
END_TEST END_TEST
START_TEST (test_find_common_prefix_multi_edge)
{
R3Node * n = r3_tree_create(10);
char *test_str1 = "{id}/foo";
R3Edge * e1 = r3_node_append_edge(n);
r3_edge_initl(e1, test_str1, strlen(test_str1), NULL);
char *test_str2 = "{id2}/bar";
R3Edge * e2 = r3_node_append_edge(n);
r3_edge_initl(e2, test_str2, strlen(test_str2), NULL);
R3Edge *ret_edge = NULL;
int prefix_len = 0;
char *errstr = NULL;
char *test_pref1 = "{id}";
ret_edge = r3_node_find_common_prefix(n, test_pref1, strlen(test_pref1), &prefix_len, &errstr);
ck_assert(ret_edge != NULL);
ck_assert_int_eq(prefix_len, 4);
ck_assert(ret_edge == e1);
SAFE_FREE(errstr);
prefix_len = 0;
errstr = NULL;
char *test_pref2 = "{id}/foo";
ret_edge = r3_node_find_common_prefix(n, test_pref2, strlen(test_pref2), &prefix_len, &errstr);
ck_assert(ret_edge != NULL);
ck_assert_int_eq(prefix_len, 8);
ck_assert(ret_edge == e1);
SAFE_FREE(errstr);
prefix_len = 0;
errstr = NULL;
char *test_pref3 = "{id2}";
ret_edge = r3_node_find_common_prefix(n, test_pref3, strlen(test_pref3), &prefix_len, &errstr);
ck_assert(ret_edge != NULL);
ck_assert_int_eq(prefix_len, 5);
ck_assert(ret_edge == e2);
SAFE_FREE(errstr);
prefix_len = 0;
errstr = NULL;
char *test_pref4 = "{id2}/bar";
ret_edge = r3_node_find_common_prefix(n, test_pref4, strlen(test_pref4), &prefix_len, &errstr);
ck_assert(ret_edge != NULL);
ck_assert_int_eq(prefix_len, 9);
ck_assert(ret_edge == e2);
SAFE_FREE(errstr);
r3_tree_free(n);
}
END_TEST
@ -804,6 +856,7 @@ Suite* r3_suite (void) {
tcase_add_test(tcase, test_find_common_prefix_middle); tcase_add_test(tcase, test_find_common_prefix_middle);
tcase_add_test(tcase, test_find_common_prefix_same_pattern); tcase_add_test(tcase, test_find_common_prefix_same_pattern);
tcase_add_test(tcase, test_find_common_prefix_same_pattern2); tcase_add_test(tcase, test_find_common_prefix_same_pattern2);
tcase_add_test(tcase, test_find_common_prefix_multi_edge);
suite_add_tcase(suite, tcase); suite_add_tcase(suite, tcase);
tcase = tcase_create("insert_testcase"); tcase = tcase_create("insert_testcase");