diff --git a/lib/decoder/PayloadDecoder.cpp b/lib/decoder/PayloadDecoder.cpp index 5789d5939..6d86a7057 100644 --- a/lib/decoder/PayloadDecoder.cpp +++ b/lib/decoder/PayloadDecoder.cpp @@ -563,7 +563,12 @@ namespace MAT_NS_BEGIN { if (result) { - out = j.dump(2); + // Use error_handler_t::replace so that malformed UTF-8 in the + // decoded telemetry payload is replaced with U+FFFD instead of + // throwing nlohmann::json::type_error (id 316). Telemetry event + // strings can legitimately contain non-UTF-8 bytes; without this + // the unhandled C++ exception terminates the hosting process. + out = j.dump(2, ' ', false, json::error_handler_t::replace); } return result; @@ -580,7 +585,9 @@ namespace MAT_NS_BEGIN { nlohmann::json j; to_json(j, in); - std::string s = j.dump(4); + // See DecodeRequest above: replace malformed UTF-8 rather than + // throwing so a bad record cannot terminate the process. + std::string s = j.dump(4, ' ', false, json::error_handler_t::replace); out.assign(s.begin(), s.end()); return true; diff --git a/tests/unittests/CMakeLists.txt b/tests/unittests/CMakeLists.txt index 945fc23df..7233d2920 100644 --- a/tests/unittests/CMakeLists.txt +++ b/tests/unittests/CMakeLists.txt @@ -40,6 +40,7 @@ set(SRCS OfflineStorageTests_Room.cpp OfflineStorageTests_SQLite.cpp PackagerTests.cpp + PayloadDecoderTests.cpp PalTests.cpp RouteTests.cpp StringUtilsTests.cpp diff --git a/tests/unittests/PayloadDecoderTests.cpp b/tests/unittests/PayloadDecoderTests.cpp new file mode 100644 index 000000000..8e8051cee --- /dev/null +++ b/tests/unittests/PayloadDecoderTests.cpp @@ -0,0 +1,86 @@ +// +// Copyright (c) Microsoft Corporation. All rights reserved. +// SPDX-License-Identifier: Apache-2.0 +// +#include "common/Common.hpp" +#include "PayloadDecoder.hpp" + +using namespace testing; +using namespace MAT; + +namespace +{ + // Builds a minimally-populated Common Schema record. to_json() in the + // PayloadDecoder unconditionally dereferences element [0] of every ext + // vector, so all seven must contain at least one element for serialization + // to succeed. + CsProtocol::Record MakeMinimalRecord() + { + CsProtocol::Record record; + record.ver = "3.0"; + record.name = "Test.Event"; + record.time = 0; + record.iKey = "o:0000"; + record.baseType = "custom"; + record.extProtocol.push_back(CsProtocol::Protocol{}); + record.extUser.push_back(CsProtocol::User{}); + record.extDevice.push_back(CsProtocol::Device{}); + record.extOs.push_back(CsProtocol::Os{}); + record.extApp.push_back(CsProtocol::App{}); + record.extNet.push_back(CsProtocol::Net{}); + record.extSdk.push_back(CsProtocol::Sdk{}); + return record; + } +} + +// A telemetry event field can legitimately contain bytes that are not valid +// UTF-8. nlohmann::json::dump() defaults to error_handler_t::strict, which +// throws type_error.316 on such input. Because DecodeRecord/DecodeRequest run +// on the decode path inside the hosting process, an unhandled throw terminates +// that process. These tests lock in the error_handler_t::replace behavior: no +// throw, and the malformed byte is emitted as the U+FFFD replacement character +// (EF BF BD). +TEST(PayloadDecoderTests, DecodeRecord_InvalidUtf8_DoesNotThrow) +{ + CsProtocol::Record record = MakeMinimalRecord(); + // Build the field with an explicit 0xFF byte (never valid UTF-8). A string + // literal escape ("...\xFF...") would rely on implementation-defined char + // conversion and can trip -Werror constant-conversion on some toolchains. + std::string name = "Bad"; + name.push_back(static_cast(0xFF)); + name += "Name"; + record.name = name; + + std::string out; + bool decoded = false; + EXPECT_NO_THROW({ decoded = exporters::DecodeRecord(record, out); }); + + // When the SDK is built with JSON + Zlib support the real decoder runs and + // must have replaced the bad byte. In a stubbed build DecodeRecord returns + // false with an empty string, in which case the no-throw guarantee above is + // what this test protects. + if (decoded) + { + EXPECT_NE(out.find("\xEF\xBF\xBD"), std::string::npos) + << "Malformed UTF-8 should be replaced with U+FFFD"; + EXPECT_EQ(out.find(static_cast(0xFF)), std::string::npos) + << "Raw invalid byte must not survive in the output"; + } +} + +TEST(PayloadDecoderTests, DecodeRecord_ValidUtf8_IsPreserved) +{ + CsProtocol::Record record = MakeMinimalRecord(); + record.name = "Valid.Event"; + + std::string out; + bool decoded = false; + EXPECT_NO_THROW({ decoded = exporters::DecodeRecord(record, out); }); + + if (decoded) + { + EXPECT_NE(out.find("Valid.Event"), std::string::npos); + EXPECT_EQ(out.find("\xEF\xBF\xBD"), std::string::npos) + << "Valid UTF-8 must not be altered"; + } +}