From 6b2d7bcacdc4c034e14fc43eb1580481e7d5e51a Mon Sep 17 00:00:00 2001 From: Davide Faconti Date: Sun, 9 Aug 2026 11:30:02 +0200 Subject: [PATCH] fix(message-parser): gate the functional route on the bound schema MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit trampoline_get_plugin_extension advertised pj.parser_functional.v1 whenever the instance had ANY registered SchemaHandler. A mixed-model plugin — handlers for some schemas, legacy parse() for the rest, the shape parse()'s own doc-comment sanctions for a generic flattener — therefore claimed the functional route on every schema, and the final parseScalars/parseObject dispatchers then rejected each message for the unhandled ones ("parser does not register schema"). A 0.21 host that prefers the functional route fails every message on those topics. Once a schema is bound, advertisement now requires a handler for THAT schema. Before binding, any registered handler still advertises the capability: no schema-specific answer exists yet, and hosts are already required to re-query after binding rather than cache an earlier absence. Header-inline behavior fix; no ABI, vtable, protocol, or member-layout change. Closes #171 Co-Authored-By: Claude Fable 5 --- .../sdk/detail/message_parser_trampolines.hpp | 17 +++++--- ...ssage_parser_functional_extension_test.cpp | 43 +++++++++++++++++++ 2 files changed, 55 insertions(+), 5 deletions(-) diff --git a/pj_plugins/include/pj_plugins/sdk/detail/message_parser_trampolines.hpp b/pj_plugins/include/pj_plugins/sdk/detail/message_parser_trampolines.hpp index 5eef0e7..37be682 100644 --- a/pj_plugins/include/pj_plugins/sdk/detail/message_parser_trampolines.hpp +++ b/pj_plugins/include/pj_plugins/sdk/detail/message_parser_trampolines.hpp @@ -124,11 +124,18 @@ inline const void* MessageParserPluginBase::trampoline_get_plugin_extension(void auto* self = static_cast(ctx); try { std::string_view sv = id.data == nullptr ? std::string_view{} : std::string_view(id.data, id.size); - // Do not falsely advertise the functional route for a newly rebuilt - // plugin that still implements only legacy parse(). Schema handlers may - // be registered in the constructor or bindSchema(); hosts query again - // after binding and do not cache an earlier absence. - if (sv == PJ_PARSER_FUNCTIONAL_EXTENSION_V1 && !self->handlers_.empty()) { + // Once a schema is bound, advertise the functional route only if THIS + // schema has a handler. A mixed-model plugin may register handlers for + // some schemas and keep legacy parse() for the rest; gating on "any + // handler exists" would make the host prefer a functional route that + // parseScalars/parseObject then reject for the unhandled schema. Before + // binding, any registered handler still advertises the capability, since + // no schema-specific answer exists yet and hosts query again after + // binding without caching an earlier absence. + const bool functional_route_available = self->bound_type_name_.empty() + ? !self->handlers_.empty() + : self->findSchemaHandler(self->bound_type_name_) != nullptr; + if (sv == PJ_PARSER_FUNCTIONAL_EXTENSION_V1 && functional_route_available) { static const PJ_parser_functional_v1_t extension{ .struct_size = sizeof(PJ_parser_functional_v1_t), .parse_scalars = trampoline_parse_scalars_functional, diff --git a/pj_plugins/tests/message_parser_functional_extension_test.cpp b/pj_plugins/tests/message_parser_functional_extension_test.cpp index 14a1a5b..3f34066 100644 --- a/pj_plugins/tests/message_parser_functional_extension_test.cpp +++ b/pj_plugins/tests/message_parser_functional_extension_test.cpp @@ -78,6 +78,23 @@ class CustomExtensionParser final : public PJ::MessageParserPluginBase { int marker_ = 17; }; +/// Registers a handler for one schema and keeps legacy parse() for every +/// other schema — the mixed model the parse() doc-comment sanctions. +class MixedModelParser final : public PJ::MessageParserPluginBase { + public: + MixedModelParser() { + PJ::sdk::SchemaHandler handler; + handler.parse_scalars = [](PJ::Timestamp, PJ::Span) -> PJ::Expected { + return PJ::sdk::ScalarRecord{}; + }; + registerSchemaHandler(kSchema, std::move(handler)); + } + + PJ::Status parse(PJ::Timestamp, PJ::Span) override { + return PJ::okStatus(); + } +}; + class BindRegisteredParser final : public PJ::MessageParserPluginBase { public: PJ::Status bindSchema(std::string_view type_name, PJ::Span schema) override { @@ -275,6 +292,32 @@ TEST(MessageParserFunctionalExtension, HandlerRegisteredDuringBindEnablesExtensi EXPECT_TRUE(handle.supportsFunctionalParsing()); } +TEST(MessageParserFunctionalExtension, MixedModelParserAdvertisesOnlyForHandledSchemas) { + PJ::MessageParserHandle handled(parserVtable()); + ASSERT_TRUE(handled.bindSchema(kSchema, {})); + EXPECT_TRUE(handled.supportsFunctionalParsing()); + + // The same plugin bound to a schema it only implements through legacy + // parse() must not claim the functional route, or the host would take a + // route that parseScalars/parseObject can only reject. + PJ::MessageParserHandle unhandled(parserVtable()); + ASSERT_TRUE(unhandled.bindSchema("example/Unhandled", {})); + EXPECT_FALSE(unhandled.supportsFunctionalParsing()); + const auto status = unhandled.parseScalarsFunctional( + 0, {}, [](std::optional, PJ::Span) { return PJ::okStatus(); }); + EXPECT_FALSE(status); + EXPECT_NE(status.error().find(PJ_PARSER_FUNCTIONAL_EXTENSION_V1), std::string::npos); +} + +TEST(MessageParserFunctionalExtension, RebindingToAnUnhandledSchemaWithdrawsTheFunctionalRoute) { + PJ::MessageParserHandle handle(parserVtable()); + ASSERT_TRUE(handle.bindSchema(kSchema, {})); + ASSERT_TRUE(handle.supportsFunctionalParsing()); + + ASSERT_TRUE(handle.bindSchema("example/Unhandled", {})); + EXPECT_FALSE(handle.supportsFunctionalParsing()); +} + TEST(MessageParserFunctionalExtension, LegacyParserWithoutExtensionRemainsDetectable) { PJ::MessageParserHandle handle(legacyStyleVtable());