Skip to content

Commit

Permalink
Improve substring handling and allow empty attribute filter values
Browse files Browse the repository at this point in the history
  • Loading branch information
pnoltes committed Dec 22, 2023
1 parent 9fe3a2f commit e596719
Show file tree
Hide file tree
Showing 3 changed files with 63 additions and 10 deletions.
8 changes: 8 additions & 0 deletions libs/utils/gtest/src/FilterErrorInjectionTestSuite.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}


Expand Down
31 changes: 26 additions & 5 deletions libs/utils/gtest/src/FilterTestSuite.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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)";
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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)");
Expand All @@ -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);
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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) {
Expand Down
34 changes: 29 additions & 5 deletions libs/utils/src/filter.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down Expand Up @@ -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));
Expand Down Expand Up @@ -660,15 +665,15 @@ 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);
}

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;
}
}
Expand Down Expand Up @@ -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;
Expand Down

0 comments on commit e596719

Please sign in to comment.