From 8b8a358c8bfdf92e54c5164fd460c4ab31e2f400 Mon Sep 17 00:00:00 2001 From: Dario Lipicar Date: Wed, 29 Jul 2026 00:26:32 -0300 Subject: [PATCH] fix: uint64 survives the event path and the plain wire (#30) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix(events): the event bridge converts through the canonical helper setEventListenerStdBridge adapts the universal event callback (name + JSON string) to the Qt EventCallback (name + QVariantList). It is the event-path counterpart of callMethodStdBridge, but it did the conversion itself: callMethodStdBridge -> logos::nlohmannToQVariant (canonical) setEventListenerStdBridge -> QJsonDocument::fromJson + QJsonValue::toVariant (Qt's parser) Two consequences, both measured by the LIDL conformance matrix as M6: * a uint64 above int64max degraded to a double. Qt 6 backs QJsonValue with QCborValue, so integers up to int64 DID survive — only values with no integral representation there fell back to double. echoUint(2^64-1) was exact while uintEvent(2^64-1) arrived as 1.8446744073709552e+19: same value, same process, one hop later. * canonical tagged bytes {"_bytes": ...} were not decoded, arriving as a QVariantMap where the method path yields a QByteArray. This never showed up end-to-end because the undecoded map round-trips to JSON and the python client decodes the tag itself — but a C++ or QML event subscriber got a map. Both now go through logos::nlohmannArgsToQVariantList, which the generated cdylib emitTrampoline already used. Numbers and bytes no longer depend on whether a value left the module as a return or as an event. Not the residue of the codec convergence, despite how M6 was originally registered. #29 converged six copies of the VALUE codec; this was a seventh conversion inside an ADAPTER, which that scope never touched. It is also not on the providers' own path — a Qt provider stores its callback verbatim and a cdylib provider already converted correctly. The one live caller is the logoscore daemon's CoreServiceImpl, which forwards every watched module event; that is why C++ and Rust providers measured identically. Why it survived: the bridge appeared in the test suite once, in test_universal_provider_dispatch.cpp, purely to satisfy the pure virtual. No test asserted anything about an event payload. The method path got 15 contract tests in #29; the event path got none. tests: 11 new cells pin the bridge directly — uint64 past int64max, 2^53+1, int64::min, large integers nested in containers, tagged bytes at top level and at depth, plus the shapes that already worked (multi-param order, double staying double, null elements, empty payload, the non-array raw-string fallback) so a future rewrite cannot quietly drop them. 210/210. verified: logos-cpp-sdk, logos-qt-sdk, logos-liblogos and logos-logoscore-cli all green against this build; the conformance matrix goes 156 -> 158 pass with M6's two cells retired, and the ext table stays 40/40. Co-Authored-By: Claude Opus 5 * test(events): pin the signedness rule the convergence brings with it nlohmannArgsToQVariantList classifies every non-negative integer as unsigned, so a LIDL `int` event argument now arrives as ULongLong where it used to be LongLong. That matches what nlohmannToQVariant (the method path) and the cdylib emitTrampoline already did — the surfaces now agree — but it is an observable metatype change that nothing asserted. Pinned in both directions (non-negative -> ULongLong, negative -> LongLong) so it stays a decision rather than a side effect. Value-level reads are unaffected. 212/212. Co-Authored-By: Claude Opus 5 * fix(plain): RpcValue can represent a uint64 above int64max The plain (tcp/tcp_ssl) wire squeezed every unsigned value through int64_t, so a LIDL `uint` above int64max wrapped — independently in each direction: outbound qvariant_rpc_value.cpp QMetaType::ULongLong -> int64_t(...) inbound json_mapping.cpp is_number_unsigned -> get() Neither wraps loudly: .get() past int64max returns -1 with no exception. Two peers both running this code agreed on -1, so nothing looked broken from inside — and no plain-tier test used an integer outside int32 range. Measured over real tcp before the fix: echoUint(2^63) -> -9223372036854775808 echoUint(2^64-1) -> -1 This was never a wire-format constraint. Both codecs carry uint64 natively (CBOR emits major type 0, `1b ff..ff`) and the envelope's own `id` field already crossed this wire as uint64_t. Only RpcValue *payloads* could not represent it. RpcValue gains a uint64_t alternative, used through `makeInteger()` and ONLY for values above int64max — the sole case where int64_t loses information. Anything broader would change the representation of every non-negative integer already on this wire, and since std::variant equality compares the alternative index it would break comparisons against int64-built values, to fix nothing. Small unsigned values keep crossing as signed, pinned by a test so the rule stays visible. Also fixes an off-by-one in the QJsonValue::Double -> int64 guard while here: double(int64max) rounds UP to exactly 2^63, so `d <= double(int64max)` admitted 2^63 and then ran int64_t(d) out of range — undefined behaviour, saturating on arm64 and INT64_MIN on x86-64. Now a strict `<` against 2^63. tests: 14 new. Both codecs round-trip 2^64-1 flat and nested; negatives stay signed; the Qt boundary is exact in both directions; the narrow representation rule and the 2^63 guard are pinned. 226/226. verified end-to-end, cross-process, with a negative control: the new 64-bit boundary cases in logos-logoscore-py fail on the pinned protocol over tcp with exactly the values above, and all 68 pass with this build — on local, tcp and tcp_ssl alike. Co-Authored-By: Claude Opus 5 --------- Co-authored-by: Claude Opus 5 --- cpp/implementations/plain/json_mapping.cpp | 8 +- .../plain/qvariant_rpc_value.cpp | 19 +- cpp/implementations/plain/rpc_value.h | 25 ++ cpp/logos_provider_interface.cpp | 25 +- tests/protocol/CMakeLists.txt | 10 + .../protocol/test_event_payload_fidelity.cpp | 248 ++++++++++++++++++ tests/protocol/test_plain_uint64.cpp | 228 ++++++++++++++++ 7 files changed, 554 insertions(+), 9 deletions(-) create mode 100644 tests/protocol/test_event_payload_fidelity.cpp create mode 100644 tests/protocol/test_plain_uint64.cpp diff --git a/cpp/implementations/plain/json_mapping.cpp b/cpp/implementations/plain/json_mapping.cpp index c306c06..b84c725 100644 --- a/cpp/implementations/plain/json_mapping.cpp +++ b/cpp/implementations/plain/json_mapping.cpp @@ -36,6 +36,7 @@ json valueToJson(const RpcValue& v) if (v.isNull()) return nullptr; if (v.isBool()) return v.asBool(); if (v.isInt()) return v.asInt(); + if (v.isUInt()) return v.asUInt(); // > int64max: nlohmann keeps it as number_unsigned if (v.isDouble()) return v.asDouble(); if (v.isString()) return v.asString(); if (v.isBytes()) return logos::bytesToJson(v.asBytes().data); @@ -56,7 +57,12 @@ RpcValue jsonToValue(const json& j) { if (j.is_null()) return RpcValue{std::monostate{}}; if (j.is_boolean()) return RpcValue{j.get()}; - if (j.is_number_integer() || j.is_number_unsigned()) + // is_number_unsigned() is checked FIRST and routed through makeInteger: + // .get() on a value above int64max wraps silently (2^64-1 -> -1) + // with no exception, so a correct peer's uint64 used to arrive as -1. + if (j.is_number_unsigned()) + return RpcValue::makeInteger(j.get()); + if (j.is_number_integer()) return RpcValue{j.get()}; if (j.is_number_float()) return RpcValue{j.get()}; if (j.is_string()) return RpcValue{j.get()}; diff --git a/cpp/implementations/plain/qvariant_rpc_value.cpp b/cpp/implementations/plain/qvariant_rpc_value.cpp index 31730d0..162f466 100644 --- a/cpp/implementations/plain/qvariant_rpc_value.cpp +++ b/cpp/implementations/plain/qvariant_rpc_value.cpp @@ -22,9 +22,14 @@ RpcValue fromJsonValue(const QJsonValue& v) case QJsonValue::Double: { double d = v.toDouble(); double intPart = 0.0; + // Strict `<` on the upper bound: double(int64max) rounds UP to exactly + // 2^63, so `d <= double(int64max)` admitted d == 2^63, and int64_t(d) on + // an out-of-range double is undefined behaviour — saturating to int64max + // on arm64, INT64_MIN on x86-64. The lower bound needs no such care: + // double(int64min) is exactly -2^63 and representable. if (std::modf(d, &intPart) == 0.0 && d >= double(std::numeric_limits::min()) && - d <= double(std::numeric_limits::max())) + d < 9223372036854775808.0) // 2^63, i.e. int64max + 1 return RpcValue{int64_t(d)}; return RpcValue{d}; } @@ -53,6 +58,12 @@ QJsonValue toJsonValue(const RpcValue& v) if (v.isNull()) return QJsonValue(QJsonValue::Null); if (v.isBool()) return QJsonValue(v.asBool()); if (v.isInt()) return QJsonValue(static_cast(v.asInt())); + // QJsonValue has no unsigned primitive and its double cannot hold the band + // above int64max exactly. This path only carries method-introspection + // METADATA (parameter descriptors), never payload values, so the lossy cast + // is acceptable here — but without this branch a uint64 would fall through + // to Null, which is worse than imprecise. + if (v.isUInt()) return QJsonValue(static_cast(v.asUInt())); if (v.isDouble()) return QJsonValue(v.asDouble()); if (v.isString()) return QJsonValue(QString::fromStdString(v.asString())); if (v.isBytes()) { @@ -126,7 +137,10 @@ RpcValue qvariantToRpcValue(const QVariant& v) case QMetaType::ULongLong: case QMetaType::UShort: case QMetaType::UChar: - return RpcValue{int64_t(v.toULongLong())}; + // makeInteger, not int64_t(): a LIDL `uint` above int64max used to wrap + // to -1 here, silently and in every direction. Values that fit int64_t + // still take the int64_t alternative, so nothing else changes. + return RpcValue::makeInteger(v.toULongLong()); case QMetaType::Float: case QMetaType::Double: return RpcValue{v.toDouble()}; @@ -181,6 +195,7 @@ QVariant rpcValueToQVariant(const RpcValue& v) if (v.isNull()) return QVariant(); if (v.isBool()) return QVariant(v.asBool()); if (v.isInt()) return QVariant(static_cast(v.asInt())); + if (v.isUInt()) return QVariant(static_cast(v.asUInt())); if (v.isDouble()) return QVariant(v.asDouble()); if (v.isString()) return QVariant(QString::fromStdString(v.asString())); if (v.isBytes()) { diff --git a/cpp/implementations/plain/rpc_value.h b/cpp/implementations/plain/rpc_value.h index a803a91..c8dfb29 100644 --- a/cpp/implementations/plain/rpc_value.h +++ b/cpp/implementations/plain/rpc_value.h @@ -3,6 +3,7 @@ #include #include +#include #include #include #include @@ -60,6 +61,7 @@ struct RpcValue { std::monostate, // null bool, int64_t, + uint64_t, // ONLY for values above int64max — see makeInteger() double, std::string, RpcBytes, @@ -74,6 +76,7 @@ struct RpcValue { RpcValue(bool b) : value(b) {} RpcValue(int i) : value(static_cast(i)) {} RpcValue(int64_t i) : value(i) {} + RpcValue(uint64_t u) : value(u) {} RpcValue(double d) : value(d) {} RpcValue(const char* s) : value(std::string(s)) {} RpcValue(std::string s) : value(std::move(s)) {} @@ -81,17 +84,39 @@ struct RpcValue { RpcValue(RpcList l) : value(std::move(l)) {} RpcValue(RpcMap m) : value(std::move(m)) {} + // Canonical way to build an integer from an unsigned source. + // + // The uint64_t alternative exists for exactly one reason: to carry values + // int64_t cannot. It is NOT used for every non-negative integer, and that is + // deliberate — std::variant equality compares the alternative index first, + // so representing 42 as uint64_t would make RpcValue{42} != decode("42") and + // silently change the metatype of every non-negative integer already + // crossing this wire, to fix nothing. Values that fit int64_t keep their + // existing representation; only the band above int64max is new. + static RpcValue makeInteger(uint64_t u) { + if (u <= static_cast(std::numeric_limits::max())) + return RpcValue{static_cast(u)}; + return RpcValue{u}; + } + bool isNull() const { return std::holds_alternative(value); } bool isBool() const { return std::holds_alternative(value); } bool isInt() const { return std::holds_alternative(value); } + bool isUInt() const { return std::holds_alternative(value); } bool isDouble() const { return std::holds_alternative(value); } bool isString() const { return std::holds_alternative(value); } bool isBytes() const { return std::holds_alternative(value); } bool isList() const { return std::holds_alternative(value); } bool isMap() const { return std::holds_alternative(value); } + // True for either integer alternative — use this when you care about "is a + // whole number" rather than about signedness, so a uint64 above int64max is + // not mistaken for a non-integer. + bool isIntegral() const { return isInt() || isUInt(); } + bool asBool() const { return std::get(value); } int64_t asInt() const { return std::get(value); } + uint64_t asUInt() const { return std::get(value); } double asDouble() const { return std::get(value); } const std::string& asString() const { return std::get(value); } const RpcBytes& asBytes() const { return std::get(value); } diff --git a/cpp/logos_provider_interface.cpp b/cpp/logos_provider_interface.cpp index 1a6cef5..fde191b 100644 --- a/cpp/logos_provider_interface.cpp +++ b/cpp/logos_provider_interface.cpp @@ -1,7 +1,6 @@ #include "logos_provider_interface.h" #include "logos_json_convert.h" #include -#include // --------------------------------------------------------------------------- // LogosProviderObject — universal virtual defaults @@ -46,11 +45,25 @@ void LogosProviderObject::setEventListenerStdBridge(EventCallback callback) setEventListenerStd([callback](const std::string& eventName, const std::string& data) { if (!callback) return; QVariantList qData; - QJsonDocument doc = QJsonDocument::fromJson( - QByteArray::fromStdString(data)); - if (doc.isArray()) { - for (const QJsonValue& v : doc.array()) - qData.append(v.toVariant()); + // Parse with nlohmann and convert with the SAME helper the method path + // uses (callMethodStdBridge above), so a value does not depend on + // whether it left the module as a return or as an event. + // + // This used QJsonDocument::fromJson + QJsonValue::toVariant. Qt 6 backs + // QJsonValue with QCborValue, so integers up to int64 survived — but a + // uint64 above int64max has no integral representation there and fell + // back to double: 18446744073709551615 arrived as 1.8446744073709552e+19, + // exact on the method path and rounded one hop later. The Qt parser also + // has no notion of the canonical {"_bytes": ...} tag, so byte payloads + // arrived as a QVariantMap and only survived because that map round-trips + // to a consumer that decodes the tag itself. + // + // parse(..., nullptr, false) is the non-throwing form: malformed input + // yields a discarded value and takes the raw-string fallback below, + // which is the behaviour QJsonDocument gave for unparseable data. + const nlohmann::json payload = nlohmann::json::parse(data, nullptr, false); + if (!payload.is_discarded() && payload.is_array()) { + qData = logos::nlohmannArgsToQVariantList(payload); } else { qData.append(QString::fromStdString(data)); } diff --git a/tests/protocol/CMakeLists.txt b/tests/protocol/CMakeLists.txt index 1f44f7d..bc1b491 100644 --- a/tests/protocol/CMakeLists.txt +++ b/tests/protocol/CMakeLists.txt @@ -6,6 +6,11 @@ add_executable(protocol_tests test_protocol_version.cpp test_json_convert_bytes.cpp test_universal_provider_dispatch.cpp + # Event payload fidelity across the universal -> Qt bridge + # (setEventListenerStdBridge). Every Qt-free provider emits through that one + # conversion; test_universal_provider_dispatch references it only to satisfy + # the pure virtual and asserts nothing about payloads. + test_event_payload_fidelity.cpp test_lp_client.cpp # The canonical LIDL <-> JSON codec (cpp/logos_codec.h): one implementation # of an encoding that used to exist six times with divergent semantics. @@ -30,6 +35,11 @@ add_executable(protocol_tests test_rpc_framing.cpp test_json_codec.cpp test_cbor_codec.cpp + # uint64 across the plain wire. RpcValue had no unsigned alternative, so a + # value above int64max wrapped to -1 independently in each direction; no + # plain-tier test used an integer outside int32 range. Also pins the + # off-by-one in the QJsonValue::Double -> int64 guard. + test_plain_uint64.cpp # Qt-free socket-access + stale-socket reaper helpers (multi-user local # transport): applySocketPerms group/mode policy, isSocketDead predicate, # reapStaleSockets never touching live sockets or regular files. diff --git a/tests/protocol/test_event_payload_fidelity.cpp b/tests/protocol/test_event_payload_fidelity.cpp new file mode 100644 index 0000000..033b235 --- /dev/null +++ b/tests/protocol/test_event_payload_fidelity.cpp @@ -0,0 +1,248 @@ +#include + +#include +#include +#include +#include +#include + +#include + +#include +#include +#include + +#include "logos_provider_interface.h" + +// --------------------------------------------------------------------------- +// Event payload fidelity across the universal -> Qt bridge. +// +// setEventListenerStdBridge adapts the universal event callback (event name + +// a JSON *string* payload) to the Qt-side EventCallback, which takes a +// QVariantList. It is the event-path counterpart of callMethodStdBridge. +// +// The two are NOT symmetric today, and that asymmetry is what these tests pin: +// +// callMethodStdBridge -> logos::nlohmannToQVariant (canonical) +// setEventListenerStdBridge-> QJsonDocument::fromJson +// + QJsonValue::toVariant (Qt's parser) +// +// Qt 6 backs QJsonValue with QCborValue, so integers up to int64 DO survive the +// Qt parser. What does not survive is a uint64 above int64max: it has no +// integral representation there and falls back to double. Hence the failure is +// narrow and easy to miss — most integers are fine. +// +// Note on who is affected: module providers do NOT go through this bridge. +// A Qt provider stores its callback verbatim (logos-qt-sdk +// QtProviderObject::setEventListener) and a cdylib provider's generated +// emitTrampoline already uses logos::nlohmannArgsToQVariantList, which handles +// is_number_unsigned. The live caller is the logoscore daemon's CoreServiceImpl +// (core_service_dispatch.cpp), which forwards every watched module event +// through here — which is why a uint64 event degrades identically no matter +// what language the emitting module was written in. +// +// The bridge had no payload assertions at all before this file: +// test_universal_provider_dispatch references it only to satisfy the pure +// virtual. That is how the defect survived the codec convergence. +// --------------------------------------------------------------------------- + +namespace { + +// Minimal universal provider: it does nothing but hand us the std-side event +// callback the bridge installs, so a test can fire an event with an exact JSON +// payload and observe what the Qt side receives. +class EventEmittingProvider : public LogosProviderObject { +public: + QVariant callMethod(const QString& m, const QVariantList& a) override { + return callMethodStdBridge(m, a); + } + QJsonArray getMethods() override { return getMethodsStdBridge(); } + void setEventListener(EventCallback cb) override { + setEventListenerStdBridge(std::move(cb)); + } + bool informModuleToken(const QString&, const QString&) override { return true; } + void init(void*) override {} + QString providerName() const override { return QStringLiteral("event_sample"); } + QString providerVersion() const override { return QStringLiteral("1.0.0"); } + + void setEventListenerStd(UniversalEventCallback cb) override { + stdCallback = std::move(cb); + } + + // Emit exactly this JSON text as the payload — no re-serialization on the + // way in, so the test controls the bytes the bridge parses. + void emitRaw(const std::string& eventName, const std::string& payloadJson) { + ASSERT_TRUE(static_cast(stdCallback)); + stdCallback(eventName, payloadJson); + } + + UniversalEventCallback stdCallback; +}; + +// Installs a Qt-side listener and records what it receives. +struct Captured { + QString name; + QVariantList args; + int count = 0; +}; + +Captured captureEvent(const std::string& eventName, const std::string& payloadJson) +{ + EventEmittingProvider provider; + Captured cap; + provider.setEventListener([&cap](const QString& n, const QVariantList& a) { + cap.name = n; + cap.args = a; + ++cap.count; + }); + provider.emitRaw(eventName, payloadJson); + return cap; +} + +} // namespace + +// --- The M6 case ---------------------------------------------------------- +// A uint64 above int64max is exact on the method path since the canonical codec +// landed. It must be exact on the event path too: same value, same process, one +// hop later. +TEST(EventPayloadFidelity, Uint64AboveInt64MaxSurvives) +{ + const Captured cap = captureEvent("uintEvent", "[18446744073709551615]"); + + ASSERT_EQ(cap.count, 1); + ASSERT_EQ(cap.args.size(), 1); + EXPECT_EQ(cap.args[0].typeId(), QMetaType::ULongLong) + << "expected qulonglong, got " << cap.args[0].typeName(); + EXPECT_EQ(cap.args[0].toULongLong(), 18446744073709551615ULL); +} + +// 2^53+1 is the smallest integer a double cannot represent. It is well inside +// int64 range, so this fails on any double round-trip while staying clear of +// the signed/unsigned question — it separates "degraded to double" from +// "unsigned not represented". +TEST(EventPayloadFidelity, IntegerPast2Pow53IsNotRounded) +{ + const Captured cap = captureEvent("intEvent", "[9007199254740993]"); + + ASSERT_EQ(cap.args.size(), 1); + EXPECT_EQ(cap.args[0].toLongLong(), 9007199254740993LL); +} + +TEST(EventPayloadFidelity, NegativeInt64MinSurvives) +{ + const Captured cap = captureEvent("intEvent", "[-9223372036854775808]"); + + ASSERT_EQ(cap.args.size(), 1); + EXPECT_EQ(cap.args[0].toLongLong(), std::numeric_limits::min()); +} + +// A large integer nested in a container, not just as a top-level element — +// containers were where the method-path equivalent (M1) hid. +TEST(EventPayloadFidelity, LargeIntegerNestedInContainersSurvives) +{ + const Captured cap = captureEvent( + "nestedEvent", R"([{"n": 18446744073709551615}, [9007199254740993]])"); + + ASSERT_EQ(cap.args.size(), 2); + EXPECT_EQ(cap.args[0].toMap().value("n").toULongLong(), 18446744073709551615ULL); + EXPECT_EQ(cap.args[1].toList().at(0).toLongLong(), 9007199254740993LL); +} + +// --- Bytes ---------------------------------------------------------------- +// Canonical tagged bytes must decode to a QByteArray, exactly as they do on the +// method path. Today they survive end-to-end only because the untouched +// {"_bytes": ...} object round-trips as a QVariantMap and a downstream consumer +// decodes the tag — which is not the same thing as the bridge decoding it. +TEST(EventPayloadFidelity, TaggedBytesDecodeToByteArray) +{ + const Captured cap = captureEvent("bytesEvent", R"([{"_bytes": "YQBiAGM"}])"); + + ASSERT_EQ(cap.args.size(), 1); + EXPECT_EQ(cap.args[0].typeId(), QMetaType::QByteArray) + << "expected QByteArray, got " << cap.args[0].typeName(); + EXPECT_EQ(cap.args[0].toByteArray(), QByteArray("a\0b\0c", 5)); +} + +TEST(EventPayloadFidelity, TaggedBytesNestedInContainerDecode) +{ + const Captured cap = captureEvent("bytesEvent", R"([[{"_bytes": "YQBiAGM"}]])"); + + ASSERT_EQ(cap.args.size(), 1); + const QVariantList inner = cap.args[0].toList(); + ASSERT_EQ(inner.size(), 1); + EXPECT_EQ(inner.at(0).typeId(), QMetaType::QByteArray); +} + +// --- Shapes that already work: guard against a fix regressing them --------- +TEST(EventPayloadFidelity, MultipleParametersKeepOrderAndTypes) +{ + const Captured cap = captureEvent("tripleEvent", R"([42, "hi", true])"); + + ASSERT_EQ(cap.args.size(), 3); + EXPECT_EQ(cap.args[0].toLongLong(), 42); + EXPECT_EQ(cap.args[1].toString(), QStringLiteral("hi")); + EXPECT_EQ(cap.args[2].toBool(), true); +} + +// Converging on the method path's helper also converges its SIGNEDNESS rule: +// nlohmannArgsToQVariantList classifies every non-negative integer as unsigned, +// so a LIDL `int` event argument now arrives as ULongLong rather than LongLong. +// That is what nlohmannToQVariant (methods) and the cdylib emitTrampoline +// already did, so this makes the surfaces agree — but it is an observable +// metatype change, pinned here so it stays a decision rather than a side effect. +// Value-level reads (toLongLong/toULongLong) are unaffected either way. +TEST(EventPayloadFidelity, NonNegativeIntegerCarriesUnsignedMetatype) +{ + const Captured cap = captureEvent("intEvent", "[42]"); + + ASSERT_EQ(cap.args.size(), 1); + EXPECT_EQ(cap.args[0].typeId(), QMetaType::ULongLong); + EXPECT_EQ(cap.args[0].toLongLong(), 42); + EXPECT_EQ(cap.args[0].toULongLong(), 42ULL); +} + +// A negative integer keeps the signed metatype — the classification is by value, +// not by declared LIDL type, so this is the other half of the rule. +TEST(EventPayloadFidelity, NegativeIntegerCarriesSignedMetatype) +{ + const Captured cap = captureEvent("intEvent", "[-42]"); + + ASSERT_EQ(cap.args.size(), 1); + EXPECT_EQ(cap.args[0].typeId(), QMetaType::LongLong); + EXPECT_EQ(cap.args[0].toLongLong(), -42); +} + +TEST(EventPayloadFidelity, DoubleStaysDouble) +{ + const Captured cap = captureEvent("doubleEvent", "[3.5]"); + + ASSERT_EQ(cap.args.size(), 1); + EXPECT_EQ(cap.args[0].typeId(), QMetaType::Double); + EXPECT_DOUBLE_EQ(cap.args[0].toDouble(), 3.5); +} + +TEST(EventPayloadFidelity, EmptyPayloadYieldsNoArguments) +{ + const Captured cap = captureEvent("bareEvent", "[]"); + + ASSERT_EQ(cap.count, 1); + EXPECT_EQ(cap.args.size(), 0); +} + +TEST(EventPayloadFidelity, NullElementSurvivesAsAnElement) +{ + const Captured cap = captureEvent("nullEvent", R"(["a", null, "b"])"); + + ASSERT_EQ(cap.args.size(), 3); + EXPECT_TRUE(cap.args[1].isNull()); +} + +// A non-array payload is the documented fallback: it is handed over as a single +// string argument rather than dropped. Pinned so a fix keeps the behaviour. +TEST(EventPayloadFidelity, NonArrayPayloadFallsBackToSingleStringArgument) +{ + const Captured cap = captureEvent("rawEvent", "not json at all"); + + ASSERT_EQ(cap.args.size(), 1); + EXPECT_EQ(cap.args[0].toString(), QStringLiteral("not json at all")); +} diff --git a/tests/protocol/test_plain_uint64.cpp b/tests/protocol/test_plain_uint64.cpp new file mode 100644 index 0000000..4f996f1 --- /dev/null +++ b/tests/protocol/test_plain_uint64.cpp @@ -0,0 +1,228 @@ +#include + +#include "json_codec.h" +#include "cbor_codec.h" +#include "qvariant_rpc_value.h" + +#include +#include +#include + +#include +#include + +// --------------------------------------------------------------------------- +// uint64 across the plain (tcp / tcp_ssl) wire. +// +// RpcValue's variant had no unsigned alternative, so every uint64 above +// int64max was squeezed through int64_t and arrived as -1 — independently in +// both directions: +// +// outbound qvariant_rpc_value.cpp QMetaType::ULongLong -> int64_t(...) +// inbound json_mapping.cpp is_number_unsigned -> get() +// +// Neither wraps loudly: .get() past int64max returns -1 with no +// exception. Two peers both running this code agreed on -1, so nothing looked +// broken from inside. +// +// This was NOT a wire-format limitation. Both codecs carry uint64 natively +// (CBOR emits major type 0, `1b ff..ff`), and the envelope's own `id` field +// already crossed this wire as a uint64_t. Only RpcValue *payloads* could not +// represent it. +// +// The plain tier had no integer test outside int32 range before this file: +// test_json_codec and test_cbor_codec used 42 and 3. +// --------------------------------------------------------------------------- + +using namespace logos::plain; + +namespace { + +constexpr uint64_t kUint64Max = std::numeric_limits::max(); +constexpr uint64_t kInt64Max = static_cast(std::numeric_limits::max()); + +AnyMessage roundtrip(IWireCodec& codec, const AnyMessage& msg) +{ + auto bytes = codec.encode(msg); + return codec.decode(messageTypeOf(msg), bytes.data(), bytes.size()); +} + +EventMessage eventCarrying(std::vector data) +{ + EventMessage e; + e.object = "test_fullapi"; + e.eventName = "uintEvent"; + e.data = std::move(data); + return e; +} + +} // namespace + +// --- The representation rule ---------------------------------------------- +// The unsigned alternative is used ONLY where int64_t loses information. +// Anything broader would change the representation of every non-negative +// integer already crossing this wire — and std::variant equality compares the +// alternative index, so it would also break comparisons against int64-built +// values, to fix nothing. + +TEST(PlainUint64, MakeIntegerUsesSignedAlternativeWhenItFits) +{ + EXPECT_TRUE(RpcValue::makeInteger(0).isInt()); + EXPECT_TRUE(RpcValue::makeInteger(42).isInt()); + EXPECT_TRUE(RpcValue::makeInteger(kInt64Max).isInt()); + EXPECT_EQ(RpcValue::makeInteger(kInt64Max).asInt(), + std::numeric_limits::max()); + + // Unchanged representation means unchanged equality. + EXPECT_EQ(RpcValue::makeInteger(42), RpcValue{int64_t(42)}); +} + +TEST(PlainUint64, MakeIntegerUsesUnsignedAlternativeOnlyPastInt64Max) +{ + EXPECT_TRUE(RpcValue::makeInteger(kInt64Max + 1).isUInt()); + EXPECT_TRUE(RpcValue::makeInteger(kUint64Max).isUInt()); + EXPECT_EQ(RpcValue::makeInteger(kUint64Max).asUInt(), kUint64Max); +} + +TEST(PlainUint64, IsIntegralCoversBothAlternatives) +{ + EXPECT_TRUE(RpcValue::makeInteger(42).isIntegral()); + EXPECT_TRUE(RpcValue::makeInteger(kUint64Max).isIntegral()); + EXPECT_FALSE(RpcValue{3.5}.isIntegral()); + EXPECT_FALSE(RpcValue{std::string("7")}.isIntegral()); +} + +// --- JSON codec ------------------------------------------------------------ + +TEST(PlainUint64, JsonCodecRoundTripsUint64Max) +{ + JsonCodec codec; + const AnyMessage out = roundtrip( + codec, eventCarrying({RpcValue::makeInteger(kUint64Max)})); + + const auto* evt = std::get_if(&out); + ASSERT_NE(evt, nullptr); + ASSERT_EQ(evt->data.size(), 1u); + ASSERT_TRUE(evt->data[0].isUInt()) << "decoded into the wrong alternative"; + EXPECT_EQ(evt->data[0].asUInt(), kUint64Max); +} + +TEST(PlainUint64, JsonCodecRoundTripsUint64NestedInContainers) +{ + RpcMap m; + m.emplace("n", RpcValue::makeInteger(kUint64Max)); + RpcList l; + l.items.push_back(RpcValue::makeInteger(kInt64Max + 1)); + + JsonCodec codec; + const AnyMessage out = roundtrip( + codec, eventCarrying({RpcValue{std::move(m)}, RpcValue{std::move(l)}})); + + const auto* evt = std::get_if(&out); + ASSERT_NE(evt, nullptr); + ASSERT_EQ(evt->data.size(), 2u); + EXPECT_EQ(evt->data[0].asMap().at("n").asUInt(), kUint64Max); + EXPECT_EQ(evt->data[1].asList().items.at(0).asUInt(), kInt64Max + 1); +} + +TEST(PlainUint64, JsonCodecKeepsNegativeIntegersSigned) +{ + JsonCodec codec; + const AnyMessage out = roundtrip( + codec, eventCarrying({RpcValue{std::numeric_limits::min()}})); + + const auto* evt = std::get_if(&out); + ASSERT_NE(evt, nullptr); + ASSERT_TRUE(evt->data[0].isInt()); + EXPECT_EQ(evt->data[0].asInt(), std::numeric_limits::min()); +} + +// --- CBOR codec ------------------------------------------------------------ + +TEST(PlainUint64, CborCodecRoundTripsUint64Max) +{ + CborCodec codec; + const AnyMessage out = roundtrip( + codec, eventCarrying({RpcValue::makeInteger(kUint64Max)})); + + const auto* evt = std::get_if(&out); + ASSERT_NE(evt, nullptr); + ASSERT_EQ(evt->data.size(), 1u); + ASSERT_TRUE(evt->data[0].isUInt()); + EXPECT_EQ(evt->data[0].asUInt(), kUint64Max); +} + +TEST(PlainUint64, CborCodecRoundTripsUint64NestedInContainers) +{ + RpcMap m; + m.emplace("n", RpcValue::makeInteger(kUint64Max)); + + CborCodec codec; + const AnyMessage out = roundtrip(codec, eventCarrying({RpcValue{std::move(m)}})); + + const auto* evt = std::get_if(&out); + ASSERT_NE(evt, nullptr); + EXPECT_EQ(evt->data[0].asMap().at("n").asUInt(), kUint64Max); +} + +// --- The Qt boundary, both directions -------------------------------------- + +TEST(PlainUint64, QVariantToRpcValuePreservesUint64) +{ + const QVariant v = QVariant::fromValue(kUint64Max); + const RpcValue r = qvariantToRpcValue(v); + + ASSERT_TRUE(r.isUInt()) << "wrapped to int64 again"; + EXPECT_EQ(r.asUInt(), kUint64Max); +} + +TEST(PlainUint64, RpcValueToQVariantPreservesUint64) +{ + const QVariant v = rpcValueToQVariant(RpcValue::makeInteger(kUint64Max)); + + EXPECT_EQ(v.typeId(), QMetaType::ULongLong) << "got " << v.typeName(); + EXPECT_EQ(v.toULongLong(), kUint64Max); +} + +TEST(PlainUint64, QtBoundaryRoundTripIsExact) +{ + const QVariant in = QVariant::fromValue(kUint64Max); + const QVariant out = rpcValueToQVariant(qvariantToRpcValue(in)); + + EXPECT_EQ(out.toULongLong(), kUint64Max); +} + +// A uint that fits int64 keeps crossing as signed, exactly as before. Pinned so +// the narrow rule stays visible rather than assumed. +TEST(PlainUint64, SmallUnsignedStillCrossesAsSigned) +{ + const QVariant in = QVariant::fromValue(42); + const RpcValue r = qvariantToRpcValue(in); + + EXPECT_TRUE(r.isInt()); + EXPECT_EQ(rpcValueToQVariant(r).typeId(), QMetaType::LongLong); +} + +// --- The off-by-one in the QJsonValue::Double guard ------------------------ +// double(int64max) rounds UP to exactly 2^63, so `d <= double(int64max)` used to +// admit 2^63 and then run int64_t(d) out of range — undefined behaviour, +// saturating on arm64 and INT64_MIN on x86-64. + +TEST(PlainUint64, DoubleAtTwoPow63DoesNotEnterTheIntegerBranch) +{ + const QVariant v = QVariant(QJsonValue(9223372036854775808.0)); // 2^63 + const RpcValue r = qvariantToRpcValue(v); + + EXPECT_FALSE(r.isInt()) << "2^63 is not representable as int64_t"; + ASSERT_TRUE(r.isDouble()); + EXPECT_DOUBLE_EQ(r.asDouble(), 9223372036854775808.0); +} + +TEST(PlainUint64, DoubleAtInt64MinStillTakesTheIntegerBranch) +{ + const QVariant v = QVariant(QJsonValue(-9223372036854775808.0)); // -2^63, exact + const RpcValue r = qvariantToRpcValue(v); + + ASSERT_TRUE(r.isInt()); + EXPECT_EQ(r.asInt(), std::numeric_limits::min()); +}