ros / ros_comm

ROS communications-related packages, including core client libraries (roscpp, rospy, roslisp) and graph introspection tools (rostopic, rosnode, rosservice, rosparam).
http://wiki.ros.org/ros_comm
753 stars 911 forks source link

rosbag reindex outputs invalid bags when run on truncated files #2282

Closed wkalt closed 1 year ago

wkalt commented 2 years ago

The rosbag reindex command is documented as "repairing broken bag files", but when run on a broken bag file it will output a bag that is indexed, but illegal per the ROS bag spec.

This script can be used to demonstrate the issue: https://gist.github.com/wkalt/836cd80f1c6a9b0e87e5e24771c7b8e0

Steps to reproduce:

wget https://assets.foxglove.dev/demo.bag
truncate -s 30000000 demo.bag
rosbag reindex demo.bag

// using go program above
$ go run main.go demo.bag
2022/09/27 09:18:10 got zero length header at offset 29578569
exit status 1

The resulting bag violates the spec in that it is not a sequence of back to back <header length><header><data length><data> records, with each header containing an "op" key, as the bag spec mandates. However, ROS tooling accepts the bag due to reliance on the message index, which is well-formed. The index simply does not point at the junk data present in the file, which occurs after the final chunk and its message indexes.

This causes problems for tooling that does not use the index and instead performs a linear scan. It is possible this issue does not present if you are lucky enough to truncate the file on a message boundary.

emersonknapp commented 2 years ago

I believe I have found the issue. I've opened #2286 as a draft, pending the addition of a regression test to validate the fix.

Your Go test program produces no errors given the same operation, with that above fix. If you are able, please verify the branch to help me make sure it's fixing the issue completely!