Skip to content

Fix: Handle encoded CBOR message exceeding buffer size #152

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 7 commits into from
Jul 6, 2020
57 changes: 56 additions & 1 deletion extras/test/src/test_encode.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@
TEST CODE
**************************************************************************************/

SCENARIO("Arduino Cloud Properties are encoded", "[ArduinoCloudThing::encode]") {
SCENARIO("Arduino Cloud Properties are encoded", "[ArduinoCloudThing::encode-1]") {
/************************************************************************************/

WHEN("A 'bool' property is added") {
Expand Down Expand Up @@ -381,4 +381,59 @@ SCENARIO("Arduino Cloud Properties are encoded", "[ArduinoCloudThing::encode]")
}

/************************************************************************************/

WHEN("The size of the encoded properties is exceeding the CBOR buffer size") {
GIVEN("CloudProtocol::V2") {
PropertyContainer property_container;

CloudString str_0; str_0 = "This string is 30 bytes long.";
CloudString str_1; str_1 = "This string is 30 bytes long.";
CloudString str_2; str_2 = "This string is 30 bytes long.";
CloudString str_3; str_3 = "This string is 30 bytes long.";
CloudString str_4; str_4 = "This string is 30 bytes long.";
CloudString str_5; str_5 = "This string is 30 bytes long.";
CloudString str_6; str_6 = "This string is 30 bytes long.";
CloudString str_7; str_7 = "This string is 30 bytes long.";
CloudString str_8; str_8 = "This string is 30 bytes long.";
CloudString str_9; str_9 = "This string is 30 bytes long.";

addPropertyToContainer(property_container, str_0, "str_0", Permission::ReadWrite);
addPropertyToContainer(property_container, str_1, "str_1", Permission::ReadWrite);
addPropertyToContainer(property_container, str_2, "str_2", Permission::ReadWrite);
addPropertyToContainer(property_container, str_3, "str_3", Permission::ReadWrite);
addPropertyToContainer(property_container, str_4, "str_4", Permission::ReadWrite);
addPropertyToContainer(property_container, str_5, "str_5", Permission::ReadWrite);
addPropertyToContainer(property_container, str_6, "str_6", Permission::ReadWrite);
addPropertyToContainer(property_container, str_7, "str_7", Permission::ReadWrite);
addPropertyToContainer(property_container, str_8, "str_8", Permission::ReadWrite);
addPropertyToContainer(property_container, str_9, "str_9", Permission::ReadWrite);

/* Due to the size if the encoded properties exceeding 256 bytes if encoded all at
* once they are encoded in subsequent calls to CBOREncoder::encode.
*/

/* [{0: "str_0", 3: "This string is 30 bytes long."}, {0: "str_1", 3: "This string is 30 bytes long."}, {0: "str_2", 3: "This string is 30 bytes long."}, {0: "str_3", 3: "This string is 30 bytes long."}]
* = 9F A2 00 65 73 74 72 5F 30 03 78 1D 54 68 69 73 20 73 74 72 69 6E 67 20 69 73 20 33 30 20 62 79 74 65 73 20 6C 6F 6E 67 2E A2 00 65 73 74 72 5F 31 03 78 1D 54 68 69 73 20 73 74 72 69 6E 67 20 69 73 20 33 30 20 62 79 74 65 73 20 6C 6F 6E 67 2E A2 00 65 73 74 72 5F 32 03 78 1D 54 68 69 73 20 73 74 72 69 6E 67 20 69 73 20 33 30 20 62 79 74 65 73 20 6C 6F 6E 67 2E A2 00 65 73 74 72 5F 33 03 78 1D 54 68 69 73 20 73 74 72 69 6E 67 20 69 73 20 33 30 20 62 79 74 65 73 20 6C 6F 6E 67 2E FF
*/
std::vector<uint8_t> const expected_1 = {0x9F, 0xA2, 0x00, 0x65, 0x73, 0x74, 0x72, 0x5F, 0x30, 0x03, 0x78, 0x1D, 0x54, 0x68, 0x69, 0x73, 0x20, 0x73, 0x74, 0x72, 0x69, 0x6E, 0x67, 0x20, 0x69, 0x73, 0x20, 0x33, 0x30, 0x20, 0x62, 0x79, 0x74, 0x65, 0x73, 0x20, 0x6C, 0x6F, 0x6E, 0x67, 0x2E, 0xA2, 0x00, 0x65, 0x73, 0x74, 0x72, 0x5F, 0x31, 0x03, 0x78, 0x1D, 0x54, 0x68, 0x69, 0x73, 0x20, 0x73, 0x74, 0x72, 0x69, 0x6E, 0x67, 0x20, 0x69, 0x73, 0x20, 0x33, 0x30, 0x20, 0x62, 0x79, 0x74, 0x65, 0x73, 0x20, 0x6C, 0x6F, 0x6E, 0x67, 0x2E, 0xA2, 0x00, 0x65, 0x73, 0x74, 0x72, 0x5F, 0x32, 0x03, 0x78, 0x1D, 0x54, 0x68, 0x69, 0x73, 0x20, 0x73, 0x74, 0x72, 0x69, 0x6E, 0x67, 0x20, 0x69, 0x73, 0x20, 0x33, 0x30, 0x20, 0x62, 0x79, 0x74, 0x65, 0x73, 0x20, 0x6C, 0x6F, 0x6E, 0x67, 0x2E, 0xA2, 0x00, 0x65, 0x73, 0x74, 0x72, 0x5F, 0x33, 0x03, 0x78, 0x1D, 0x54, 0x68, 0x69, 0x73, 0x20, 0x73, 0x74, 0x72, 0x69, 0x6E, 0x67, 0x20, 0x69, 0x73, 0x20, 0x33, 0x30, 0x20, 0x62, 0x79, 0x74, 0x65, 0x73, 0x20, 0x6C, 0x6F, 0x6E, 0x67, 0x2E, 0xFF};
std::vector<uint8_t> const actual_1 = cbor::encode(property_container);
REQUIRE(actual_1 == expected_1);

/* [{0: "str_4", 3: "This string is 30 bytes long."}, {0: "str_5", 3: "This string is 30 bytes long."}, {0: "str_6", 3: "This string is 30 bytes long."}, {0: "str_7", 3: "This string is 30 bytes long."}]
* = 9F A2 00 65 73 74 72 5F 34 03 78 1D 54 68 69 73 20 73 74 72 69 6E 67 20 69 73 20 33 30 20 62 79 74 65 73 20 6C 6F 6E 67 2E A2 00 65 73 74 72 5F 35 03 78 1D 54 68 69 73 20 73 74 72 69 6E 67 20 69 73 20 33 30 20 62 79 74 65 73 20 6C 6F 6E 67 2E A2 00 65 73 74 72 5F 36 03 78 1D 54 68 69 73 20 73 74 72 69 6E 67 20 69 73 20 33 30 20 62 79 74 65 73 20 6C 6F 6E 67 2E A2 00 65 73 74 72 5F 37 03 78 1D 54 68 69 73 20 73 74 72 69 6E 67 20 69 73 20 33 30 20 62 79 74 65 73 20 6C 6F 6E 67 2E FF
*/
std::vector<uint8_t> const expected_2 = {0x9F, 0xA2, 0x00, 0x65, 0x73, 0x74, 0x72, 0x5F, 0x34, 0x03, 0x78, 0x1D, 0x54, 0x68, 0x69, 0x73, 0x20, 0x73, 0x74, 0x72, 0x69, 0x6E, 0x67, 0x20, 0x69, 0x73, 0x20, 0x33, 0x30, 0x20, 0x62, 0x79, 0x74, 0x65, 0x73, 0x20, 0x6C, 0x6F, 0x6E, 0x67, 0x2E, 0xA2, 0x00, 0x65, 0x73, 0x74, 0x72, 0x5F, 0x35, 0x03, 0x78, 0x1D, 0x54, 0x68, 0x69, 0x73, 0x20, 0x73, 0x74, 0x72, 0x69, 0x6E, 0x67, 0x20, 0x69, 0x73, 0x20, 0x33, 0x30, 0x20, 0x62, 0x79, 0x74, 0x65, 0x73, 0x20, 0x6C, 0x6F, 0x6E, 0x67, 0x2E, 0xA2, 0x00, 0x65, 0x73, 0x74, 0x72, 0x5F, 0x36, 0x03, 0x78, 0x1D, 0x54, 0x68, 0x69, 0x73, 0x20, 0x73, 0x74, 0x72, 0x69, 0x6E, 0x67, 0x20, 0x69, 0x73, 0x20, 0x33, 0x30, 0x20, 0x62, 0x79, 0x74, 0x65, 0x73, 0x20, 0x6C, 0x6F, 0x6E, 0x67, 0x2E, 0xA2, 0x00, 0x65, 0x73, 0x74, 0x72, 0x5F, 0x37, 0x03, 0x78, 0x1D, 0x54, 0x68, 0x69, 0x73, 0x20, 0x73, 0x74, 0x72, 0x69, 0x6E, 0x67, 0x20, 0x69, 0x73, 0x20, 0x33, 0x30, 0x20, 0x62, 0x79, 0x74, 0x65, 0x73, 0x20, 0x6C, 0x6F, 0x6E, 0x67, 0x2E, 0xFF};
std::vector<uint8_t> const actual_2 = cbor::encode(property_container);
REQUIRE(actual_2 == expected_2);

/* [{0: "str_8", 3: "This string is 30 bytes long."}, {0: "str_9", 3: "This string is 30 bytes long."}]
* = 9F A2 00 65 73 74 72 5F 38 03 78 1D 54 68 69 73 20 73 74 72 69 6E 67 20 69 73 20 33 30 20 62 79 74 65 73 20 6C 6F 6E 67 2E A2 00 65 73 74 72 5F 39 03 78 1D 54 68 69 73 20 73 74 72 69 6E 67 20 69 73 20 33 30 20 62 79 74 65 73 20 6C 6F 6E 67 2E FF
*/
std::vector<uint8_t> const expected_3 = {0x9F, 0xA2, 0x00, 0x65, 0x73, 0x74, 0x72, 0x5F, 0x38, 0x03, 0x78, 0x1D, 0x54, 0x68, 0x69, 0x73, 0x20, 0x73, 0x74, 0x72, 0x69, 0x6E, 0x67, 0x20, 0x69, 0x73, 0x20, 0x33, 0x30, 0x20, 0x62, 0x79, 0x74, 0x65, 0x73, 0x20, 0x6C, 0x6F, 0x6E, 0x67, 0x2E, 0xA2, 0x00, 0x65, 0x73, 0x74, 0x72, 0x5F, 0x39, 0x03, 0x78, 0x1D, 0x54, 0x68, 0x69, 0x73, 0x20, 0x73, 0x74, 0x72, 0x69, 0x6E, 0x67, 0x20, 0x69, 0x73, 0x20, 0x33, 0x30, 0x20, 0x62, 0x79, 0x74, 0x65, 0x73, 0x20, 0x6C, 0x6F, 0x6E, 0x67, 0x2E, 0xFF};
std::vector<uint8_t> const actual_3 = cbor::encode(property_container);
REQUIRE(actual_3 == expected_3);
}
}

/************************************************************************************/
}
10 changes: 5 additions & 5 deletions extras/test/src/util/CBORTestUtil.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -25,13 +25,13 @@ namespace cbor
**************************************************************************************/

std::vector<uint8_t> encode(PropertyContainer & property_container, bool lightPayload) {
int bytes_encoded = 0;
uint8_t buf[200] = {0};
int const bytes_buf = CBOREncoder::encode(property_container, buf, 200, lightPayload);
if (bytes_buf == -1) {

if (CBOREncoder::encode(property_container, buf, 200, bytes_encoded, lightPayload) == CborNoError)
return std::vector<uint8_t>(buf, buf + bytes_encoded);
else
return std::vector<uint8_t>();
} else {
return std::vector<uint8_t>(buf, buf + bytes_buf);
}
}

void print(std::vector<uint8_t> const & vect) {
Expand Down
9 changes: 5 additions & 4 deletions src/ArduinoIoTCloudLPWAN.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -126,11 +126,12 @@ void ArduinoIoTCloudLPWAN::disconnect()

void ArduinoIoTCloudLPWAN::sendPropertiesToCloud()
{
int bytes_encoded = 0;
uint8_t data[CBOR_LORA_MSG_MAX_SIZE];
int const length = CBOREncoder::encode(_property_container, data, sizeof(data), true);
if (length > 0) {
writeProperties(data, length);
}

if (CBOREncoder::encode(_property_container, data, sizeof(data), bytes_encoded, true) == CborNoError)
if (bytes_encoded > 0)
writeProperties(data, bytes_encoded);
}

int ArduinoIoTCloudLPWAN::writeProperties(const byte data[], int length)
Expand Down
24 changes: 13 additions & 11 deletions src/ArduinoIoTCloudTCP.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -283,18 +283,20 @@ void ArduinoIoTCloudTCP::handleMessage(int length)

void ArduinoIoTCloudTCP::sendPropertiesToCloud()
{
int bytes_encoded = 0;
uint8_t data[MQTT_TRANSMIT_BUFFER_SIZE];
int const length = CBOREncoder::encode(_property_container, data, sizeof(data));
if (length > 0)
{
/* If properties have been encoded store them in the back-up buffer
* in order to allow retransmission in case of failure.
*/
_mqtt_data_len = length;
memcpy(_mqtt_data_buf, data, _mqtt_data_len);
/* Transmit the properties to the MQTT broker */
write(_dataTopicOut, _mqtt_data_buf, _mqtt_data_len);
}

if (CBOREncoder::encode(_property_container, data, sizeof(data), bytes_encoded, false) == CborNoError)
if (bytes_encoded > 0)
{
/* If properties have been encoded store them in the back-up buffer
* in order to allow retransmission in case of failure.
*/
_mqtt_data_len = bytes_encoded;
memcpy(_mqtt_data_buf, data, _mqtt_data_len);
/* Transmit the properties to the MQTT broker */
write(_dataTopicOut, _mqtt_data_buf, _mqtt_data_len);
}
}

void ArduinoIoTCloudTCP::requestLastValue()
Expand Down
40 changes: 31 additions & 9 deletions src/cbor/CBOREncoder.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -21,31 +21,53 @@

#include "CBOREncoder.h"

#undef max
#undef min
#include <algorithm>

#include "lib/tinycbor/cbor-lib.h"

/******************************************************************************
* PUBLIC MEMBER FUNCTIONS
******************************************************************************/

int CBOREncoder::encode(PropertyContainer & property_container, uint8_t * data, size_t const size, bool lightPayload)
CborError CBOREncoder::encode(PropertyContainer & property_container, uint8_t * data, size_t const size, int & bytes_encoded, bool lightPayload)
{
CborEncoder encoder, arrayEncoder;

cbor_encoder_init(&encoder, data, size, 0);

if (cbor_encoder_create_array(&encoder, &arrayEncoder, CborIndefiniteLength) != CborNoError)
return -1;
CHECK_CBOR(cbor_encoder_create_array(&encoder, &arrayEncoder, CborIndefiniteLength));

/* Check if backing storage and cloud has diverged
* time interval may be elapsed or property may be changed
* and if that's the case encode the property into the CBOR.
*/
if (appendChangedProperties(property_container, &arrayEncoder, lightPayload) < 1)
return -1;
CborError error = CborNoError;
int num_encoded_properties = 0;
std::for_each(property_container.begin(),
property_container.end(),
[lightPayload, &arrayEncoder, &error, &num_encoded_properties](Property * p)
{
if (p->shouldBeUpdated() && p->isReadableByCloud())
{
error = p->append(&arrayEncoder, lightPayload);
if(error == CborNoError)
num_encoded_properties++;
else
return;
}
});
if ((CborNoError != error) &&
(CborErrorOutOfMemory != error))
return error;

CHECK_CBOR(cbor_encoder_close_container(&encoder, &arrayEncoder));

if (cbor_encoder_close_container(&encoder, &arrayEncoder) != CborNoError)
return -1;
if (num_encoded_properties > 0)
bytes_encoded = cbor_encoder_get_buffer_size(&encoder, data);
else
bytes_encoded = 0;

int const bytes_encoded = cbor_encoder_get_buffer_size(&encoder, data);
return bytes_encoded;
return CborNoError;
}
2 changes: 1 addition & 1 deletion src/cbor/CBOREncoder.h
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ class CBOREncoder

/* encode return > 0 if a property has changed and encodes the changed properties in CBOR format into the provided buffer */
/* if lightPayload is true the integer identifier of the property will be encoded in the message instead of the property name in order to reduce the size of the message payload*/
static int encode(PropertyContainer & property_container, uint8_t * data, size_t const size, bool lightPayload = false);
static CborError encode(PropertyContainer & property_container, uint8_t * data, size_t const size, int & bytes_encoded, bool lightPayload = false);

private:

Expand Down
18 changes: 18 additions & 0 deletions src/cbor/lib/tinycbor/cbor-lib.h
Original file line number Diff line number Diff line change
@@ -1,6 +1,24 @@
#ifndef CBOR_LIB_H
#define CBOR_LIB_H

/******************************************************************************
INCLUDE
******************************************************************************/

#include "src/cbor.h"

/******************************************************************************
* DEFINE
******************************************************************************/

#ifndef CHECK_CBOR
#define CHECK_CBOR(expr) \
do { \
CborError error = CborNoError; \
error = (expr); \
if (CborNoError != error) \
return error; \
} while(0);
#endif /* CHECK_CBOR */

#endif
72 changes: 43 additions & 29 deletions src/property/Property.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -142,78 +142,92 @@ void Property::execCallbackOnSync() {
}
}

void Property::append(CborEncoder *encoder, bool lightPayload) {
CborError Property::append(CborEncoder *encoder, bool lightPayload) {
_lightPayload = lightPayload;
_attributeIdentifier = 0;
appendAttributesToCloudReal(encoder);
CHECK_CBOR(appendAttributesToCloudReal(encoder));
fromLocalToCloud();
_has_been_updated_once = true;
_update_requested = false;
_last_updated_millis = millis();
return CborNoError;
}

void Property::appendAttributeReal(bool value, String attributeName, CborEncoder *encoder) {
appendAttributeName(attributeName, [value](CborEncoder & mapEncoder) {
cbor_encode_int(&mapEncoder, static_cast<int>(CborIntegerMapKey::BooleanValue));
cbor_encode_boolean(&mapEncoder, value);
CborError Property::appendAttributeReal(bool value, String attributeName, CborEncoder *encoder) {
return appendAttributeName(attributeName, [value](CborEncoder & mapEncoder)
{
CHECK_CBOR(cbor_encode_int(&mapEncoder, static_cast<int>(CborIntegerMapKey::BooleanValue)));
CHECK_CBOR(cbor_encode_boolean(&mapEncoder, value));
return CborNoError;
}, encoder);
}

void Property::appendAttributeReal(int value, String attributeName, CborEncoder *encoder) {
appendAttributeName(attributeName, [value](CborEncoder & mapEncoder) {
cbor_encode_int(&mapEncoder, static_cast<int>(CborIntegerMapKey::Value));
cbor_encode_int(&mapEncoder, value);
CborError Property::appendAttributeReal(int value, String attributeName, CborEncoder *encoder) {
return appendAttributeName(attributeName, [value](CborEncoder & mapEncoder)
{
CHECK_CBOR(cbor_encode_int(&mapEncoder, static_cast<int>(CborIntegerMapKey::Value)));
CHECK_CBOR(cbor_encode_int(&mapEncoder, value));
return CborNoError;
}, encoder);
}

void Property::appendAttributeReal(float value, String attributeName, CborEncoder *encoder) {
appendAttributeName(attributeName, [value](CborEncoder & mapEncoder) {
cbor_encode_int(&mapEncoder, static_cast<int>(CborIntegerMapKey::Value));
cbor_encode_float(&mapEncoder, value);
CborError Property::appendAttributeReal(float value, String attributeName, CborEncoder *encoder) {
return appendAttributeName(attributeName, [value](CborEncoder & mapEncoder)
{
CHECK_CBOR(cbor_encode_int(&mapEncoder, static_cast<int>(CborIntegerMapKey::Value)));
CHECK_CBOR(cbor_encode_float(&mapEncoder, value));
return CborNoError;
}, encoder);
}

void Property::appendAttributeReal(String value, String attributeName, CborEncoder *encoder) {
appendAttributeName(attributeName, [value](CborEncoder & mapEncoder) {
cbor_encode_int(&mapEncoder, static_cast<int>(CborIntegerMapKey::StringValue));
cbor_encode_text_stringz(&mapEncoder, value.c_str());
CborError Property::appendAttributeReal(String value, String attributeName, CborEncoder *encoder) {
return appendAttributeName(attributeName, [value](CborEncoder & mapEncoder)
{
CHECK_CBOR(cbor_encode_int(&mapEncoder, static_cast<int>(CborIntegerMapKey::StringValue)));
CHECK_CBOR(cbor_encode_text_stringz(&mapEncoder, value.c_str()));
return CborNoError;
}, encoder);
}

void Property::appendAttributeName(String attributeName, std::function<void (CborEncoder& mapEncoder)>appendValue, CborEncoder *encoder) {
CborError Property::appendAttributeName(String attributeName, std::function<CborError (CborEncoder& mapEncoder)>appendValue, CborEncoder *encoder) {
if (attributeName != "") {
// when the attribute name string is not empty, the attribute identifier is incremented in order to be encoded in the message if the _lightPayload flag is set
_attributeIdentifier++;
}
CborEncoder mapEncoder;
unsigned int num_map_properties = _encode_timestamp ? 3 : 2;
cbor_encoder_create_map(encoder, &mapEncoder, num_map_properties);
cbor_encode_int(&mapEncoder, static_cast<int>(CborIntegerMapKey::Name));
CHECK_CBOR(cbor_encoder_create_map(encoder, &mapEncoder, num_map_properties));
CHECK_CBOR(cbor_encode_int(&mapEncoder, static_cast<int>(CborIntegerMapKey::Name)));

// if _lightPayload is true, the property and attribute identifiers will be encoded instead of the property name
if (_lightPayload) {
if (_lightPayload)
{
// the most significant byte of the identifier to be encoded represent the property identifier
int completeIdentifier = _attributeIdentifier * 256;
// the least significant byte of the identifier to be encoded represent the attribute identifier
completeIdentifier += _identifier;
cbor_encode_int(&mapEncoder, completeIdentifier);
} else {
CHECK_CBOR(cbor_encode_int(&mapEncoder, completeIdentifier));
}
else
{
String completeName = _name;
if (attributeName != "") {
completeName += ":" + attributeName;
}
cbor_encode_text_stringz(&mapEncoder, completeName.c_str());
CHECK_CBOR(cbor_encode_text_stringz(&mapEncoder, completeName.c_str()));
}
/* Encode the value */
appendValue(mapEncoder);
CHECK_CBOR(appendValue(mapEncoder));

/* Encode the timestamp if that has been required. */
if(_encode_timestamp)
{
cbor_encode_int (&mapEncoder, static_cast<int>(CborIntegerMapKey::Time));
cbor_encode_uint(&mapEncoder, _timestamp);
CHECK_CBOR(cbor_encode_int (&mapEncoder, static_cast<int>(CborIntegerMapKey::Time)));
CHECK_CBOR(cbor_encode_uint(&mapEncoder, _timestamp));
}
/* Close the container */
cbor_encoder_close_container(encoder, &mapEncoder);
CHECK_CBOR(cbor_encoder_close_container(encoder, &mapEncoder));
return CborNoError;
}

void Property::setAttributesFromCloud(std::list<CborMapData> * map_data_list) {
Expand Down
Loading