Repository navigation
fix: six defects found in the pre-1.0 review (1.0.0) - #236
Merged
Merged
Conversation
facontidavide
force-pushed
the
fix/pre-1.0-bugfixes
branch
2 times, most recently
from
November 13, 2026 23:22
5bd1e86 to
f8ee1c8
Compare
- toStringView returned a view of `size` bytes past a one-byte literal for a
null pointer with a non-zero size; it now returns an empty view.
- The dialog marker decoders (chart markers, timeline marks) threw on a
wrong-typed field, which the host calls without a try/catch. They now skip
the malformed entry, like their sibling list decoders. The host view reuses
the shared decoder instead of a private copy; ChartMarkerView is an alias of
ChartMarker.
- DialogHandle kept a null context when the plugin's create() failed and then
passed it to the plugin. It gains valid() and its methods return their empty
value without calling the plugin.
- One non-UTF-8 string made WidgetData::toJson() throw, and the trampoline
replaced the whole update with {}. Invalid bytes now become U+FFFD.
- The Conan assert_throws option did not reach consumers, so their inline
PJ_ASSERT disagreed with the archives.
- Builtin object types are now host surfaces in the feature-floor table, so a
plugin emitting MeshMap or MeshMapUpdate must declare min_sdk_required 1.0.0.
Co-Authored-By: Claude Fable 5.2 <noreply@anthropic.com>
…w floor surface
toStringView was fixed, but four host-side readers of plugin-supplied views
carried their own copy of the same pattern and still read `size` bytes past a
literal: the service-registry lookup and the saveConfig() readers of the
DataSource, MessageParser and Toolbox handles (plus the parser write recorder
used in tests). A registry lookup with {nullptr, 64} was an ASAN
global-buffer-overflow; it is now an unknown service.
The changelog says so, and notes that an integer marker field no longer
accepts a float. ARCHITECTURE and the plugin skill list builtin object types
among the surfaces that set a host floor.
Co-Authored-By: Claude Fable 5.2 <noreply@anthropic.com>
facontidavide
force-pushed
the
fix/pre-1.0-bugfixes
branch
from
November 13, 2026 23:52
d1d72ee to
2284411
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
Six defects from the pre-1.0 review. None changes an ABI layout, a wire format or a public signature in a source-incompatible way. Each code fix has one regression test that was seen failing before the fix.
toStringViewreturned a view ofsizebytes past a one-byte literal for{nullptr, size > 0}. It is applied to plugin-supplied views in the host-side adapters.chartMarkers(),timelineMarksFromJson) used nlohmann's throwingvalue(). A plugin sending{"kind": 1}threw through the host, which calls them without a try/catch.chartSeries,rangeSliderMarkers) do.DialogHandlekept a null context when the plugin'screate()failed, then passed it to plugin trampolines that dereference it.valid(); each method returns its empty value without calling the plugin. Before the fix the new test hit a null dereference under ASAN.WidgetData::toJson()throw; the trampoline then replaced the whole update with{}, silently.WidgetEventBuilderserializers had the same unguardeddump()and use the same helper.assert_throwsoption did not reach consumers, so their inlinePJ_ASSERTdisagreed with the archives (ODR).package_info()adds the define to thebasecomponent, as the conan-center recipe already did.MeshMapwith a floor below 1.0.0 passed.PJ_builtin_object_type_tenumerators;feature_floors.jsongains a1.0.0group forkMeshMapandkMeshMapUpdate.Things a reviewer should know
chartMarkersFromJsonhad no callers while the host view re-implemented it, despite a comment calling it the single source of truth. The view now calls it, soChartMarkerViewis an alias ofChartMarker(same fields, order and defaults). PlotJuggler only uses the result throughauto.start: 3.7) used to be truncated and is now skipped. The SDK's writers always emit integers.baseline. I checked against git tags that each first shipped by 0.26.0, below the 0.28.0 minimum floor, so that is accurate.MeshMap/kMeshMapalso match a plugin that only reads meshes, which would work on an older host. It will be asked to raise its floor. That was chosen over letting an emitting plugin slip through.Host contract:paragraph that said the checker cannot see builtin types are rewritten.PlotJuggler follow-up (optional)
pj_dialog_host/src/dialog_engine.cppconstructs a parserDialogHandleand uses it at once. With fix 3 that no longer crashes, but a dialog whose constructor threw would show an empty UI. It should checkvalid().Verification
./build.sh --debugand./test.sh(Debug + ASAN): 107 of 107 passcheck_feature_floors.py: OK, 162 surfaces.test_declared_versions.py: 7 tests pass, including a newMeshMapFloorTestpre-commitcleanconan createwithassert_throws=True, then a consumer that fails to compile withoutPJ_ASSERT_THROWS: passesRebased onto
mainafter the 1.0.0-dev.1 commit; not rebuilt after the rebase. This PR and #230/#231 edit nearby lines infeature_floors.json,extract_host_surfaces.pyandconanfile.py; whichever merges later needs a small rebase.🤖 Generated with Claude Code