Skip to content

Add unit tests for MQTTPropAdd buffer and packet size limit error paths#377

Open
guclumhg wants to merge 1 commit into
FreeRTOS:mainfrom
guclumhg:test/prop-serializer-branch-coverage
Open

Add unit tests for MQTTPropAdd buffer and packet size limit error paths#377
guclumhg wants to merge 1 commit into
FreeRTOS:mainfrom
guclumhg:test/prop-serializer-branch-coverage

Conversation

@guclumhg

Copy link
Copy Markdown

Description

Adds six unit tests covering previously untested error paths in core_mqtt_prop_serializer.c:

  • Buffer-full-at-currentIndex conditions in the addPropUint8/addPropUint16/addPropUint32 helpers (exercised through MQTTPropAdd_PayloadFormat, MQTTPropAdd_TopicAliasMax and MQTTPropAdd_SessionExpiry) and in MQTTPropAdd_SubscriptionId: the buffer is large enough overall, but currentIndex leaves fewer bytes than needed to encode the property.
  • The maximum MQTT remaining length (268,435,455) limit checks in MQTTPropAdd_SubscriptionId.
  • The 16-bit key/value length validation in MQTTPropAdd_UserProp.

Coverage impact (lcov, branch coverage enabled):

  • core_mqtt_prop_serializer.c: lines 98.5% -> 100%, branches 171/182 (94.0%) -> 180/182 (98.9%)
  • Total: branches 93.8% -> 94.1%

The two remaining branch directions in addPropUtf8 (the propertyLength > SIZE_MAX - 2 and propertyLength + 2 > 268435456 conditions) require propertyLength > 65535, which cannot be exercised by a test that passes the assert-enabled build due to assert( !CHECK_SIZE_T_OVERFLOWS_16BIT( propertyLength ) ); the public wrappers additionally validate the same bound before calling the helper.

Test Steps

Both CI unit-test configurations pass locally (5/5 test suites):

cmake -S test -B build -G "Unix Makefiles" -DCMAKE_BUILD_TYPE=Debug \
  -DBUILD_CLONE_SUBMODULES=ON -DUNITTEST=1 [-DCOV_ANALYSIS=1] \
  -DCMAKE_C_FLAGS='--coverage -Wall -Wextra -Wsign-compare -Werror -DLIBRARY_LOG_LEVEL=LOG_DEBUG'
make -C build all
ctest --test-dir build -E system --output-on-failure
make -C build coverage

Formatting verified with uncrustify 0.69.0 using the config from FreeRTOS/CI-CD-Github-Actions.

Checklist:

  • I have tested my changes. No regression in existing tests.
  • I have modified and/or added unit-tests to cover the code changes in this Pull Request.

Related Issue

N/A

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

Cover previously untested error branches in core_mqtt_prop_serializer.c:
- Buffer-full-at-currentIndex paths in the addPropUint8/16/32 helpers
  and MQTTPropAdd_SubscriptionId.
- Maximum MQTT remaining length (268,435,455) checks in
  MQTTPropAdd_SubscriptionId.
- 16-bit key/value length validation in MQTTPropAdd_UserProp.

Raises the file to 100% line coverage and 180/182 branches. The two
remaining branch directions in addPropUtf8 are unreachable in
assert-enabled builds.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant