Skip to content

Commit

Permalink
gh-685: Fix several memleaks in properties encoding
Browse files Browse the repository at this point in the history
  • Loading branch information
@pnoltes
pnoltes committed Apr 14, 2024
1 parent ac2b131 commit 0e58944
Show file tree
Hide file tree
Showing 4 changed files with 126 additions and 64 deletions.
8 changes: 4 additions & 4 deletions libs/utils/gtest/src/PropertiesEncodingErrorInjectionTestSuite.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ TEST_F(PropertiesEncodingErrorInjectionTestSuite, SaveErrorTest) {
celix_ei_expect_open_memstream((void*)celix_properties_saveToString, 0, nullptr);

//When I call celix_properties_saveToString
char* out = nullptr;
char* out;
status = celix_properties_saveToString(props, 0, &out);

//Then I expect an error
Expand All @@ -90,7 +90,7 @@ TEST_F(PropertiesEncodingErrorInjectionTestSuite, EncodeErrorTest) {
celix_ei_expect_celix_utils_writeOrCreateString((void*)celix_properties_saveToString, 2, nullptr);

// And I call celix_properties_saveToString using NESTED encoding (whitebox-knowledge)
char* out = nullptr;
char* out;
auto status = celix_properties_saveToString(props, CELIX_PROPERTIES_ENCODE_NESTED_STYLE, &out);

// Then I expect an error
Expand Down Expand Up @@ -149,7 +149,7 @@ TEST_F(PropertiesEncodingErrorInjectionTestSuite, EncodeArrayErrorTest) {
celix_ei_expect_json_array((void*)celix_properties_saveToString, 4, nullptr);

// And I call celix_properties_saveToString
char* out = nullptr;
char* out;
auto status = celix_properties_saveToString(props, 0, &out);

// Then I expect an error
Expand Down Expand Up @@ -189,7 +189,7 @@ TEST_F(PropertiesEncodingErrorInjectionTestSuite, EncodeVersionErrorTest) {
celix_ei_expect_json_sprintf((void*)celix_properties_saveToString, 4, nullptr);

// And I call celix_properties_saveToString
char* out = nullptr;
char* out;
auto status = celix_properties_saveToString(props, 0, &out);

// Then I expect an error
Expand Down
107 changes: 61 additions & 46 deletions libs/utils/gtest/src/PropertiesEncodingTestSuite.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -93,14 +93,17 @@ TEST_F(PropertiesSerializationTestSuite, SavePropertiesWithNaNAndInfValuesTest)
celix_autoptr(celix_properties_t) props = celix_properties_create();
celix_properties_setDouble(props, key, strtod(key, nullptr));

//And an in-memory stream
celix_autofree char* buf = nullptr;
size_t bufLen = 0;
FILE* stream = open_memstream(&buf, &bufLen);
// Then saving the properties to a string succeeds, but value is not added to the JSON (because JSON does not
// support NAN, INF and -INF)
celix_autofree char* output;
auto status = celix_properties_saveToString(props, 0, &output);
ASSERT_EQ(CELIX_SUCCESS, status);
EXPECT_STREQ("{}", output);

//Then saving the properties to the stream fails, because JSON does not support NAN, INF and -INF
//And saving the properties to a string with the flag CELIX_PROPERTIES_ENCODE_ERROR_ON_NAN_INF fails
celix_err_resetErrors();
auto status = celix_properties_saveToStream(props, stream, 0);
char* output2;
status = celix_properties_saveToString(props, CELIX_PROPERTIES_ENCODE_ERROR_ON_NAN_INF, &output2);
EXPECT_EQ(CELIX_ILLEGAL_ARGUMENT, status);

//And an error msg is added to celix_err
Expand All @@ -112,25 +115,30 @@ TEST_F(PropertiesSerializationTestSuite, SavePropertiesWithNaNAndInfValuesTest)
TEST_F(PropertiesSerializationTestSuite, SavePropertiesWithArrayListsTest) {
// Given a properties object with array list values
celix_autoptr(celix_properties_t) props = celix_properties_create();

celix_array_list_t* list1 = celix_arrayList_createStringArray();
celix_arrayList_addString(list1, "value1");
celix_arrayList_addString(list1, "value2");
celix_properties_assignArrayList(props, "key1", list1);

celix_array_list_t* list2 = celix_arrayList_createLongArray();
celix_arrayList_addLong(list2, 1);
celix_arrayList_addLong(list2, 2);
celix_properties_assignArrayList(props, "key2", list2);

celix_array_list_t* list3 = celix_arrayList_createDoubleArray();
celix_arrayList_addDouble(list3, 1.0);
celix_arrayList_addDouble(list3, 2.0);
celix_properties_assignArrayList(props, "key3", list3);

celix_array_list_t* list4 = celix_arrayList_createBoolArray();
celix_arrayList_addBool(list4, true);
celix_arrayList_addBool(list4, false);
celix_properties_assignArrayList(props, "key4", list4);

celix_array_list_t* list5 = celix_arrayList_createVersionArray();
celix_arrayList_addVersion(list5, celix_version_create(1, 2, 3, "qualifier"));
celix_arrayList_addVersion(list5, celix_version_create(4, 5, 6, "qualifier"));
celix_arrayList_assignVersion(list5, celix_version_create(1, 2, 3, "qualifier"));
celix_arrayList_assignVersion(list5, celix_version_create(4, 5, 6, "qualifier"));
celix_properties_assignArrayList(props, "key5", list5);

// And an in-memory stream
Expand Down Expand Up @@ -170,17 +178,18 @@ TEST_F(PropertiesSerializationTestSuite, SaveEmptyArrayTest) {
EXPECT_EQ(5, celix_properties_size(props));

//When saving the properties to a string
char* output = nullptr;
auto status = celix_properties_saveToString(props, 0, &output);
celix_autofree char* output1;
auto status = celix_properties_saveToString(props, 0, &output1);

//Then the save went ok
ASSERT_EQ(CELIX_SUCCESS, status);

//And the output contains an empty JSON object, because empty arrays are treated as unset
EXPECT_STREQ("{}", output);
EXPECT_STREQ("{}", output1);

//When saving the properties to a string with an error on empty array flag
status = celix_properties_saveToString(props, CELIX_PROPERTIES_ENCODE_ERROR_ON_EMPTY_ARRAYS, &output);
char* output2;
status = celix_properties_saveToString(props, CELIX_PROPERTIES_ENCODE_ERROR_ON_EMPTY_ARRAYS, &output2);

//Then the save fails, because the empty array generates an error
ASSERT_EQ(CELIX_ILLEGAL_ARGUMENT, status);
Expand Down Expand Up @@ -268,7 +277,7 @@ TEST_F(PropertiesSerializationTestSuite, SavePropertiesWithKeyNamesWithSlashesTe


//When saving the properties to a string
char* output = nullptr;
celix_autofree char* output;
auto status = celix_properties_saveToString(props, CELIX_PROPERTIES_ENCODE_NESTED_STYLE, &output);
ASSERT_EQ(CELIX_SUCCESS, status);

Expand Down Expand Up @@ -329,18 +338,19 @@ TEST_F(PropertiesSerializationTestSuite, SavePropertiesWithKeyCollision) {
celix_properties_set(props, "key1/key2", "value2"); //collision with object "key1/key2" -> overwrite

//When saving the properties to a string
char* output = nullptr;
auto status = celix_properties_saveToString(props, CELIX_PROPERTIES_ENCODE_NESTED_STYLE, &output);
celix_autofree char* output1;
auto status = celix_properties_saveToString(props, CELIX_PROPERTIES_ENCODE_NESTED_STYLE, &output1);

//Then the save succeeds
ASSERT_EQ(CELIX_SUCCESS, status);

// And both keys are serialized (one as a flat key) (flat key name is whitebox knowledge)
EXPECT_NE(nullptr, strstr(output, R"({"key1":{"key2":"value2"}})")) << "JSON: " << output;
EXPECT_NE(nullptr, strstr(output1, R"({"key1":{"key2":"value2"}})")) << "JSON: " << output1;

//When saving the properties to a string with the error on key collision flag
char* output2;
status = celix_properties_saveToString(
props, CELIX_PROPERTIES_ENCODE_NESTED_STYLE | CELIX_PROPERTIES_ENCODE_ERROR_ON_COLLISIONS, &output);
props, CELIX_PROPERTIES_ENCODE_NESTED_STYLE | CELIX_PROPERTIES_ENCODE_ERROR_ON_COLLISIONS, &output2);

//Then the save fails, because the keys collide
ASSERT_EQ(CELIX_ILLEGAL_ARGUMENT, status);
Expand All @@ -357,7 +367,7 @@ TEST_F(PropertiesSerializationTestSuite, SavePropertiesWithAndWithoutStrictFlagT
celix_properties_assignArrayList(props, "key1", list);

//When saving the properties to a string without the strict flag
char* output = nullptr;
celix_autofree char* output;
auto status = celix_properties_saveToString(props, 0, &output);

//Then the save succeeds
Expand All @@ -381,7 +391,7 @@ TEST_F(PropertiesSerializationTestSuite, SavePropertiesWithPrettyPrintTest) {
celix_properties_set(props, "key2", "value2");

//When saving the properties to a string with pretty print
char* output = nullptr;
celix_autofree char* output;
auto status = celix_properties_saveToString(props, CELIX_PROPERTIES_ENCODE_PRETTY, &output);

//Then the save succeeds
Expand Down Expand Up @@ -414,17 +424,14 @@ TEST_F(PropertiesSerializationTestSuite, SaveWithInvalidStreamTest) {
TEST_F(PropertiesSerializationTestSuite, LoadEmptyPropertiesTest) {
//Given an empty JSON object
const char* json = "{}";
FILE* stream = fmemopen((void*)json, strlen(json), "r");

//When loading the properties from the stream
celix_autoptr(celix_properties_t) props = nullptr;
auto status = celix_properties_loadFromStream(stream, 0, &props);
auto status = celix_properties_loadFromString2(json, 0, &props);
ASSERT_EQ(CELIX_SUCCESS, status);

//Then the properties object is empty
EXPECT_EQ(0, celix_properties_size(props));

fclose(stream);
}

TEST_F(PropertiesSerializationTestSuite, LoadPropertiesWithSingleValuesTest) {
Expand Down Expand Up @@ -586,7 +593,8 @@ TEST_F(PropertiesSerializationTestSuite, LoadPropertiesWithEmptyArrayTest) {
EXPECT_EQ(0, celix_properties_size(props));

//When loading the properties from string with a strict flag
status = celix_properties_loadFromString2(inputJSON, CELIX_PROPERTIES_DECODE_ERROR_ON_EMPTY_ARRAYS, &props);
celix_properties_t* props2;
status = celix_properties_loadFromString2(inputJSON, CELIX_PROPERTIES_DECODE_ERROR_ON_EMPTY_ARRAYS, &props2);

//Then loading fails, because the empty array generates an error
ASSERT_EQ(CELIX_ILLEGAL_ARGUMENT, status);
Expand Down Expand Up @@ -651,7 +659,7 @@ TEST_F(PropertiesSerializationTestSuite, LoadPropertiesWithDuplicatesTest) {
EXPECT_EQ(3, celix_properties_getLong(props, "key", 0));

// When decoding the properties from the stream using a flog that does not allow duplicates
celix_autoptr(celix_properties_t) props2 = nullptr;
celix_autoptr(celix_properties_t) props2;
status = celix_properties_loadFromString2(jsonInput, CELIX_PROPERTIES_DECODE_ERROR_ON_DUPLICATES, &props2);

// Then loading fails, because of a duplicate key
Expand Down Expand Up @@ -687,7 +695,7 @@ TEST_F(PropertiesSerializationTestSuite, LoadPropertiesEscapedSlashesTest) {
})";

// When loading the properties from a string.
celix_autoptr(celix_properties_t) props = nullptr;
celix_autoptr(celix_properties_t) props;
auto status = celix_properties_loadFromString2(jsonInput, 0, &props);

// Then loading succeeds
Expand All @@ -703,7 +711,7 @@ TEST_F(PropertiesSerializationTestSuite, LoadPropertiesEscapedSlashesTest) {
EXPECT_STREQ("value7", celix_properties_getString(props, "object3/key4"));

// When decoding the properties from a string using a flag that allows duplicates
celix_autoptr(celix_properties_t) props2 = nullptr;
celix_autoptr(celix_properties_t) props2;
status = celix_properties_loadFromString2(jsonInput, CELIX_PROPERTIES_DECODE_ERROR_ON_DUPLICATES, &props2);

// Then loading fails, because of a duplicate key
Expand All @@ -714,7 +722,7 @@ TEST_F(PropertiesSerializationTestSuite, LoadPropertiesEscapedSlashesTest) {
celix_err_printErrors(stderr, "Test Error: ", "\n");

// When decoding the properties from a string using a flag that allows collisions
celix_autoptr(celix_properties_t) props3 = nullptr;
celix_autoptr(celix_properties_t) props3;
status = celix_properties_loadFromString2(jsonInput, CELIX_PROPERTIES_DECODE_ERROR_ON_COLLISIONS, &props3);

// Then loading fails, because of a collision
Expand Down Expand Up @@ -861,34 +869,41 @@ TEST_F(PropertiesSerializationTestSuite, LoadWithInvalidStreamTest) {
TEST_F(PropertiesSerializationTestSuite, SaveAndLoadFlatProperties) {
// Given a properties object with all possible types (but no empty arrays)
celix_autoptr(celix_properties_t) props = celix_properties_create();
celix_properties_set(props, "strKey", "strValue");
celix_properties_setLong(props, "longKey", 42);
celix_properties_setDouble(props, "doubleKey", 2.0);
celix_properties_setBool(props, "boolKey", true);
celix_properties_setVersion(props, "versionKey", celix_version_create(1, 2, 3, "qualifier"));
auto* strArr = celix_arrayList_createStringArray();

celix_properties_set(props, "single/strKey", "strValue");
celix_properties_setLong(props, "single/longKey", 42);
celix_properties_setDouble(props, "single/doubleKey", 2.0);
celix_properties_setBool(props, "single/boolKey", true);
celix_properties_assignVersion(props, "single/versionKey", celix_version_create(1, 2, 3, "qualifier"));

celix_array_list_t* strArr = celix_arrayList_createStringArray();
celix_arrayList_addString(strArr, "value1");
celix_arrayList_addString(strArr, "value2");
auto* longArr = celix_arrayList_createLongArray();
celix_properties_assignArrayList(props, "array/stringArr", strArr);

celix_array_list_t* longArr = celix_arrayList_createLongArray();
celix_arrayList_addLong(longArr, 1);
celix_arrayList_addLong(longArr, 2);
celix_properties_assignArrayList(props, "longArr", longArr);
auto* doubleArr = celix_arrayList_createDoubleArray();
celix_properties_assignArrayList(props, "array/longArr", longArr);

celix_array_list_t* doubleArr = celix_arrayList_createDoubleArray();
celix_arrayList_addDouble(doubleArr, 1.0);
celix_arrayList_addDouble(doubleArr, 2.0);
celix_properties_assignArrayList(props, "doubleArr", doubleArr);
auto* boolArr = celix_arrayList_createBoolArray();
celix_properties_assignArrayList(props, "array/doubleArr", doubleArr);

celix_array_list_t* boolArr = celix_arrayList_createBoolArray();
celix_arrayList_addBool(boolArr, true);
celix_arrayList_addBool(boolArr, false);
celix_properties_assignArrayList(props, "boolArr", boolArr);
auto* versionArr = celix_arrayList_createVersionArray();
celix_arrayList_addVersion(versionArr, celix_version_create(1, 2, 3, "qualifier"));
celix_arrayList_addVersion(versionArr, celix_version_create(4, 5, 6, "qualifier"));
celix_properties_assignArrayList(props, "versionArr", versionArr);
celix_properties_assignArrayList(props, "array/boolArr", boolArr);

celix_array_list_t* versionArr = celix_arrayList_createVersionArray();
celix_arrayList_assignVersion(versionArr, celix_version_create(1, 2, 3, "qualifier"));
celix_arrayList_assignVersion(versionArr, celix_version_create(4, 5, 6, "qualifier"));
celix_properties_assignArrayList(props, "array/versionArr", versionArr);

// When saving the properties to a properties_test.json file
const char* filename = "properties_test.json";
auto status = celix_properties_save(props, filename, 0);
auto status = celix_properties_save(props, filename, CELIX_PROPERTIES_ENCODE_PRETTY);

// Then saving succeeds
ASSERT_EQ(CELIX_SUCCESS, status);
Expand All @@ -902,4 +917,4 @@ TEST_F(PropertiesSerializationTestSuite, SaveAndLoadFlatProperties) {

// And the loaded properties are equal to the original properties
EXPECT_TRUE(celix_properties_equals(props, loadedProps));
}
}
52 changes: 50 additions & 2 deletions libs/utils/include/celix_properties.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1020,6 +1020,16 @@ CELIX_UTILS_EXPORT bool celix_propertiesIterator_equals(const celix_properties_i
*/
#define CELIX_PROPERTIES_ENCODE_ERROR_ON_EMPTY_ARRAYS 0x20

/**
* @brief Flag to indicate that the encoding should fail if the JSON representation will contain NaN or Inf values.
*
* NaN, Inf and -Inf are not valid JSON values and as such properties entries with these values are not encoded.
*
* If this flag is set, the encoding will fail if the JSON representation will contain NaN or Inf values and if this
* flag is not set, the encoding will not fail and the NaN and Inf entries will be ignored.
*/
#define CELIX_PROPERTIES_ENCODE_ERROR_ON_NAN_INF 0x40

/**
* @brief Flag to indicate that all encode "error on" flags should be set.
*/
Expand All @@ -1031,7 +1041,26 @@ CELIX_UTILS_EXPORT bool celix_propertiesIterator_equals(const celix_properties_i
*
* The stream is expected to be a valid stream and is not reset or closed by this function.
*
* TODO document the JSON format
* Properties are encoded as a JSON object.
*
* If no encoding style flag is set of when the CELIX_PROPERTIES_ENCODE_FLAT_STYLE flag is set, properties
* entries are written as top level field entries.
*
* If the CELIX_PROPERTIES_ENCODE_NESTED_STYLE flag is set, properties entry keys are split on '/' and nested in
* JSON objects. This leads to a more natural JSON representation, but if there are colliding properties keys (e.g.
* `{"key": "value1", "key/with/slash": "value2"}`), not all properties entries will be written.
*
* With all encoding styles, the empty array properties entries are ignored, because they cannot be decoded to a valid
* properties array entry.
*
* Properties type entries are encoded as follows:
* - CELIX_PROPERTIES_TYPE_STRING: The value is encoded as a JSON string.
* - CELIX_PROPERTIES_TYPE_LONG: The value is encoded as a JSON number.
* - CELIX_PROPERTIES_TYPE_DOUBLE: The value is encoded as a JSON number.
* - CELIX_PROPERTIES_TYPE_BOOL: The value is encoded as a JSON boolean.
* - CELIX_PROPERTIES_TYPE_ARRAY: The value is encoded as a JSON array, with each element encoded according to its type.
* - CELIX_PROPERTIES_TYPE_VERSION: The value is encoded as a JSON string with a "version<" prefix and a ">" suffix
* (e.g. "version<1.2.3>").
*
* For a overview of the possible encode flags, see the CELIX_PROPERTIES_ENCODE_* flags documentation.
* The default encoding style is a compact and flat JSON representation.
Expand Down Expand Up @@ -1168,7 +1197,26 @@ CELIX_UTILS_EXPORT celix_status_t celix_properties_saveToString(const celix_prop
* The stream is expected to be a valid readable stream and is not reset or closed by this function.
* The content of the stream is expected to be in the format of a JSON object.
*
* TODO describe allowed and disallowed JSON objects.
* For decoding a single JSON object is decoded to a properties object.
*
* The keys of the JSON object are used as
* properties keys and the values of the JSON object are used as properties values. If there are nested
* JSON objects, the keys are concatenated with a '/' separator (e.g. `{"key": {"nested": "value"}}` will be
* decoded to a properties object with a single entry with key `key/nested` and (string) value `value`).
*
* Because properties keys are created by concatenating the JSON keys, there there could be collisions
* (e.g. `{"obj/key": "value", "obj": {"key": "value2"}}`, two entries with the key `obj/key`. In this case
* the last decoded JSON entry will be used.
*
* Properties entry types are determined by the JSON value type:
* - JSON string values are decoded as string properties entries.
* - JSON number values are decoded as long or double properties entries, depending on the value.
* - JSON boolean values are decoded as boolean properties entries.
* - jSON string values with a "version<" prefix and a ">" suffix are decoded as version properties entries (e.g.
* "version<1.2.3>").
* - JSON array values are decoded as array properties entries. The array can contain any of the above types, but mixed
* arrays are not supported.
* - JSON null values are ignored.
*
* For a overview of the possible decode flags, see the CELIX_PROPERTIES_DECODE_* flags documentation.
*
Expand Down
Loading

0 comments on commit 0e58944

Please sign in to comment.