fix: reject deltas with invalid values entries#2460
Open
dirkwa wants to merge 1 commit intoSignalK:masterfrom
Open
fix: reject deltas with invalid values entries#2460dirkwa wants to merge 1 commit intoSignalK:masterfrom
dirkwa wants to merge 1 commit intoSignalK:masterfrom
Conversation
dirkwa
changed the title
Member
|
This does not fix #1473 as is ws specific. Would a better fix be to make security deny malformed deltas? I am not terribly crazy about adding the filter step to each and every delta. we could just ignore the whole delta as invalid if there is bad data. Processing overhead would be a bit less. It would affect only a very miniscule portion of data, as the vast majority is good. |
dirkwa
force-pushed
the
fix-null-values-crash
branch
from
ec0dc94 to
4676c7e
Compare
dirkwa
changed the title
Contributor
Author
|
Thanks for the quick feedback. All topics addressed. |
tkurki
reviewed
Mar 17, 2026
Malformed deltas with null entries or missing/null path in the values array caused shouldAllowWrite() to crash with "Cannot read properties of null". The entire delta is now rejected as invalid rather than filtering individual bad values, minimizing per-delta overhead. Validation is added in two places: - handleMessage (covers all entry points: WS, plugins, providers) - shouldAllowWrite (prevents crash before handleMessage in WS path) Fixes SignalK#1708, closes SignalK#1473
dirkwa
force-pushed
the
fix-null-values-crash
branch
from
4676c7e to
3eace22
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
2 participants
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.
Fixes #1708
Fixes #1473
Summary
Malformed deltas with null entries or pathvalues with missing/null
pathin thevaluesarray crashshouldAllowWrite()with "Cannot read properties of null". The entire delta is rejected as invalid rather than filtering individual bad values, keeping per-delta processing overhead minimal.Validation covers all entry points:
handleMessageinindex.ts— rejects before any processing (source assignment, timestamps, delta chain), covers WS, plugins, and providersshouldAllowWriteintokensecurity.ts— prevents crash in the WS security check that runs beforehandleMessageTested manually against server by sending deltas over WebSocket:
nullentry in values array → entire delta rejected, warning loggedpath({value: 3.14}) → rejectedpath: null→ rejectedServer console output confirmed three rejection warnings followed by successful GET of the valid value.