Skip to content

fix: reject inconsistent point clouds; make ros_message_definitions.hpp includable twice - #141

Merged
facontidavide merged 1 commit into
mainfrom
fix/ros-msg-robustness
Sep 20, 2026
Merged

facontidavide merged 1 commit into
mainfrom
fix/ros-msg-robustness

Conversation

@facontidavide

Copy link
Copy Markdown
Owner

Summary

Three defects found while integrating Cloudini into pj_bridge, where point clouds arrive from arbitrary DDS peers. Each was reproduced by a test (or a link failure) before the fix.

  • Field outside the point → out-of-bounds read. The field encoders read SizeOf(type) bytes at field.offset inside every point, and nothing checked that the field fits in point_step. PointcloudEncoder now refuses such an EncodingInfo at construction, which covers every caller (ROS, PCL, Python, WASM).
  • width*height*point_step disagreeing with the data → undecodable output. convertPointCloud2ToCompressedCloud encodes data.size()/point_step points but the header repeats the message's width/height; decoders then fail with "ended before all declared points". It now throws instead.
  • ros_message_definitions.hpp could only be included once per binary. It defined non-inline const char* globals, so a second translation unit including it failed to link (multiple definition of 'compressed_schema_data'). They are now inline constexpr.

Tests

InconsistentPointCloud2IsRejected, EncoderRejectsFieldOutsidePointStep, and the header is now included from both test_ros_msg.cpp and test_header.cpp (which is what reproduced the link error). 28/28 pass.

Not changed

is_bigendian and PointField.count are still ignored by the deserializer — known limitations, left as they are.

🤖 Generated with Claude Code

https://claude.ai/code/session_01711BZcEtp3KsoF9uZHNFME

…pp includable twice

- PointcloudEncoder refuses a field that does not fit in point_step: the field
  encoders read at field.offset inside every point, so such a field read past
  the end of the cloud. Covers every caller (ROS, PCL, Python, WASM).
- convertPointCloud2ToCompressedCloud refuses a cloud whose
  width*height*point_step disagrees with its data size: the header repeats
  width/height, so the result could not be decoded.
- ros_message_definitions.hpp defined non-inline globals: including it from two
  translation units failed to link (multiple definition). Now inline constexpr.

Each was reproduced by a test (or a link failure) before the fix.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01711BZcEtp3KsoF9uZHNFME
@facontidavide
facontidavide merged commit eef5c03 into main Sep 20, 2026
2 of 6 checks passed
@facontidavide
facontidavide deleted the fix/ros-msg-robustness branch September 20, 2026 19:20
facontidavide added a commit that referenced this pull request Sep 20, 2026
facontidavide added a commit to PlotJuggler/plotjuggler_bridge that referenced this pull request Sep 20, 2026
…validation

Cloudini 1.3.1 (facontidavide/cloudini#141, #142) can be embedded as-is and
validates malformed clouds itself, so this removes what compensated for that:
the scoped fetch function (hidden ament_cmake, disabled zstd/PCL lookups,
undone CMAKE_BUILD_TYPE, FetchContent_Populate + EXCLUDE_FROM_ALL), the
data_path.hpp relocation, and check_cloud() in the transform. The
InconsistentCloudMetadataIsRejected test stays and now covers the upstream check.

Clean rebuild against the released tarball: 303 tests, static link, no PCL,
nothing of Cloudini installed.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01711BZcEtp3KsoF9uZHNFME
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.

1 participant