Skip to content

Fix remaining ReduceSDFExtension*FrameReplace tinyxml2 bugs - #1691

Open
trippyogi wants to merge 2 commits into
gazebosim:mainfrom
trippyogi:fix/746-reducesdf-frame-replace-leftovers
Open

Fix remaining ReduceSDFExtension*FrameReplace tinyxml2 bugs#1691
trippyogi wants to merge 2 commits into
gazebosim:mainfrom
trippyogi:fix/746-reducesdf-frame-replace-leftovers

Conversation

@trippyogi

@trippyogi trippyogi commented Aug 3, 2026

Copy link
Copy Markdown

🦟 Bug fix

Fixes #746

Summary

ReduceSDFExtensionContactSensorFrameReplace, ReduceSDFExtensionGripperFrameReplace, and ReduceSDFExtensionProjectorFrameReplace still walked the tinyxml2 document instead of the blob root element after the #745 / #1089 fixes for joint/plugin. Nested tags (contact/collision, gripper_link/palm_link, plugin projector) were never found, so fixed-joint reduction left stale frame names in those extensions.

Before: load a URDF that lumps a fixed joint and has one of those nested gazebo extensions referencing the child link — the reduced SDF keeps the child frame name.
After: the helpers take tinyxml2::XMLElement *, call sites pass (*blobIt)->FirstChildElement(), and the nested tags are rewritten to the parent link. Also grab _blob->GetDocument() before DeleteChild in the projector/plugin paths (TinyXML2 frees the node).

New integration URDFs + cases in INTEGRATION_fixed_joint_reduction cover the three leftover helpers. Bazel already globs fixed_joint_reduction*.urdf, so no BUILD change.

Backport Policy

Internal URDF-parser helpers only; no public API/ABI change intended. Same class of fix as #745 / #1089.

  • This is safe to backport to the following versions:
    • Jetty
    • Ionic
    • Harmonic
    • Fortress
  • This should not be backported
  • I am not sure
  • Other (fill in yourself)

Checklist

  • Signed all commits for DCO
  • Added a screen capture or video to the PR description that demonstrates the fix (as needed)
  • Added tests
  • Updated documentation (as needed)
  • Updated migration guide (as needed)
  • Consider updating Python bindings (if the library has them)
  • codecheck passed (See contributing)
  • All tests passed (See test coverage)
  • Updated Bazel files (if adding new files). Created an issue otherwise.
  • While waiting for a review on your PR, please help review another open pull request to support the maintainers
  • Was GenAI used to generate this PR? If so, make sure to add "Assisted-by" to your commits. (See this policy for more info.)

Assisted-by: Cursor (Composer)

I used Cursor to help draft the patch and tests; I reviewed the diff against the #745/#1089 pattern and TinyXML2’s DeleteChild behavior. I have not run the full sdformat C++ suite locally (no Linux build env here) — relying on CI for INTEGRATION_fixed_joint_reduction / codecheck.

Note to maintainers: Remember to use Squash-Merge and edit the commit message to match the pull request summary while retaining Signed-off-by and Assisted-by / Generated-by messages.

@trippyogi

Copy link
Copy Markdown
Author

Follow-up: use-after-free fix

Addressed the projector DeleteChildGetDocument UAF by capturing _blob->GetDocument() first. Applied the same pattern to the Plugin FrameReplace helper, which had the identical bug.

Ready for another look / ASan if convenient.

@trippyogi
trippyogi marked this pull request as ready for review August 3, 2026 16:03
@azeey

azeey commented Aug 3, 2026

Copy link
Copy Markdown
Collaborator

@trippyogi Thanks for the contribution. Could you fill out the PR template properly? Though we haven't incorporated it in our contribution guidelines yet, the Gazebo PMC has agreed to adopt these guidelines for AI-assisted contributions, so please review that as well.

Repeat the gazebosim#745/gazebosim#1089 FirstChildElement pattern for ContactSensor,
Gripper, and Projector so fixed-joint reduction updates nested SDF
extension frame references. Also capture the XMLDocument from the
parent blob before DeleteChild (TinyXML2 frees the node).

Adds integration URDFs/tests that fail without the FirstChildElement fix.

Fixes gazebosim#746

Assisted-by: Cursor (Composer)
Signed-off-by: Jeremy <peace@trippyogi.com>
@trippyogi
trippyogi force-pushed the fix/746-reducesdf-frame-replace-leftovers branch from 6abc64d to 4ace0b5 Compare August 3, 2026 16:29
@trippyogi

Copy link
Copy Markdown
Author

@azeey Thanks — done.

I refilled the Gazebo bug-fix PR template (summary, backport, checklist) and read the AI-assisted contribution guidelines plus the OSRA generative-tools policy. GenAI use is disclosed with Assisted-by: Cursor (Composer) on the commit and in the PR description.

I have not run the full test suite locally; CI will need to cover that.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

Status: Inbox

Development

Successfully merging this pull request may close these issues.

parser_urdf: ReduceSDFExtension*FrameReplace functions are broken in libsdformat10+

2 participants