diff --git a/src/bacnet/bacapp.c b/src/bacnet/bacapp.c index adecab4e..a97aba58 100644 --- a/src/bacnet/bacapp.c +++ b/src/bacnet/bacapp.c @@ -1531,7 +1531,7 @@ bool bacapp_copy(BACNET_APPLICATION_DATA_VALUE *dest_value, * @param apdu_len_max Bytes valid in the buffer * @param property ID of the property to get the length for. * - * @return Length in bytes or BACNET_STATUS_ERROR. + * @return Length in bytes 0..N, or BACNET_STATUS_ERROR. */ int bacapp_data_len( uint8_t *apdu, unsigned apdu_len_max, BACNET_PROPERTY_ID property) diff --git a/src/bacnet/wp.c b/src/bacnet/wp.c index ee9d30fd..2195539e 100644 --- a/src/bacnet/wp.c +++ b/src/bacnet/wp.c @@ -40,9 +40,22 @@ /** @file wp.c Encode/Decode BACnet Write Property APDUs */ #if BACNET_SVC_WP_A -/** Initialize the APDU for encode service. +/** + * @brief Initialize the APDU for encode service. * - * @param apdu Pointer to the buffer. + * WriteProperty-Request ::= SEQUENCE { + * object-identifier [0] BACnetObjectIdentifier, + * property-identifier [1] BACnetPropertyIdentifier, + * property-array-index [2] Unsigned OPTIONAL, + * -- used only with array datatype + * -- if omitted with an array the entire + * -- array is referenced + * property-value [3] ABSTRACT-SYNTAX.&Type, + * priority [4] Unsigned (1..16) OPTIONAL + * -– used only when property is commandable + * } + * + * @param apdu Pointer to the buffer, or NULL for length * @param invoke_id ID of service invoked. * @param wpdata Pointer to the write property data. * @@ -55,66 +68,96 @@ int wp_encode_apdu( int len = 0; /* total length of the apdu, return value */ int imax = 0; /* maximum application data length */ + if (!wpdata) { + return BACNET_STATUS_ERROR; + } if (apdu) { apdu[0] = PDU_TYPE_CONFIRMED_SERVICE_REQUEST; apdu[1] = encode_max_segs_max_apdu(0, MAX_APDU); apdu[2] = invoke_id; apdu[3] = SERVICE_CONFIRMED_WRITE_PROPERTY; /* service choice */ - apdu_len = 4; - len = encode_context_object_id( - &apdu[apdu_len], 0, wpdata->object_type, wpdata->object_instance); + } + len = 4; + apdu_len += len; + if (apdu) { + apdu += len; + } + len = encode_context_object_id( + apdu, 0, wpdata->object_type, wpdata->object_instance); + apdu_len += len; + if (apdu) { + apdu += len; + } + len = encode_context_enumerated(apdu, 1, wpdata->object_property); + apdu_len += len; + if (apdu) { + apdu += len; + } + /* optional array index; ALL is -1 which is assumed when missing */ + if (wpdata->array_index != BACNET_ARRAY_ALL) { + len = encode_context_unsigned(apdu, 2, wpdata->array_index); apdu_len += len; - len = encode_context_enumerated( - &apdu[apdu_len], 1, wpdata->object_property); + if (apdu) { + apdu += len; + } + } + /* propertyValue */ + len = encode_opening_tag(apdu, 3); + apdu_len += len; + if (apdu) { + apdu += len; + } + for (len = 0; len < wpdata->application_data_len; len++) { + if (apdu) { + *apdu = wpdata->application_data[len]; + apdu += 1; + } + apdu_len++; + } + len = encode_closing_tag(apdu, 3); + apdu_len += len; + if (apdu) { + apdu += len; + } + /* optional priority - 0 if not set, 1..16 if set */ + if (wpdata->priority != BACNET_NO_PRIORITY) { + len = encode_context_unsigned(apdu, 4, wpdata->priority); apdu_len += len; - /* optional array index; ALL is -1 which is assumed when missing */ - if (wpdata->array_index != BACNET_ARRAY_ALL) { - len = encode_context_unsigned( - &apdu[apdu_len], 2, wpdata->array_index); - apdu_len += len; - } - /* propertyValue */ - len = encode_opening_tag(&apdu[apdu_len], 3); - apdu_len += len; - imax = wpdata->application_data_len; - if (imax > (MAX_APDU - 2 /*closing*/ - apdu_len)) { - imax = MAX_APDU - 2 - apdu_len; - } - for (len = 0; len < imax; len++) { - apdu[apdu_len + len] = wpdata->application_data[len]; - } - apdu_len += imax; - len = encode_closing_tag(&apdu[apdu_len], 3); - apdu_len += len; - /* optional priority - 0 if not set, 1..16 if set */ - if (wpdata->priority != BACNET_NO_PRIORITY) { - len = encode_context_unsigned(&apdu[apdu_len], 4, wpdata->priority); - apdu_len += len; - } } return apdu_len; } #endif -/** Decode the service request only +/** + * @brief Decode the service request only * - * FIXME: there could be various error messages returned - * using unique values less than zero. + * WriteProperty-Request ::= SEQUENCE { + * object-identifier [0] BACnetObjectIdentifier, + * property-identifier [1] BACnetPropertyIdentifier, + * property-array-index [2] Unsigned OPTIONAL, + * -- used only with array datatype + * -- if omitted with an array the entire + * -- array is referenced + * property-value [3] ABSTRACT-SYNTAX.&Type, + * priority [4] Unsigned (1..16) OPTIONAL + * -– used only when property is commandable + * } * * @param apdu Pointer to the buffer. - * @param apdu_len Valid bytes in the buffer + * @param apdu_size Valid bytes in the buffer * @param wpdata Pointer to the write property data. * - * @return Bytes encoded or a negative value as error. + * @return number of bytes decoded, or #BACNET_STATUS_ERROR */ int wp_decode_service_request( - uint8_t *apdu, unsigned apdu_len, BACNET_WRITE_PROPERTY_DATA *wpdata) + uint8_t *apdu, unsigned apdu_size, BACNET_WRITE_PROPERTY_DATA *wpdata) { int len = 0; + int apdu_len = 0; int tag_len = 0; uint8_t tag_number = 0; - uint32_t len_value_type = 0; + uint32_t instance = 0; BACNET_OBJECT_TYPE type = OBJECT_NONE; /* for decoding */ uint32_t property = 0; /* for decoding */ BACNET_UNSIGNED_INTEGER unsigned_value = 0; @@ -122,80 +165,104 @@ int wp_decode_service_request( int imax = 0; /* max application data length */ /* check for value pointers */ - if (apdu_len && wpdata) { - /* Tag 0: Object ID */ - if (!decode_is_context_tag(&apdu[len++], 0)) { - return -1; + if (!apdu) { + return BACNET_STATUS_ERROR; + } + /* object-identifier [0] BACnetObjectIdentifier */ + len = bacnet_object_id_context_decode( + &apdu[apdu_len], apdu_size - apdu_len, 0, &type, &instance); + if (len > 0) { + if (instance > BACNET_MAX_INSTANCE) { + return BACNET_STATUS_ERROR; } - len += decode_object_id(&apdu[len], &type, &wpdata->object_instance); - wpdata->object_type = type; - /* Tag 1: Property ID */ - len += decode_tag_number_and_value( - &apdu[len], &tag_number, &len_value_type); - if (tag_number != 1) { - return -1; + apdu_len += len; + if (wpdata) { + wpdata->object_type = type; + wpdata->object_instance = instance; } - len += decode_enumerated(&apdu[len], len_value_type, &property); - wpdata->object_property = (BACNET_PROPERTY_ID)property; - /* Tag 2: Optional Array Index */ - /* note: decode without incrementing len so we can check for opening tag - */ - tag_len = decode_tag_number_and_value( - &apdu[len], &tag_number, &len_value_type); - if (tag_number == 2) { - len += tag_len; - len += decode_unsigned(&apdu[len], len_value_type, &unsigned_value); + } else { + return BACNET_STATUS_ERROR; + } + /* property-identifier [1] BACnetPropertyIdentifier */ + len = bacnet_enumerated_context_decode( + &apdu[apdu_len], apdu_size - apdu_len, 1, &property); + if (len > 0) { + apdu_len += len; + if (wpdata) { + wpdata->object_property = (BACNET_PROPERTY_ID)property; + } + } else { + return BACNET_STATUS_ERROR; + } + /* property-array-index [2] Unsigned OPTIONAL */ + len = bacnet_unsigned_context_decode( + &apdu[apdu_len], apdu_size - apdu_len, 2, &unsigned_value); + if (len > 0) { + apdu_len += len; + if (wpdata) { wpdata->array_index = unsigned_value; - } else { + } + } else { + /* wrong tag - skip apdu_len increment and go to next field */ + if (wpdata) { wpdata->array_index = BACNET_ARRAY_ALL; } - /* Tag 3: opening context tag */ - if (!decode_is_opening_tag_number(&apdu[len], 3)) { - return -1; - } - /* determine the length of the data blob */ - imax = bacapp_data_len( - &apdu[len], apdu_len - len, (BACNET_PROPERTY_ID)property); - if (imax == BACNET_STATUS_ERROR) { - return -2; - } - /* a tag number of 3 is not extended so only one octet */ - len++; - /* copy the data from the APDU */ - if (imax > (MAX_APDU - len - 1 /*closing*/)) { - imax = (MAX_APDU - len - 1); - } + } + /* property-value [3] ABSTRACT-SYNTAX.&Type */ + if (!bacnet_is_opening_tag_number( + &apdu[apdu_len], apdu_size - apdu_len, 3, &len)) { + return BACNET_STATUS_ERROR; + } + /* determine the length of the data blob */ + imax = bacapp_data_len( + &apdu[apdu_len], apdu_size - apdu_len, (BACNET_PROPERTY_ID)property); + if (imax == BACNET_STATUS_ERROR) { + return BACNET_STATUS_ERROR; + } + /* count the opening tag number length */ + apdu_len += len; + /* copy the data from the APDU */ + if (imax > MAX_APDU) { + /* not enough size in application_data to store the data chunk */ + return BACNET_STATUS_ERROR; + } else if (wpdata) { for (i = 0; i < imax; i++) { - wpdata->application_data[i] = apdu[len + i]; + wpdata->application_data[i] = apdu[apdu_len + i]; } wpdata->application_data_len = imax; - /* add on the data length */ - len += imax; - if (!decode_is_closing_tag_number(&apdu[len], 3)) { - return -2; - } - /* a tag number of 3 is not extended so only one octet */ - len++; - /* Tag 4: optional Priority - assumed MAX if not explicitly set */ + } + /* add on the data length */ + apdu_len += imax; + if (!bacnet_is_closing_tag_number( + &apdu[apdu_len], apdu_size - apdu_len, 3, &len)) { + return BACNET_STATUS_ERROR; + } + /* count the closing tag number length */ + apdu_len += len; + /* priority [4] Unsigned (1..16) OPTIONAL */ + /* assumed MAX priority if not explicitly set */ + if (wpdata) { wpdata->priority = BACNET_MAX_PRIORITY; - if ((unsigned)len < apdu_len) { - tag_len = decode_tag_number_and_value( - &apdu[len], &tag_number, &len_value_type); - if (tag_number == 4) { - len += tag_len; - len = decode_unsigned( - &apdu[len], len_value_type, &unsigned_value); - if ((unsigned_value >= BACNET_MIN_PRIORITY) && - (unsigned_value <= BACNET_MAX_PRIORITY)) { + } + if ((unsigned)apdu_len < apdu_size) { + len = bacnet_unsigned_context_decode( + &apdu[apdu_len], apdu_len - apdu_size, 4, &unsigned_value); + if (len > 0) { + apdu_len += len; + if ((unsigned_value >= BACNET_MIN_PRIORITY) && + (unsigned_value <= BACNET_MAX_PRIORITY)) { + if (wpdata) { wpdata->priority = (uint8_t)unsigned_value; - } else { - return -5; } + } else { + return BACNET_STATUS_ERROR; } + } else { + return BACNET_STATUS_ERROR; } } - return len; + return apdu_len; } /** diff --git a/test/CMakeLists.txt b/test/CMakeLists.txt index a23b0d47..56417dfc 100644 --- a/test/CMakeLists.txt +++ b/test/CMakeLists.txt @@ -52,6 +52,7 @@ list(APPEND testdirs bacnet/bacapp bacnet/bacdcode bacnet/bacdevobjpropref + bacnet/bacdest bacnet/bacerror bacnet/bacint bacnet/bacpropstates diff --git a/test/bacnet/bacdest/src/main.c b/test/bacnet/bacdest/src/main.c index e4c5f330..ad356bb5 100644 --- a/test/bacnet/bacdest/src/main.c +++ b/test/bacnet/bacdest/src/main.c @@ -11,7 +11,7 @@ #include #include #include -#include +#include #include /** diff --git a/test/bacnet/wp/src/main.c b/test/bacnet/wp/src/main.c index 2a6a8d45..63649196 100644 --- a/test/bacnet/wp/src/main.c +++ b/test/bacnet/wp/src/main.c @@ -17,33 +17,49 @@ */ /** - * @brief Test encode/decode API for unsigned 16b integers + * @brief Decode stub for WriteProperty service + * @return number of bytes decoded, or #BACNET_STATUS_ERROR */ static int wp_decode_apdu(uint8_t *apdu, - unsigned apdu_len, + unsigned apdu_size, uint8_t *invoke_id, BACNET_WRITE_PROPERTY_DATA *wpdata) { int len = 0; - unsigned offset = 0; + int apdu_len = 0; - if (!apdu) + if (!apdu) { return -1; + } + if (apdu_size < 4) { + return BACNET_STATUS_ERROR; + } /* optional checking - most likely was already done prior to this call */ - if (apdu[0] != PDU_TYPE_CONFIRMED_SERVICE_REQUEST) - return -1; + if (apdu[0] != PDU_TYPE_CONFIRMED_SERVICE_REQUEST) { + return BACNET_STATUS_ERROR; + } /* apdu[1] = encode_max_segs_max_apdu(0, MAX_APDU); */ - *invoke_id = apdu[2]; /* invoke id - filled in by net layer */ - if (apdu[3] != SERVICE_CONFIRMED_WRITE_PROPERTY) - return -1; - offset = 4; - - if (apdu_len > offset) { + if (invoke_id) { + *invoke_id = apdu[2]; /* invoke id - filled in by net layer */ + } + if (apdu[3] != SERVICE_CONFIRMED_WRITE_PROPERTY) { + return BACNET_STATUS_ERROR; + } + len = 4; + apdu_len += len; + if (apdu_len < apdu_size) { len = - wp_decode_service_request(&apdu[offset], apdu_len - offset, wpdata); + wp_decode_service_request(&apdu[apdu_len], apdu_size - apdu_len, wpdata); + if (len > 0) { + apdu_len += len; + } else { + apdu_len = len; + } + } else { + return BACNET_STATUS_ERROR; } - return len; + return apdu_len; } static void testWritePropertyTag(BACNET_APPLICATION_DATA_VALUE *value) @@ -53,6 +69,7 @@ static void testWritePropertyTag(BACNET_APPLICATION_DATA_VALUE *value) BACNET_APPLICATION_DATA_VALUE test_value; uint8_t apdu[480] = { 0 }; int len = 0; + int null_len = 0; int apdu_len = 0; uint8_t invoke_id = 128; uint8_t test_invoke_id = 0; @@ -60,17 +77,38 @@ static void testWritePropertyTag(BACNET_APPLICATION_DATA_VALUE *value) wpdata.application_data_len = bacapp_encode_application_data(&wpdata.application_data[0], value); + null_len = wp_encode_apdu(NULL, invoke_id, &wpdata); len = wp_encode_apdu(&apdu[0], invoke_id, &wpdata); + zassert_equal(null_len, len, "null_len=%d len=%d", null_len, len); zassert_not_equal(len, 0, "len=%d", len); /* decode the data */ apdu_len = len; + null_len = wp_decode_apdu(&apdu[0], apdu_len, &test_invoke_id, NULL); len = wp_decode_apdu(&apdu[0], apdu_len, &test_invoke_id, &test_data); + zassert_equal(null_len, len, "null_len=%d len=%d", null_len, len); zassert_true(len > 0, "len=%d", len); zassert_equal(test_data.object_type, wpdata.object_type, NULL); zassert_equal(test_data.object_instance, wpdata.object_instance, NULL); zassert_equal(test_data.object_property, wpdata.object_property, NULL); zassert_equal(test_data.array_index, wpdata.array_index, NULL); + /* test the OPTIONAL property-array-index */ + wpdata.array_index = BACNET_ARRAY_ALL; + len = wp_encode_apdu(&apdu[0], invoke_id, &wpdata); + apdu_len = wp_decode_apdu(&apdu[0], len, &test_invoke_id, &test_data); + zassert_equal(apdu_len, len, "apdu_len=%d len=%d", apdu_len, len); + zassert_true(len > 0, "len=%d", len); + wpdata.array_index = 0; + /* test the OPTIONAL priority */ + wpdata.priority = BACNET_MAX_PRIORITY; + len = wp_encode_apdu(&apdu[0], invoke_id, &wpdata); + apdu_len = wp_decode_apdu(&apdu[0], len, &test_invoke_id, &test_data); + zassert_equal(apdu_len, len, "apdu_len=%d len=%d", apdu_len, len); + zassert_true(len > 0, "len=%d", len); + wpdata.priority = 0; /* decode the application value of the request */ + len = wp_encode_apdu(&apdu[0], invoke_id, &wpdata); + apdu_len = wp_decode_apdu(&apdu[0], len, &test_invoke_id, &test_data); + apdu_len = len; len = bacapp_decode_application_data(test_data.application_data, test_data.application_data_len, &test_value); zassert_equal(test_value.tag, value->tag, NULL); @@ -98,29 +136,37 @@ static void testWritePropertyTag(BACNET_APPLICATION_DATA_VALUE *value) test_value.type.Enumerated, value->type.Enumerated, NULL); break; case BACNET_APPLICATION_TAG_DATE: - zassert_equal(test_value.type.Date.year, value->type.Date.year, NULL); + zassert_equal( + test_value.type.Date.year, value->type.Date.year, NULL); zassert_equal( test_value.type.Date.month, value->type.Date.month, NULL); zassert_equal(test_value.type.Date.day, value->type.Date.day, NULL); - zassert_equal(test_value.type.Date.wday, value->type.Date.wday, NULL); + zassert_equal( + test_value.type.Date.wday, value->type.Date.wday, NULL); break; case BACNET_APPLICATION_TAG_TIME: - zassert_equal(test_value.type.Time.hour, value->type.Time.hour, NULL); + zassert_equal( + test_value.type.Time.hour, value->type.Time.hour, NULL); zassert_equal(test_value.type.Time.min, value->type.Time.min, NULL); zassert_equal(test_value.type.Time.sec, value->type.Time.sec, NULL); - zassert_equal( - test_value.type.Time.hundredths, value->type.Time.hundredths, NULL); + zassert_equal(test_value.type.Time.hundredths, + value->type.Time.hundredths, NULL); break; case BACNET_APPLICATION_TAG_OBJECT_ID: - zassert_equal( - test_value.type.Object_Id.type, value->type.Object_Id.type, NULL); - zassert_equal( - test_value.type.Object_Id.instance, - value->type.Object_Id.instance, NULL); + zassert_equal(test_value.type.Object_Id.type, + value->type.Object_Id.type, NULL); + zassert_equal(test_value.type.Object_Id.instance, + value->type.Object_Id.instance, NULL); break; default: break; } + /* test packets that are too short */ + while (apdu_len) { + apdu_len--; + len = wp_decode_apdu(&apdu[0], apdu_len, &test_invoke_id, &test_data); + zassert_true(len <= 0, "len=%d tag=%d", len, value->tag); + } } #if defined(CONFIG_ZTEST_NEW_API) @@ -199,22 +245,17 @@ static void testWriteProperty(void) value.type.Object_Id.type = OBJECT_LIFE_SAFETY_ZONE; value.type.Object_Id.instance = BACNET_MAX_INSTANCE; testWritePropertyTag(&value); - - return; } /** * @} */ - #if defined(CONFIG_ZTEST_NEW_API) ZTEST_SUITE(wp_tests, NULL, NULL, NULL, NULL, NULL); #else void test_main(void) { - ztest_test_suite(wp_tests, - ztest_unit_test(testWriteProperty) - ); + ztest_test_suite(wp_tests, ztest_unit_test(testWriteProperty)); ztest_run_test_suite(wp_tests); }