Skip to content

fix: validate data messages in Header mode - #22

Open
anilkir wants to merge 2 commits into
PX4:mainfrom
anilkir:fix/header-mode-data-size-validation
Open

anilkir wants to merge 2 commits into
PX4:mainfrom
anilkir:fix/header-mode-data-size-validation

Conversation

@anilkir

@anilkir anilkir commented Nov 5, 2026

Copy link
Copy Markdown

Problem

In StorageConfig::Header mode, DataContainer never records subscriptions because the header is marked complete on the first AddLoggedMessage. As a result:

  • isValidDataMessage() returns false for every data message so valid data near a corrupted region is skipped until a different message type comes in
  • data() skips the payload size check that FullLog mode runs
  • Invalid AddLoggedMessages inside a corrupted region are ignored instead of raising an error, so the reader keeps reading out of sync

Change

In Header mode, keep a msg_id -> MessageFormat map. No Subscription is created and no samples are stored so memory use stays the same as before. With the map:

  • addLoggedMessage() throws on a duplicate msg_id or a missing format (same as FullLog)
  • data() throws on an unknown msg_id or a payload size outside [minWireSizeBytes(), sizeBytes()]
  • isValidDataMessage() uses the map so recovery can resync on data messages

Testing

  • Added two test cases:
    • Header mode recovers on the next valid data message after injected corruption
    • Header mode rejects a wrong-size data message for a known msg_id
  • Both fail without the fix. All 8 tests pass.
  • On two real flight logs with corrupted regions, Header mode now delivers exactly the same data messages as FullLog mode. Before, one log had about 540 fewer messages in Header mode.

Behaviour change

Header-mode users now get a ParsingException (reported through error()) for invalid data or subscriptions, the same as FullLog mode, instead of having them silently ignored.

@bkueng bkueng left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you check CI as well? For clang-tidy you can add the pragma once to the ignore list.

Comment thread ulog_cpp/data_container.cpp Resolved
@anilkir
anilkir force-pushed the fix/header-mode-data-size-validation branch from 64e4e48 to adb65d7 Compare November 6, 2026 19:37
@anilkir
anilkir requested a review from bkueng November 6, 2026 19:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants