diff --git a/ChangeLog.txt b/ChangeLog.txt index c7e8d2ba..8212cbc9 100644 --- a/ChangeLog.txt +++ b/ChangeLog.txt @@ -149,6 +149,9 @@ Client library: - `mosquitto_unsubscribe*()` now returns MOSQ_ERR_INVAL if an empty string is passed as a topic filter. - Add websockets support. +- `mosquitto_property_read_binary/string/string_pair` will now set the + name/value parameter to NULL if the binary/string is empty. This aligns the + behaviour with other property functions. Closes #2648. Clients: - Add `-W` timeout support to Windows. diff --git a/include/mosquitto.h b/include/mosquitto.h index bd6f1af7..5aac8c1a 100644 --- a/include/mosquitto.h +++ b/include/mosquitto.h @@ -3246,6 +3246,7 @@ libmosq_EXPORT const mosquitto_property *mosquitto_property_read_varint( * property - property to read * identifier - property identifier (e.g. MQTT_PROP_PAYLOAD_FORMAT_INDICATOR) * value - pointer to store the value, or NULL if the value is not required. + * Will be set to NULL if the value has zero length. * skip_first - boolean that indicates whether the first item in the list * should be ignored or not. Should usually be set to false. * @@ -3275,6 +3276,7 @@ libmosq_EXPORT const mosquitto_property *mosquitto_property_read_binary( * identifier - property identifier (e.g. MQTT_PROP_PAYLOAD_FORMAT_INDICATOR) * value - pointer to char*, for the property data to be stored in, or NULL if * the value is not required. + * Will be set to NULL if the value has zero length. * skip_first - boolean that indicates whether the first item in the list * should be ignored or not. Should usually be set to false. * @@ -3303,8 +3305,10 @@ libmosq_EXPORT const mosquitto_property *mosquitto_property_read_string( * identifier - property identifier (e.g. MQTT_PROP_PAYLOAD_FORMAT_INDICATOR) * name - pointer to char* for the name property data to be stored in, or NULL * if the name is not required. + * Will be set to NULL if the name has zero length. * value - pointer to char*, for the property data to be stored in, or NULL if * the value is not required. + * Will be set to NULL if the value has zero length. * skip_first - boolean that indicates whether the first item in the list * should be ignored or not. Should usually be set to false. * diff --git a/lib/property_mosq.c b/lib/property_mosq.c index ba319427..f250dc5a 100644 --- a/lib/property_mosq.c +++ b/lib/property_mosq.c @@ -1112,10 +1112,14 @@ BROKER_EXPORT const mosquitto_property *mosquitto_property_read_binary(const mos if(value){ *len = p->value.bin.len; - *value = calloc(1, *len + 1U); - if(!(*value)) return NULL; + if(p->value.bin.len){ + *value = calloc(1, *len + 1U); + if(!(*value)) return NULL; - memcpy(*value, p->value.bin.v, *len); + memcpy(*value, p->value.bin.v, *len); + }else{ + *value = NULL; + } } return p; @@ -1141,10 +1145,14 @@ BROKER_EXPORT const mosquitto_property *mosquitto_property_read_string(const mos } if(value){ - *value = calloc(1, (size_t)p->value.s.len+1); - if(!(*value)) return NULL; + if(p->value.s.len){ + *value = calloc(1, (size_t)p->value.s.len+1); + if(!(*value)) return NULL; - memcpy(*value, p->value.s.v, p->value.s.len); + memcpy(*value, p->value.s.v, p->value.s.len); + }else{ + *value = NULL; + } } return p; @@ -1164,20 +1172,28 @@ BROKER_EXPORT const mosquitto_property *mosquitto_property_read_string_pair(cons if(p->identifier != MQTT_PROP_USER_PROPERTY) return NULL; if(name){ - *name = calloc(1, (size_t)p->name.len+1); - if(!(*name)) return NULL; - memcpy(*name, p->name.v, p->name.len); + if(p->name.len){ + *name = calloc(1, (size_t)p->name.len+1); + if(!(*name)) return NULL; + memcpy(*name, p->name.v, p->name.len); + }else{ + *name = NULL; + } } if(value){ - *value = calloc(1, (size_t)p->value.s.len+1); - if(!(*value)){ - if(name){ - SAFE_FREE(*name); + if(p->value.s.len){ + *value = calloc(1, (size_t)p->value.s.len+1); + if(!(*value)){ + if(name){ + SAFE_FREE(*name); + } + return NULL; } - return NULL; + memcpy(*value, p->value.s.v, p->value.s.len); + }else{ + *value = NULL; } - memcpy(*value, p->value.s.v, p->value.s.len); } return p; @@ -1246,37 +1262,45 @@ BROKER_EXPORT int mosquitto_property_copy_all(mosquitto_property **dest, const m case MQTT_PROP_SERVER_REFERENCE: case MQTT_PROP_REASON_STRING: pnew->value.s.len = src->value.s.len; - pnew->value.s.v = strdup(src->value.s.v); - if(!pnew->value.s.v){ - mosquitto_property_free_all(dest); - return MOSQ_ERR_NOMEM; + if(src->value.s.v){ + pnew->value.s.v = strdup(src->value.s.v); + if(!pnew->value.s.v){ + mosquitto_property_free_all(dest); + return MOSQ_ERR_NOMEM; + } } break; case MQTT_PROP_AUTHENTICATION_DATA: case MQTT_PROP_CORRELATION_DATA: pnew->value.bin.len = src->value.bin.len; - pnew->value.bin.v = malloc(pnew->value.bin.len); - if(!pnew->value.bin.v){ - mosquitto_property_free_all(dest); - return MOSQ_ERR_NOMEM; + if(src->value.bin.len){ + pnew->value.bin.v = malloc(pnew->value.bin.len); + if(!pnew->value.bin.v){ + mosquitto_property_free_all(dest); + return MOSQ_ERR_NOMEM; + } + memcpy(pnew->value.bin.v, src->value.bin.v, pnew->value.bin.len); } - memcpy(pnew->value.bin.v, src->value.bin.v, pnew->value.bin.len); break; case MQTT_PROP_USER_PROPERTY: pnew->value.s.len = src->value.s.len; - pnew->value.s.v = strdup(src->value.s.v); - if(!pnew->value.s.v){ - mosquitto_property_free_all(dest); - return MOSQ_ERR_NOMEM; + if(src->value.s.v){ + pnew->value.s.v = strdup(src->value.s.v); + if(!pnew->value.s.v){ + mosquitto_property_free_all(dest); + return MOSQ_ERR_NOMEM; + } } pnew->name.len = src->name.len; - pnew->name.v = strdup(src->name.v); - if(!pnew->name.v){ - mosquitto_property_free_all(dest); - return MOSQ_ERR_NOMEM; + if(src->name.v){ + pnew->name.v = strdup(src->name.v); + if(!pnew->name.v){ + mosquitto_property_free_all(dest); + return MOSQ_ERR_NOMEM; + } } break; diff --git a/test/unit/Makefile b/test/unit/Makefile index 2c53615b..8624598e 100644 --- a/test/unit/Makefile +++ b/test/unit/Makefile @@ -106,7 +106,7 @@ SUBS_OBJS = \ topic_tok.o \ utf8_mosq.o \ -all : test +all : test-compile check : test diff --git a/test/unit/property_user_read.c b/test/unit/property_user_read.c index ebaabc12..3b2f8439 100644 --- a/test/unit/property_user_read.c +++ b/test/unit/property_user_read.c @@ -200,9 +200,13 @@ static void read_binary_helper(const mosquitto_property *proplist, int identifie prop = mosquitto_property_read_binary(proplist, identifier, &value, &length, false); CU_ASSERT_PTR_NOT_NULL(prop); CU_ASSERT_EQUAL(length, expected_length); - CU_ASSERT_PTR_NOT_NULL(value); - if(value){ - CU_ASSERT_NSTRING_EQUAL(value, expected_value, expected_length); + if(expected_value){ + CU_ASSERT_PTR_NOT_NULL(value); + if(value){ + CU_ASSERT_NSTRING_EQUAL(value, expected_value, expected_length); + } + }else{ + CU_ASSERT_PTR_NULL(value); } SAFE_FREE(value); } @@ -214,9 +218,13 @@ static void read_string_helper(const mosquitto_property *proplist, int identifie prop = mosquitto_property_read_string(proplist, identifier, &value, false); CU_ASSERT_PTR_NOT_NULL(prop); - CU_ASSERT_PTR_NOT_NULL(value); - if(value){ - CU_ASSERT_STRING_EQUAL(value, expected_value); + if(expected_value){ + CU_ASSERT_PTR_NOT_NULL(value); + if(value){ + CU_ASSERT_STRING_EQUAL(value, expected_value); + } + }else{ + CU_ASSERT_PTR_NULL(value); } SAFE_FREE(value); } @@ -229,20 +237,91 @@ static void read_string_pair_helper(const mosquitto_property *proplist, int iden prop = mosquitto_property_read_string_pair(proplist, identifier, &key, &value, false); CU_ASSERT_PTR_NOT_NULL(prop); - CU_ASSERT_PTR_NOT_NULL(key); - if(key){ - CU_ASSERT_STRING_EQUAL(key, expected_key); + if(expected_key){ + CU_ASSERT_PTR_NOT_NULL(key); + if(key){ + CU_ASSERT_STRING_EQUAL(key, expected_key); + } + }else{ + CU_ASSERT_PTR_NULL(key); } - CU_ASSERT_PTR_NOT_NULL(value); - if(value){ - CU_ASSERT_STRING_EQUAL(value, expected_value); + if(expected_value){ + CU_ASSERT_PTR_NOT_NULL(value); + if(value){ + CU_ASSERT_STRING_EQUAL(value, expected_value); + } + }else{ + CU_ASSERT_PTR_NULL(value); } SAFE_FREE(key); SAFE_FREE(value); } +static void TEST_read_null_binary(void) +{ + int rc; + mosquitto_property *proplist = NULL, *proplist_copy = NULL; + + rc = mosquitto_property_add_binary(&proplist, MQTT_PROP_CORRELATION_DATA, NULL, 0); + CU_ASSERT_EQUAL(rc, MOSQ_ERR_SUCCESS); + if(rc != MOSQ_ERR_SUCCESS) return; + + read_binary_helper(proplist, MQTT_PROP_CORRELATION_DATA, NULL, 0); + + rc = mosquitto_property_copy_all(&proplist_copy, proplist); + CU_ASSERT_EQUAL(rc, MOSQ_ERR_SUCCESS); + CU_ASSERT_PTR_NOT_NULL(proplist_copy); + + read_binary_helper(proplist_copy, MQTT_PROP_CORRELATION_DATA, NULL, 0); + + mosquitto_property_free_all(&proplist); + mosquitto_property_free_all(&proplist_copy); +} + +static void TEST_read_null_string(void) +{ + int rc; + mosquitto_property *proplist = NULL, *proplist_copy = NULL; + + rc = mosquitto_property_add_string(&proplist, MQTT_PROP_CONTENT_TYPE, NULL); + CU_ASSERT_EQUAL(rc, MOSQ_ERR_SUCCESS); + if(rc != MOSQ_ERR_SUCCESS) return; + + read_string_helper(proplist, MQTT_PROP_CONTENT_TYPE, NULL); + + rc = mosquitto_property_copy_all(&proplist_copy, proplist); + CU_ASSERT_EQUAL(rc, MOSQ_ERR_SUCCESS); + CU_ASSERT_PTR_NOT_NULL(proplist_copy); + + read_string_helper(proplist_copy, MQTT_PROP_CONTENT_TYPE, NULL); + + mosquitto_property_free_all(&proplist); + mosquitto_property_free_all(&proplist_copy); +} + +static void TEST_read_null_string_pair(void) +{ + int rc; + mosquitto_property *proplist = NULL, *proplist_copy = NULL; + + rc = mosquitto_property_add_string_pair(&proplist, MQTT_PROP_USER_PROPERTY, NULL, NULL); + CU_ASSERT_EQUAL(rc, MOSQ_ERR_SUCCESS); + if(rc != MOSQ_ERR_SUCCESS) return; + + read_string_pair_helper(proplist, MQTT_PROP_USER_PROPERTY, NULL, NULL); + + rc = mosquitto_property_copy_all(&proplist_copy, proplist); + CU_ASSERT_EQUAL(rc, MOSQ_ERR_SUCCESS); + CU_ASSERT_PTR_NOT_NULL(proplist_copy); + + read_string_pair_helper(proplist_copy, MQTT_PROP_USER_PROPERTY, NULL, NULL); + + mosquitto_property_free_all(&proplist); + mosquitto_property_free_all(&proplist_copy); +} + static void TEST_read_single_byte(void) { int rc; @@ -621,6 +700,9 @@ int init_property_user_read_tests(void) || !CU_add_test(test_suite, "Read single string", TEST_read_single_string) || !CU_add_test(test_suite, "Read single string pair", TEST_read_single_string_pair) || !CU_add_test(test_suite, "Read missing", TEST_read_missing) + || !CU_add_test(test_suite, "Read NULL binary", TEST_read_null_binary) + || !CU_add_test(test_suite, "Read NULL string", TEST_read_null_string) + || !CU_add_test(test_suite, "Read NULL string pair", TEST_read_null_string_pair) || !CU_add_test(test_suite, "String to property info", TEST_string_to_property_info) ){