From ff1ef2c14884b92c80197420839f97291dd1999a Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Bj=C3=B6rn=20Svensson?= <bjorn.a.svensson@est.tech>
Date: Mon, 20 Sep 2021 16:27:12 +0200
Subject: [PATCH] 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.
---
 src/node.c         | 11 +++++-----
 tests/check_tree.c | 53 ++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 59 insertions(+), 5 deletions(-)

diff --git a/src/node.c b/src/node.c
index 6f4fa0c..f08bc88 100644
--- a/src/node.c
+++ b/src/node.c
@@ -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) {
     unsigned int i = 0;
-    int prefix = 0;
+    int edge_prefix, prefix = 0;
     *prefix_len = 0;
     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
-        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
-        if ( prefix > 0 ) {
+        if (edge_prefix > prefix) {
+            prefix = edge_prefix;
             e = n->edges.entries + i;
-            break;
         }
     }
 
diff --git a/tests/check_tree.c b/tests/check_tree.c
index a9ac00f..9533757 100644
--- a/tests/check_tree.c
+++ b/tests/check_tree.c
@@ -227,7 +227,59 @@ START_TEST (test_find_common_prefix_same_pattern2)
 }
 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_same_pattern);
         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);
 
         tcase = tcase_create("insert_testcase");