From d5ba950313b3e4bb9d42d97af4c28df9083cfb41 Mon Sep 17 00:00:00 2001 From: Dario Lipicar Date: Fri, 17 Jul 2026 19:40:26 -0300 Subject: [PATCH] fix(json): keep nested bytes/ints tagged in qvariantToNlohmann containers (#23) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit qvariantToNlohmann ran its canConvert/ fallbacks BEFORE the type-preserving QVariantList/QVariantMap recursion. A QVariantList/QVariantMap also reports canConvert()==true, so a container was routed through QJson — which has no byte type and degrades numerics to double. A nested QByteArray was therefore flattened to a plain string, losing the canonical {"_bytes":...} tag. Concretely this broke bstr method ARGUMENTS to cdylib (Rust) modules: LogosProviderObject::callMethodStdBridge feeds each call arg through qvariantToNlohmann, and a bstr arg arrives (over QtRO) as a QByteArray nested in the QVariantList of call args. It was flattened to "hello", so the cdylib's {"_bytes":...} decoder produced an empty Vec (e.g. echoBytes returned null). The QtRO C++ path was unaffected (native QByteArray marshaling) and the plain-lp path was already correct; only the container-through-QVariant leg dropped the tag. Fix: move the container recursion (QStringList/QVariantList/QVariantMap) ahead of the QJson fallbacks so nested elements recurse element-by-element (bytes stay tagged, integers stay integers); only genuine QJson-typed variants reach the fallbacks. Adds nested-bytes-in-list/map + bridge-shape regression tests. Co-authored-by: Claude Opus 4.8 --- cpp/logos_json_convert.cpp | 30 ++++++---- tests/protocol/test_json_convert_bytes.cpp | 65 ++++++++++++++++++++++ 2 files changed, 84 insertions(+), 11 deletions(-) diff --git a/cpp/logos_json_convert.cpp b/cpp/logos_json_convert.cpp index dc3c1fa..3a5a00c 100644 --- a/cpp/logos_json_convert.cpp +++ b/cpp/logos_json_convert.cpp @@ -60,17 +60,6 @@ nlohmann::json qvariantToNlohmann(const QVariant& v) return obj; } - if (v.canConvert()) { - QJsonDocument doc(v.toJsonObject()); - try { return nlohmann::json::parse(doc.toJson(QJsonDocument::Compact).toStdString()); } - catch (...) {} - } - if (v.canConvert()) { - QJsonDocument doc(qvariant_cast(v)); - try { return nlohmann::json::parse(doc.toJson(QJsonDocument::Compact).toStdString()); } - catch (...) {} - } - // Integers stay integers: QJsonValue::fromVariant degrades every numeric // to double, which a strict consumer on the other side of the C ABI // (e.g. a generated dispatch reading an int param) must not see as 5.0. @@ -109,6 +98,25 @@ nlohmann::json qvariantToNlohmann(const QVariant& v) return obj; } + // Fallbacks for QVariants that are natively Qt JSON types (e.g. a module + // handed back a QJsonObject/QJsonArray directly). These run AFTER the + // container recursion above ON PURPOSE: a QVariantList/QVariantMap also + // reports canConvert(), and routing it through QJson here + // would flatten a nested QByteArray to a plain string (QJson has no byte + // type) and degrade nested numerics to double — the exact losses the + // recursion prevents. So containers must be handled first; only genuine + // QJson-typed variants reach this point. + if (v.canConvert()) { + QJsonDocument doc(v.toJsonObject()); + try { return nlohmann::json::parse(doc.toJson(QJsonDocument::Compact).toStdString()); } + catch (...) {} + } + if (v.canConvert()) { + QJsonDocument doc(qvariant_cast(v)); + try { return nlohmann::json::parse(doc.toJson(QJsonDocument::Compact).toStdString()); } + catch (...) {} + } + QJsonValue jv = QJsonValue::fromVariant(v); if (jv.isString()) return jv.toString().toStdString(); if (jv.isBool()) return jv.toBool(); diff --git a/tests/protocol/test_json_convert_bytes.cpp b/tests/protocol/test_json_convert_bytes.cpp index 0395463..50e9f93 100644 --- a/tests/protocol/test_json_convert_bytes.cpp +++ b/tests/protocol/test_json_convert_bytes.cpp @@ -97,6 +97,71 @@ TEST(JsonConvertBytes, LogosResultValueBytesAreTagged) EXPECT_EQ(back.toByteArray(), QByteArray("p\0q", 3)); } +// Bytes NESTED inside a container argument must keep the tagged form. +// LogosProviderObject::callMethodStdBridge converts each call argument +// individually via qvariantToNlohmann, so a top-level QByteArray argument +// already hits the tagged case above — the break was specifically an argument +// that is itself a QVariantList/QVariantMap CONTAINING a QByteArray. In the +// cdylib path this is exactly callModuleMethod's 3rd argument (the nested +// call-args list), which over QtRO arrives as a QVariantList holding the bstr +// param as a QByteArray. Before the fix, the canConvert/ +// fallbacks caught that container FIRST and routed it through QJson, which has no +// byte type — the nested QByteArray was flattened to a plain string, so a Rust +// cdylib's {"_bytes":...} decoder saw "hello" and produced an empty Vec +// (echoBytes → null). +TEST(JsonConvertBytes, NestedByteArrayInListStaysTagged) +{ + const QVariantList list{QVariant(QByteArray("hi\0!", 4))}; + nlohmann::json j = qvariantToNlohmann(QVariant(list)); + + ASSERT_TRUE(j.is_array()) << j.dump(); + ASSERT_EQ(j.size(), 1u); + ASSERT_TRUE(j[0].is_object()) << j.dump(); + ASSERT_TRUE(j[0].contains("_bytes")) << j.dump(); + + QVariant back = nlohmannToQVariant(j[0]); + ASSERT_EQ(back.userType(), QMetaType::QByteArray); + EXPECT_EQ(back.toByteArray(), QByteArray("hi\0!", 4)); +} + +TEST(JsonConvertBytes, NestedByteArrayInMapStaysTagged) +{ + QVariantMap m; + m.insert("blob", QVariant(QByteArray("p\0q", 3))); + m.insert("name", QStringLiteral("n")); + nlohmann::json j = qvariantToNlohmann(QVariant(m)); + + ASSERT_TRUE(j.is_object()) << j.dump(); + ASSERT_TRUE(j["blob"].is_object()) << j.dump(); + ASSERT_TRUE(j["blob"].contains("_bytes")) << j.dump(); + EXPECT_EQ(j["name"].get(), "n"); + + QVariant back = nlohmannToQVariant(j["blob"]); + ASSERT_EQ(back.userType(), QMetaType::QByteArray); + EXPECT_EQ(back.toByteArray(), QByteArray("p\0q", 3)); +} + +TEST(JsonConvertBytes, ByteArrayArgInListSurvivesBridgeConversion) +{ + // A list-valued argument that contains a bstr — the shape of + // callModuleMethod's nested call-args argument, which callMethodStdBridge + // hands to qvariantToNlohmann as a single QVariantList. It must yield + // [{"_bytes":...}] so the next hop's nlohmannArgsToQVariantList recovers a + // QByteArray, not the string "hello". + const QVariantList callArgs{QVariant(QByteArray("hello"))}; + nlohmann::json jArgs = qvariantToNlohmann(QVariant(callArgs)); + + ASSERT_TRUE(jArgs.is_array()) << jArgs.dump(); + ASSERT_EQ(jArgs.size(), 1u); + ASSERT_TRUE(jArgs[0].is_object()) << jArgs.dump(); + ASSERT_TRUE(jArgs[0].contains("_bytes")) << jArgs.dump(); + + const QVariantList redecoded = nlohmannArgsToQVariantList(jArgs); + ASSERT_EQ(redecoded.size(), 1); + ASSERT_EQ(redecoded[0].userType(), QMetaType::QByteArray); + EXPECT_EQ(redecoded[0].toByteArray(), QByteArray("hello")); +} + TEST(JsonConvertBytes, OrdinaryObjectsAreNotMistakenForBytes) { // Two keys → a real map, even though one key is "_bytes".