From f5dc2be9b880fb9168882e5a711a958d6e8933b1 Mon Sep 17 00:00:00 2001 From: Davide Faconti Date: Sat, 18 Jul 2026 19:29:12 +0200 Subject: [PATCH] fix(dialog): range-check table_delta indexes before narrowing (0.18.1) is_number_integer() accepts the full 64-bit range, but the decoder narrowed with an unchecked get(), so an oversized index (e.g. 2^32) silently wrapped into a valid-looking row instead of rejecting. Indexes are now range-checked against int before narrowing; any out-of-range value rejects the whole delta, matching the documented strict-decode contract. Also documents the host-side whole-delta rejection and seq-gate reset contract in dialog-plugin-guide.md, and removes the version-history comment block that had crept back into conanfile.py (CHANGELOG.md is the only ledger). Co-Authored-By: Claude Fable 5 --- CHANGELOG.md | 13 +++++++++- CMakeLists.txt | 2 +- conanfile.py | 25 +------------------ .../pj_plugins/host/widget_data_view.hpp | 24 +++++++++++++----- .../tests/widget_data_view_test.cpp | 13 ++++++++++ pj_plugins/docs/dialog-plugin-guide.md | 9 ++++--- recipe.yaml | 2 +- 7 files changed, 52 insertions(+), 36 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 7373100..3929f2f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,6 +3,17 @@ All notable changes to `plotjuggler_sdk` are recorded here. Versioning policy is in [`CLAUDE.md`](./CLAUDE.md) → "Release Versioning". +## [0.18.1] + +### Fix: strict index bounds in `WidgetDataView::tableDelta()` (PATCH) + +`table_delta` row/column indexes were validated with `is_number_integer()` but +then narrowed with an unchecked `get()`, so an oversized value (e.g. +`4294967296`) silently wrapped into a valid-looking index instead of rejecting. +Indexes are now range-checked against `int` before narrowing; any out-of-range +value rejects the whole delta, matching the documented strict-decode contract. +Header-only fix, no ABI change. + ## [0.18.0] ### Feature: batch table deltas for large QTableWidgets (MINOR) @@ -24,7 +35,7 @@ JSON addition; no C ABI change, `PJ_DIALOG_PROTOCOL_VERSION` unchanged): omit-unchanged-fields pattern (with measured costs) and the delta ops. - SDK-side only: hosts apply `table_delta` from the companion PlotJuggler change onward; older hosts ignore the key (harmless no-op). - + ### Feature: QDateTimeEdit event surface (MINOR) The dialog protocol's QDateTimeEdit setters (`setDateTime` / `setDateTimeRange`, diff --git a/CMakeLists.txt b/CMakeLists.txt index afa90a5..b1b0b0e 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -126,7 +126,7 @@ endif() if(PJ_INSTALL_SDK) include(CMakePackageConfigHelpers) - set(PJ_PACKAGE_VERSION "0.18.0") + set(PJ_PACKAGE_VERSION "0.18.1") set(PJ_PACKAGE_CMAKE_DIR ${CMAKE_INSTALL_LIBDIR}/cmake/plotjuggler_sdk) install(EXPORT plotjuggler_sdkTargets diff --git a/conanfile.py b/conanfile.py index 506d6fd..79dfcf6 100644 --- a/conanfile.py +++ b/conanfile.py @@ -30,30 +30,7 @@ class PlotjugglerSdkConan(ConanFile): name = "plotjuggler_sdk" - # 0.14.0 unified markers + transforms into the single host service - # `pj.data_processors.v1` via a `kind` discriminator (removed the old - # `pj.markers.v1` and the interim `pj.generators.v1`; generalized - # `create_data_processor`/`validate_data_processor_script` with - # kind/language/flags) and added the dialog-protocol additions (radio - # column + interactive sub-panel). Shipped as a MINOR bump rather than - # 1.0.0 because no public tag had ever carried `pj.data_processors.v1`, - # so no released plugin broke. - # 0.15.0 adds DataSource per-topic pause (advertise + demand-driven - # subscription) — strictly additive. See CHANGELOG.md. - # 0.16.0 removes the host-side PluginRuntimeCatalog (duplicate-resolution - # policy moved to the app, pj_runtime). No plugin links it and - # abi/baseline.abi is unchanged, so per the plugin-impact rule this is a - # MINOR, not a MAJOR. See CHANGELOG.md. - # 0.16.1 fixes plugin_data_api.hpp double formatting on Apple deployment - # targets older than macOS 13.3 (FP std::to_chars unavailable there) — - # PATCH, installed-header bug fix. See CHANGELOG.md. - # 0.17.0 extends the dialog protocol with backward-compatible additions - # (list_deletable / list_placeholder / chart_placeholder keys and the - # item_delete_index event) — MINOR. See CHANGELOG.md. - # 0.18.0 adds the QDateTimeEdit event surface (datetime_iso event, - # WidgetEvent::dateTimeChanged, DialogPluginTyped::onDateTimeChanged) — - # MINOR, header-only additions. See CHANGELOG.md. - version = "0.18.0" + version = "0.18.1" # Apache-2.0 covers the whole SDK (pj_base + pj_plugins). See LICENSE. license = "Apache-2.0" url = "https://github.com/PlotJuggler/plotjuggler_sdk" diff --git a/pj_plugins/dialog_protocol/include/pj_plugins/host/widget_data_view.hpp b/pj_plugins/dialog_protocol/include/pj_plugins/host/widget_data_view.hpp index a979544..260222b 100644 --- a/pj_plugins/dialog_protocol/include/pj_plugins/host/widget_data_view.hpp +++ b/pj_plugins/dialog_protocol/include/pj_plugins/host/widget_data_view.hpp @@ -5,6 +5,7 @@ #include #include #include +#include #include #include #include @@ -174,6 +175,16 @@ class WidgetDataView { // Strict decode: one malformed op rejects the whole delta. Partially // applying and then treating the seq as consumed would leave the table // diverged from the plugin's model with no way to repair it. + // is_number_integer() accepts the full 64-bit range, so indexes are + // range-checked before the narrowing get() — otherwise an oversized + // value (e.g. 2^32) would silently wrap into a valid-looking row. + const auto is_row_index = [](const nlohmann::json& v) { + if (!v.is_number_integer()) { + return false; + } + const auto value = v.get(); + return value >= 0 && value <= std::numeric_limits::max(); + }; TableDeltaView delta; delta.seq = *seq; if (auto append_it = it->find("append"); append_it != it->end()) { @@ -200,12 +211,13 @@ class WidgetDataView { return std::nullopt; } for (const auto& update : *cells_it) { - if (!update.is_array() || update.size() != 3 || !update[0].is_number_integer() || - !update[1].is_number_integer() || !update[2].is_string() || update[0].get() < 0 || - update[1].get() < 0) { + if (!update.is_array() || update.size() != 3 || !is_row_index(update[0]) || !is_row_index(update[1]) || + !update[2].is_string()) { return std::nullopt; } - delta.update_cells.push_back({update[0].get(), update[1].get(), update[2].get()}); + delta.update_cells.push_back( + {static_cast(update[0].get()), static_cast(update[1].get()), + update[2].get()}); } } if (auto remove_it = it->find("remove_rows"); remove_it != it->end()) { @@ -213,10 +225,10 @@ class WidgetDataView { return std::nullopt; } for (const auto& row : *remove_it) { - if (!row.is_number_integer() || row.get() < 0) { + if (!is_row_index(row)) { return std::nullopt; } - delta.remove_rows.push_back(row.get()); + delta.remove_rows.push_back(static_cast(row.get())); } // Normalized for direct application: descending and duplicate-free, so a // host removing in vector order can never hit index shift. diff --git a/pj_plugins/dialog_protocol/tests/widget_data_view_test.cpp b/pj_plugins/dialog_protocol/tests/widget_data_view_test.cpp index a5453f8..1ebb356 100644 --- a/pj_plugins/dialog_protocol/tests/widget_data_view_test.cpp +++ b/pj_plugins/dialog_protocol/tests/widget_data_view_test.cpp @@ -345,6 +345,19 @@ TEST(WidgetDataViewTest, TableDeltaNegativeIndexRejectsWholeDelta) { EXPECT_FALSE(view.tableDelta("tbl").has_value()); } +TEST(WidgetDataViewTest, TableDeltaOversizedIndexRejectsWholeDelta) { + // 2^32 passes is_number_integer() but would wrap to 0 through a narrowing + // get() — it must reject, not silently target a valid-looking row. + PJ::WidgetDataView remove_view(R"({"tbl": {"table_delta": {"seq": 3, "remove_rows": [4294967296]}}})"); + EXPECT_FALSE(remove_view.tableDelta("tbl").has_value()); + + PJ::WidgetDataView row_view(R"({"tbl": {"table_delta": {"seq": 3, "update_cells": [[4294967296, 0, "x"]]}}})"); + EXPECT_FALSE(row_view.tableDelta("tbl").has_value()); + + PJ::WidgetDataView col_view(R"({"tbl": {"table_delta": {"seq": 3, "update_cells": [[0, 4294967296, "x"]]}}})"); + EXPECT_FALSE(col_view.tableDelta("tbl").has_value()); +} + TEST(WidgetDataViewTest, TableDeltaRemoveRowsNormalizedDescendingUnique) { PJ::WidgetDataView view(R"({"tbl": {"table_delta": {"seq": 4, "remove_rows": [2, 7, 2, 5]}}})"); auto delta = view.tableDelta("tbl"); diff --git a/pj_plugins/docs/dialog-plugin-guide.md b/pj_plugins/docs/dialog-plugin-guide.md index 89478a2..c6adf88 100644 --- a/pj_plugins/docs/dialog-plugin-guide.md +++ b/pj_plugins/docs/dialog-plugin-guide.md @@ -678,9 +678,12 @@ discarding prior ops) and apply as: `update_cells`, then `remove_rows`, then `append`. **All indexes address the table as it was before the delta** — the plugin's own row space, the same space `setTableRows` and `setSelectedRows` use, unaffected by user sorting; ops cannot target rows the same delta -appends. A delta with any malformed op is rejected whole (never partially -applied). Sending `rows` in the same refresh wins: the host applies the full -replace and the delta counts as consumed. +appends. A delta with any malformed op — or any op the host cannot resolve +against the live table (e.g. an out-of-range row) — is rejected whole (never +partially applied) and its seq stays unconsumed, so a corrected retransmission +of the same seq applies. Sending `rows` in the same refresh wins: the host +applies the full replace and the delta counts as consumed; a `rows` resync also +resets the host's seq gate, so a restarted counter self-heals. > **Host support:** this SDK release ships the protocol (setters + the > `WidgetDataView::tableDelta` decoder); the PlotJuggler host applies deltas diff --git a/recipe.yaml b/recipe.yaml index 72ded35..8e6ae36 100644 --- a/recipe.yaml +++ b/recipe.yaml @@ -1,7 +1,7 @@ schema_version: 1 context: - version: "0.18.0" + version: "0.18.1" package: name: plotjuggler_sdk