From e596719e4f951a3d85d93f5c3ecf0d41e9095091 Mon Sep 17 00:00:00 2001 From: Pepijn Noltes Date: Fri, 22 Dec 2023 16:38:39 +0100 Subject: [PATCH] Improve substring handling and allow empty attribute filter values --- .../src/FilterErrorInjectionTestSuite.cc | 8 +++++ libs/utils/gtest/src/FilterTestSuite.cc | 31 ++++++++++++++--- libs/utils/src/filter.c | 34 ++++++++++++++++--- 3 files changed, 63 insertions(+), 10 deletions(-) diff --git a/libs/utils/gtest/src/FilterErrorInjectionTestSuite.cc b/libs/utils/gtest/src/FilterErrorInjectionTestSuite.cc index add870087..0714ef560 100644 --- a/libs/utils/gtest/src/FilterErrorInjectionTestSuite.cc +++ b/libs/utils/gtest/src/FilterErrorInjectionTestSuite.cc @@ -194,6 +194,14 @@ TEST_F(FilterErrorInjectionTestSuite, ErrorStrdupTest) { //Then the filter creation should fail, because it strdup the filter string celix_filter_t* filter = celix_filter_create(filterStr); EXPECT_EQ(nullptr, filter); + + //Given an error injection for celix_utils_strdup + celix_ei_expect_celix_utils_strdup((void*)celix_filter_create, 4, nullptr); + //When creating a filter with an empty attribute value + filterStr = "(key1=)"; + //Then the filter creation should fail, because it strdup the filter string + filter = celix_filter_create(filterStr); + EXPECT_EQ(nullptr, filter); } diff --git a/libs/utils/gtest/src/FilterTestSuite.cc b/libs/utils/gtest/src/FilterTestSuite.cc index cf1262979..ca6b9c111 100644 --- a/libs/utils/gtest/src/FilterTestSuite.cc +++ b/libs/utils/gtest/src/FilterTestSuite.cc @@ -129,7 +129,7 @@ TEST_F(FilterTestSuite, MiscInvalidCreateTest) { // test parsing a value of 0 length const char* str5 = "(test_attr3>=)"; filter = celix_filter_create(str5); - ASSERT_TRUE(filter == nullptr); + ASSERT_FALSE(filter == nullptr); // test parsing a value with an escaped closing parenthesis "\ ")" const char* str6 = "(test_attr3>=strWith\\)inIt)"; @@ -163,12 +163,12 @@ TEST_F(FilterTestSuite, MiscInvalidCreateTest) { // test parsing APPROX operator missing value const char* str11 = "(a~=)"; filter = celix_filter_create(str11); - ASSERT_TRUE(filter == nullptr); + ASSERT_FALSE(filter == nullptr); // test parsing LESS operator missing value const char* str12 = "(a<)"; filter = celix_filter_create(str12); - ASSERT_TRUE(filter == nullptr); + ASSERT_FALSE(filter == nullptr); } TEST_F(FilterTestSuite, MatchEqualTest) { @@ -203,6 +203,7 @@ TEST_F(FilterTestSuite, MatchTest) { ASSERT_NE(nullptr, props); celix_properties_set(props, "test_attr1", "attr1"); celix_properties_set(props, "test_attr2", "attr2"); + celix_properties_set(props, "empty_attr", ""); //note empty string as value // Test EQUALS filter = celix_filter_create("(test_attr1=attr1)"); @@ -226,6 +227,16 @@ TEST_F(FilterTestSuite, MatchTest) { EXPECT_TRUE(result); celix_filter_destroy(filter); + filter = celix_filter_create("(empty_attr="); + result = celix_filter_match(filter, props); + EXPECT_TRUE(result); + celix_filter_destroy(filter); + + filter = celix_filter_create("(empty_attr=val)"); + result = celix_filter_match(filter, props); + EXPECT_FALSE(result); + celix_filter_destroy(filter); + // Test PRESENT filter = celix_filter_create("(test_attr1=*)"); result = celix_filter_match(filter, props); @@ -399,14 +410,19 @@ TEST_F(FilterTestSuite, FilterEqualsTest) { celix_autoptr(celix_filter_t) f4 = celix_filter_create("(&(test_attr1=attr1)(test_attr2=attr2))"); celix_autoptr(celix_filter_t) f5 = celix_filter_create("(&(test_attr1=attr1)(test_attr2=attr2))"); celix_autoptr(celix_filter_t) f6 = celix_filter_create("(&(test_attr1=attr1)(test_attr2=attr2)(test_attr3=attr3))"); - - EXPECT_TRUE(celix_filter_equals(f1, f1)); EXPECT_FALSE(celix_filter_equals(f1, nullptr)); EXPECT_TRUE(celix_filter_equals(f1, f2)); EXPECT_FALSE(celix_filter_equals(f1, f3)); EXPECT_TRUE(celix_filter_equals(f4, f5)); EXPECT_FALSE(celix_filter_equals(f4, f6)); + + // equals with substring test + celix_autoptr(celix_filter_t) f7 = celix_filter_create("(test_attr1=*test*)"); + celix_autoptr(celix_filter_t) f8 = celix_filter_create("(test_attr1=*test*)"); + celix_autoptr(celix_filter_t) f9 = celix_filter_create("(test_attr1=*test)"); + EXPECT_TRUE(celix_filter_equals(f7, f8)); + EXPECT_FALSE(celix_filter_equals(f7, f9)); } TEST_F(FilterTestSuite, AutoCleanupTest) { @@ -527,6 +543,11 @@ TEST_F(FilterTestSuite, SubStringTest) { celix_autoptr(celix_filter_t) filter14 = celix_filter_create("(test3= *)"); EXPECT_NE(nullptr, filter14); EXPECT_TRUE(celix_filter_match(filter14, props)); + + //test filter with double escaped asterisk on both sides + celix_autoptr(celix_filter_t) filter15 = celix_filter_create("(test=*John*)"); + EXPECT_NE(nullptr, filter15); + EXPECT_TRUE(celix_filter_match(filter15, props)); } TEST_F(FilterTestSuite, CreateEmptyFilter) { diff --git a/libs/utils/src/filter.c b/libs/utils/src/filter.c index 8c09ff90a..e47285b8f 100644 --- a/libs/utils/src/filter.c +++ b/libs/utils/src/filter.c @@ -212,7 +212,7 @@ static celix_filter_t* celix_filter_parseNot(const char* filterString, int* pos) static celix_filter_t* celix_filter_parseItem(const char* filterString, int* pos) { celix_autofree char* attr = celix_filter_parseAttributeOrValue(filterString, pos, true); - if (!attr) { + if (!attr || strlen(attr) == 0) { celix_err_push("Filter Error: Missing attr."); return NULL; } @@ -371,8 +371,13 @@ static char* celix_filter_parseAttributeOrValue(const char* filterString, int* p } if (!stream) { - celix_err_push("Filter Error: Empty value.\n"); - return NULL; + // empty value + value = celix_utils_strdup(""); + if (!value) { + celix_err_push("Filter Error: Failed to allocate memory."); + return NULL; + } + return celix_steal_ptr(value); } int rc = fclose(celix_steal_ptr(stream)); @@ -660,7 +665,7 @@ static bool celix_filter_matchSubString(const celix_filter_t* filter, const celi for (int i = 1; i < celix_arrayList_size(filter->children) - 1; i++) { const char* substr = celix_arrayList_get(filter->children, i); const char* found = strstr(currentValue, substr); - if (!found || found <= currentValue) { + if (!found) { return false; } currentValue = found + celix_utils_strlen(substr); @@ -668,7 +673,7 @@ static bool celix_filter_matchSubString(const celix_filter_t* filter, const celi if (final) { const char* found = strstr(currentValue, final); - if (!found || found <= currentValue || found + celix_utils_strlen(final) != entry->value + strLen) { + if (!found || found + celix_utils_strlen(final) != entry->value + strLen) { return false; } } @@ -842,6 +847,25 @@ bool celix_filter_equals(const celix_filter_t* filter1, const celix_filter_t* fi return false; } + if (filter1->operand == CELIX_FILTER_OPERAND_SUBSTRING) { + assert(filter1->children != NULL); + assert(filter2->children != NULL); + int sizeSrc = celix_arrayList_size(filter1->children); + int sizeDest = celix_arrayList_size(filter2->children); + if (sizeSrc == sizeDest) { + int i; + for (i = 0; i < celix_arrayList_size(filter1->children); i++) { + const char* srcPart = celix_arrayList_get(filter1->children, i); + const char* destPart = celix_arrayList_get(filter2->children, i); + if (!celix_utils_stringEquals(srcPart, destPart)) { + break; + } + } + return i == sizeSrc; + } + return false; + } + // compare attr and value bool attrSame = false; bool valSame = false;