mirror of
https://github.com/logos-co/logos-cpp-sdk.git
synced 2026-08-31 01:31:10 +00:00
fix(generator): widen BOTH rejection detectors to a CLOSED SET of codes
The two emitted detectors — logosDispatchRejection (QVariant, Qt surface) and
logosDispatchRejectionJson (nlohmann, lp surface) — each matched the single
literal "dispatch_failed". Providers have been answering a wrong argument COUNT
with "invalid_args" all along: this repo's own cdylib dispatch emits it
(experimental/lidl_gen_cdylib.cpp:805) and so does logos-rust-sdk's
args::invalid_args. Nothing detected it. The refusal therefore arrived as a
VALUE and the return table erased it — `_result.toList()` on that map is `[]`,
`.toString()` is "", `.toLongLong()` is 0 — so a caller could not tell "you sent
me the wrong number of arguments" from "the provider returned nothing".
Measured on the untyped surface, where the erasure is visible:
logosctl call test_basic_module isPositive (missing required argument)
-> exit 0, status:"ok", result {"code":"invalid_args", ...}
The set is now {dispatch_failed, invalid_args, unknown_method}, in ONE
kRejectionCodes array. Both emitters build their condition text from it, so the
Qt and Qt-free twins cannot drift apart — which is what two hand-written copies
of the same literal were always going to do.
"unknown_method" is listed before any provider emits it, on purpose. An unknown
method is currently answered with a bare null, byte-identical to a legitimate
null return (logos-protocol logos_protocol.h says so outright), and closing that
is a provider-contract change across the SDKs. Detectors go first because
widening one is backwards-compatible on its own — nothing emits the code, so
nothing changes — whereas a new provider code shipped against narrow detectors
would arrive at consumers as DATA: the same silent-success bug, freshly minted.
The set stays CLOSED. NOT "any three-key object with a code": a method may
legitimately return a three-string map, and an `any` return certainly can, so a
shape-only match would let user data impersonate a refusal. Every guard above
the compare — exactly three keys, all three present, all three strings — is
untouched.
WHY FIVE COPIES AND NOT ONE. The other four are logos-qt-sdk's byte-identical
lidl_gen_qt_consumer.cpp, logos-rust-sdk's args::as_dispatch_rejection, and
logos-logoscore-cli's core_service/call_envelope.cpp. Two shared homes were
considered, both rejected here and both recorded at kRejectionCodes: a shared
EMITTER in share/lidl-frontend (the channel exists — it already ships
lidl_emit_common to logos-qt-generator) collapses 2 of 5 and turns four
independently landable fixes into an ordered stack; a runtime predicate in
logos-protocol is the principled end state, and is how the analogous CONVERSION
duplication was actually solved, but it trades a text-level duplication for a
build-level version coupling — the emitted body is self-contained today, so a
wrapper compiles against whatever protocol its module pins. Each repo instead
holds the vocabulary in one named place, so a drift is visible.
Tests: each code asserted present in the emitted condition on both surfaces,
plus the negatives that keep the match closed — exactly three comparisons and
no more, the shape guards still emitted, and the compare still ahead of
`return true`. 316/316 ctest.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
committed by
Dario Lipicar
co-authored by
Claude Opus 5
parent
dbe1d63677
commit
b505ee95eb
@@ -799,13 +799,91 @@ QString makeHeader(const QString& moduleName, const QString& className, const QJ
|
||||
return h;
|
||||
}
|
||||
|
||||
// ---------------------------------------------------------------------------
|
||||
// The provider REJECTION codes, as a CLOSED SET.
|
||||
//
|
||||
// This array is the single source of truth for BOTH detectors emitted below —
|
||||
// the nlohmann::json one for the lp surface and the QVariant one for the Qt
|
||||
// surface. They used to spell the literal out separately, which is exactly how
|
||||
// two detectors drift apart; the condition text is now built from here, so a
|
||||
// code added to this array reaches both emitters or neither.
|
||||
//
|
||||
// Why a closed set and not "any {code,message,origin} object": a method may
|
||||
// legitimately RETURN a three-string map, and an `any` return certainly can.
|
||||
// Matching the shape alone would let user data impersonate a refusal. The set
|
||||
// is what keeps the in-band signal narrow.
|
||||
//
|
||||
// "dispatch_failed" — the provider ran and refused well-formed-looking
|
||||
// arguments (a type it could not decode).
|
||||
// "invalid_args" — wrong argument COUNT. Emitted today by the generated
|
||||
// cdylib dispatch (experimental/lidl_gen_cdylib.cpp) and
|
||||
// by logos-rust-sdk `args::invalid_args`, and until now
|
||||
// detected by nobody: `logosctl call m isPositive` with
|
||||
// the argument missing exited 0 with status "ok" and the
|
||||
// refusal object as its RESULT.
|
||||
// "unknown_method" — NOT emitted by any provider yet. Listed now on purpose.
|
||||
// logos_protocol.h records that an unknown method is
|
||||
// currently answered with a bare null, indistinguishable
|
||||
// from a legitimate null return, and that closing it
|
||||
// needs a provider-contract change across the SDKs. The
|
||||
// detector has to be widened FIRST: widening is
|
||||
// backwards-compatible on its own (nothing emits the code,
|
||||
// so nothing changes), whereas a new provider code shipped
|
||||
// against old detectors would arrive at consumers as DATA
|
||||
// — the same silent-success bug, freshly minted.
|
||||
//
|
||||
// WHY THIS IS A PER-REPO CONSTANT AND NOT A SHARED ONE. There are five copies of
|
||||
// this detector: the two emitted below, logos-qt-sdk's byte-identical
|
||||
// lidl_gen_qt_consumer.cpp, logos-rust-sdk's args::as_dispatch_rejection, and
|
||||
// logos-logoscore-cli's core_service/call_envelope.cpp. Two candidate shared
|
||||
// homes were considered and both rejected FOR NOW:
|
||||
//
|
||||
// * a shared EMITTER in share/lidl-frontend (which already ships
|
||||
// lidl_emit_common to logos-qt-generator, so the channel exists). It would
|
||||
// collapse 2 of the 5 — not the QVariant twin, not Rust, not core_service —
|
||||
// and it would make logos-qt-sdk's commit depend on this one plus a pin
|
||||
// bump, turning four independently landable fixes into an ordered stack for
|
||||
// no behavioural gain.
|
||||
// * a runtime predicate in logos-protocol that the generated code CALLS. This
|
||||
// is the principled end state, and it is how the analogous CONVERSION
|
||||
// duplication was actually solved (logos_json_convert, reached through
|
||||
// logos_qt_lp_bridge.h) rather than by sharing an emitter. It is a separate
|
||||
// change because it converts a TEXT-level duplication into a BUILD-level
|
||||
// version coupling: the emitted body is self-contained today, so a wrapper
|
||||
// generated by any generator compiles against any logos-protocol a module
|
||||
// happens to pin. Calling a protocol symbol ends that.
|
||||
//
|
||||
// So: copies stay, and each repo holds the vocabulary in ONE named place so a
|
||||
// drift between them is visible rather than silent. Here that place is this
|
||||
// array, and BOTH emitters below build their condition from it.
|
||||
// ---------------------------------------------------------------------------
|
||||
static const char* const kRejectionCodes[] = {
|
||||
"dispatch_failed", "invalid_args", "unknown_method",
|
||||
};
|
||||
|
||||
// `c != "a" && c != "b" && ...` over kRejectionCodes, with each literal passed
|
||||
// through `wrap` (identity for std::string, QStringLiteral for QString).
|
||||
static QString rejectionCodeMismatch(const QString& var,
|
||||
QString (*wrap)(const char*),
|
||||
const QString& joinIndent)
|
||||
{
|
||||
QStringList terms;
|
||||
for (const char* code : kRejectionCodes)
|
||||
terms << var + " != " + wrap(code);
|
||||
return terms.join("\n" + joinIndent + "&& ");
|
||||
}
|
||||
|
||||
static QString plainLiteral(const char* c) { return QString("\"") + c + "\""; }
|
||||
static QString qtLiteral(const char* c) { return QString("QStringLiteral(\"") + c + "\")"; }
|
||||
|
||||
// The Qt consumer's rejection detector, emitted once per generated wrapper.
|
||||
//
|
||||
// A provider that REJECTS a call answers the canonical
|
||||
// {"code":"dispatch_failed", "message":..., "origin":...} object as its RESULT,
|
||||
// not as a transport error. Every provider flavour produces the same object
|
||||
// (logos-qt-sdk `dispatchFailedVariant`, the generated cdylib dispatch, the Rust
|
||||
// provider's `args::dispatch_failed`), and the Qt return table converts it like
|
||||
// {"code":..., "message":..., "origin":...} object as its RESULT, with `code`
|
||||
// drawn from kRejectionCodes above, not as a transport error. Every provider
|
||||
// flavour produces the same object (logos-qt-sdk `dispatchFailedVariant`, the
|
||||
// generated cdylib dispatch, logos-rust-sdk's `args::dispatch_failed` and
|
||||
// `args::invalid_args`), and the Qt return table converts it like
|
||||
// any other value — which ERASES it: `_result.toList()` on a map is `[]`,
|
||||
// `.toString()` is "", `.toLongLong()` is 0. A caller then cannot tell "you sent
|
||||
// me the wrong thing" from "the provider returned nothing".
|
||||
@@ -819,9 +897,9 @@ QString makeHeader(const QString& moduleName, const QString& className, const QJ
|
||||
// several of these in ONE translation unit. Internal linkage handles the
|
||||
// separate-TU case; only the preprocessor handles this one.
|
||||
// The Qt-free twin of emitDispatchRejectionDetector, for the lp surface, whose
|
||||
// results arrive as nlohmann::json rather than QVariant. Same exact match on
|
||||
// the same three string fields and the same code, for the same reason: an `any`
|
||||
// or map return carrying user data must never false-match.
|
||||
// results arrive as nlohmann::json rather than QVariant. Same match on the same
|
||||
// three string fields against the same closed code set, for the same reason: an
|
||||
// `any` or map return carrying user data must never false-match.
|
||||
//
|
||||
// Guarded identically — the umbrella (`logos_sdk.cpp`) textually #includes every
|
||||
// generated `<dep>_api.cpp`, so a module with more than one dependency puts
|
||||
@@ -844,8 +922,9 @@ static void emitDispatchRejectionDetectorJson(QTextStream& s)
|
||||
s << " auto code = v.find(\"code\"), message = v.find(\"message\"), origin = v.find(\"origin\");\n";
|
||||
s << " if (code == v.end() || message == v.end() || origin == v.end()) return false;\n";
|
||||
s << " if (!code->is_string() || !message->is_string() || !origin->is_string()) return false;\n";
|
||||
s << " if (code->get<std::string>() != \"dispatch_failed\") return false;\n";
|
||||
s << " out.code = code->get<std::string>();\n";
|
||||
s << " const std::string _code = code->get<std::string>();\n";
|
||||
s << " if (" << rejectionCodeMismatch("_code", plainLiteral, " ") << ") return false;\n";
|
||||
s << " out.code = _code;\n";
|
||||
s << " out.message = message->get<std::string>();\n";
|
||||
s << " out.origin = origin->get<std::string>();\n";
|
||||
s << " return true;\n";
|
||||
@@ -862,9 +941,10 @@ static void emitDispatchRejectionDetector(QTextStream& s)
|
||||
s << "// True when `v` is the canonical provider REJECTION object rather than a\n";
|
||||
s << "// value; fills `out` with its {code, message, origin} on a match.\n";
|
||||
s << "//\n";
|
||||
s << "// The match is exact — those three fields, all strings, and that code — for the\n";
|
||||
s << "// same reason logos_rpc_status.h's isUnauthorizedSentinel is exact: an `any` or\n";
|
||||
s << "// map return carrying user data must never false-match.\n";
|
||||
s << "// The match is narrow — those three fields, all strings, and a code from the\n";
|
||||
s << "// CLOSED SET above — for the same reason logos_rpc_status.h's\n";
|
||||
s << "// isUnauthorizedSentinel is exact: an `any` or map return carrying user data\n";
|
||||
s << "// must never false-match. Any other code stays DATA.\n";
|
||||
s << "bool logosDispatchRejection(const QVariant& v, logos::CallError& out)\n";
|
||||
s << "{\n";
|
||||
s << " QVariantMap m;\n";
|
||||
@@ -881,8 +961,9 @@ static void emitDispatchRejectionDetector(QTextStream& s)
|
||||
s << " if (code.userType() != QMetaType::QString\n";
|
||||
s << " || message.userType() != QMetaType::QString\n";
|
||||
s << " || origin.userType() != QMetaType::QString) return false;\n";
|
||||
s << " if (code.toString() != QStringLiteral(\"dispatch_failed\")) return false;\n";
|
||||
s << " out.code = code.toString().toStdString();\n";
|
||||
s << " const QString _code = code.toString();\n";
|
||||
s << " if (" << rejectionCodeMismatch("_code", qtLiteral, " ") << ") return false;\n";
|
||||
s << " out.code = _code.toStdString();\n";
|
||||
s << " out.message = message.toString().toStdString();\n";
|
||||
s << " out.origin = origin.toString().toStdString();\n";
|
||||
s << " return true;\n";
|
||||
|
||||
Reference in New Issue
Block a user