diff --git a/src/ThingSetServer.cpp b/src/ThingSetServer.cpp index 2145400..84904d5 100644 --- a/src/ThingSetServer.cpp +++ b/src/ThingSetServer.cpp @@ -96,6 +96,9 @@ int _ThingSetServer::handleGet(ThingSetRequestContext &context) if (encodable->encode(context.encoder())) { return context.encoder().getEncodedLength() + context.getHeaderLength(); } + // encoding fails when the value does not fit in the response buffer; reply 0xAD + context.setStatus(ThingSetStatusCode::requestTooLarge); + return context.getHeaderLength(); } else if (context.node->tryCastTo(ThingSetNodeType::hasChildren, &target)) { ThingSetParentNode *parent = reinterpret_cast(target); @@ -108,24 +111,32 @@ int _ThingSetServer::handleGet(ThingSetRequestContext &context) ids.push_back(child->getId()); } } - context.encoder().encodeMapStart(ids.size()); + bool encoded = context.encoder().encodeMapStart(ids.size()); for (ThingSetNode *child : *parent) { + if (!encoded) + { + break; + } if (child->tryCastTo(ThingSetNodeType::encodable, &target)) { ThingSetEncodable *encodable = reinterpret_cast(target); if (context.useIds()) { - context.encoder().encode(std::make_pair(child->getId(), encodable)); + encoded = context.encoder().encode(std::make_pair(child->getId(), encodable)); } else { - context.encoder().encode(std::make_pair(child->getName(), encodable)); + encoded = context.encoder().encode(std::make_pair(child->getName(), encodable)); } } } - context.encoder().encodeMapEnd(ids.size()); - return context.encoder().getEncodedLength() + context.getHeaderLength(); + if (encoded && context.encoder().encodeMapEnd(ids.size())) + { + return context.encoder().getEncodedLength() + context.getHeaderLength(); + } + context.setStatus(ThingSetStatusCode::requestTooLarge); + return context.getHeaderLength(); } context.setStatus(ThingSetStatusCode::unsupportedFormat); return context.getHeaderLength(); diff --git a/src/can/zephyr/ThingSetZephyrCanClientTransport.cpp b/src/can/zephyr/ThingSetZephyrCanClientTransport.cpp index 491268a..ba23ab7 100644 --- a/src/can/zephyr/ThingSetZephyrCanClientTransport.cpp +++ b/src/can/zephyr/ThingSetZephyrCanClientTransport.cpp @@ -12,6 +12,10 @@ bool ThingSetZephyrCanClientTransport::connect() return _requestResponseContext.bind(_targetNodeAddress, [&](const CanID &sender, uint8_t *rxBuffer, size_t rxSize, uint8_t *, size_t) { ResponseMessage message; + if (rxSize > sizeof(message.buffer)) { + // context rx buffer larger than CONFIG_THINGSET_PLUS_PLUS_CAN_CLIENT_RX_BUFFER_SIZE + return -ENOMEM; + } memcpy(message.buffer, rxBuffer, rxSize); message.length = rxSize; return k_msgq_put(&_responseQueue, &message, K_MSEC(100)); diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index 8567153..b83001d 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -37,7 +37,8 @@ target_sources(testapp PRIVATE TestBinaryEncoder.cpp TestTextEncodingRecords.cpp TestRequestRewriter.cpp TestEui.cpp - TestClient.cpp) + TestClient.cpp + TestServer.cpp) # regrettably exlcude this test until we figure out why Socket server is broken on macOS if(NOT APPLE) diff --git a/tests/TestServer.cpp b/tests/TestServer.cpp new file mode 100644 index 0000000..f960fe1 --- /dev/null +++ b/tests/TestServer.cpp @@ -0,0 +1,89 @@ +/* + * Copyright (c) 2025 Brill Power. + * + * SPDX-License-Identifier: Apache-2.0 + */ +#include "gtest/gtest.h" +#include +#include + +using namespace ThingSet; + +namespace { + +class TestableServer : public _ThingSetServer +{ +public: + TestableServer() : _ThingSetServer(nullptr) + {} + + bool listen() override + { + return true; + } + + using _ThingSetServer::handleBinaryRequest; +}; + +/// @brief Builds a binary GET request for the given node ID. +static size_t buildGetRequest(uint16_t id, uint8_t *request, size_t requestSize) +{ + request[0] = (uint8_t)ThingSetBinaryRequestType::get; + FixedDepthThingSetBinaryEncoder encoder(request + 1, requestSize - 1); + EXPECT_TRUE(encoder.encode(id)); + return 1 + encoder.getEncodedLength(); +} + +} // namespace + +TEST(Server, GetGroupThatFitsReturnsContent) +{ + ThingSetGroup<0x700, 0, "TestFitGroup"> group; + ThingSetReadOnlyProperty f32 { 0x701, 0x700, "f32", 1.5f }; + + TestableServer server; + uint8_t request[8]; + size_t requestLen = buildGetRequest(0x700, request, sizeof(request)); + uint8_t response[256]; + int responseLen = server.handleBinaryRequest(request, requestLen, response, sizeof(response)); + + ASSERT_GT(responseLen, 1); + ASSERT_EQ(ThingSetStatusCode::content, response[0]); +} + +TEST(Server, GetGroupTooBigForResponseBufferReturnsRequestTooLarge) +{ + ThingSetGroup<0x710, 0, "TestBigGroup"> group; + std::array table; + table.fill(1.0f); + ThingSetReadOnlyReferenceProperty> bigArray { 0x711, 0x710, "bigArray", + table }; + + TestableServer server; + uint8_t request[8]; + size_t requestLen = buildGetRequest(0x710, request, sizeof(request)); + // response buffer far too small for ~880 bytes of encoded floats + uint8_t response[128]; + int responseLen = server.handleBinaryRequest(request, requestLen, response, sizeof(response)); + + // only the status byte; no truncated payload + ASSERT_EQ(1, responseLen); + ASSERT_EQ(ThingSetStatusCode::requestTooLarge, response[0]); +} + +TEST(Server, GetPropertyTooBigForResponseBufferReturnsRequestTooLarge) +{ + std::array table; + table.fill(1.0f); + ThingSetReadOnlyReferenceProperty> bigArray { 0x721, 0, "bigArrayTopLevel", + table }; + + TestableServer server; + uint8_t request[8]; + size_t requestLen = buildGetRequest(0x721, request, sizeof(request)); + uint8_t response[128]; + int responseLen = server.handleBinaryRequest(request, requestLen, response, sizeof(response)); + + ASSERT_EQ(1, responseLen); + ASSERT_EQ(ThingSetStatusCode::requestTooLarge, response[0]); +}