Skip to content

Xpp3Dom.removeChild(int): fix childMap corruption with duplicate names - #395

Open
elharo wants to merge 3 commits into
masterfrom
fix/xpp3dom-removechild
Open

Xpp3Dom.removeChild(int): fix childMap corruption with duplicate names#395
elharo wants to merge 3 commits into
masterfrom
fix/xpp3dom-removechild

Conversation

@elharo

@elharo elharo commented Jul 1, 2026

Copy link
Copy Markdown
Contributor

Xpp3Dom.removeChild(int) (line 262) calls childMap.values().remove(child) to remove the child from the internal name-to-child map. This is incorrect when multiple children share the same name:

  • childMap stores only the last child added with a given name (entries are overwritten by addChild)
  • values().remove(child) removes the first matching value — which is the only value for that name in the map
  • When removing a child that was overwritten in the map (i.e., a later child with the same name was added), the map entry is left pointing to the wrong child
  • When removing the last child with that name, getChild(name) correctly returns null but only by coincidence — the map entry was simply removed

Fix: After removing the child from childList, check if childMap points to that specific child (by reference). If so, scan childList for the last remaining child with the same name and update the map, or remove the entry if none remain.

Fixes #394

@elharo
elharo requested a review from slachiewicz July 1, 2026 14:12
@slachiewicz slachiewicz added the bug Something isn't working label Jul 2, 2026
@slachiewicz
slachiewicz removed their request for review July 2, 2026 06:05
@elharo
elharo requested a review from Copilot July 24, 2026 10:53

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

This PR fixes Xpp3Dom.removeChild(int) so the internal childMap stays consistent when multiple children share the same name, addressing the corruption described in issue #394.

Changes:

  • Update Xpp3Dom.removeChild(int) to only adjust childMap when it currently points to the removed child, then re-point it to the last remaining child with that name (or remove the entry).
  • Add a regression test covering removal-by-index with duplicate child names.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File Description
src/main/java/org/apache/maven/shared/utils/xml/Xpp3Dom.java Fixes removeChild(int) to correctly maintain childMap when duplicate-named children exist.
src/test/java/org/apache/maven/shared/utils/xml/pull/Xpp3DomTest.java Adds a regression test ensuring removeChild(int) updates childMap correctly with duplicate child names.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread src/test/java/org/apache/maven/shared/utils/xml/pull/Xpp3DomTest.java Outdated
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Xpp3Dom.removeChild(int) corrupts childMap on duplicate names

3 participants