trailofbits / trailofbits/graphtage
XML formatter drops inserted attributes and child elements
Nobody has claimed this yet.
- Dominant language
- Python
- Stars
- 2.5k
- Forks
- 61
- Avg merge
- 6h 45m
- Merged PRs (30d)
- 45
Description
Adding an attribute to an XML or HTML element that previously had no attributes drops it from the output. The attribute is silently absent from the diff, with no insertion marker and no trace that anything changed.
Reproducer
$ printf '<r><d>x</d></r>' > b1.xml
$ printf '<r><d a="1">x</d></r>' > b2.xml
$ graphtage --no-status b1.xml b2.xml
<r><d>x</d>
</r>
Expected something like <r><d++ a="1"++>x</d></r>.
Only this case is affected. Adding an attribute to an element that already has one works, as do value changes and removals:
$ graphtage --no-status b2.xml b4.xml # a="1" -> a="1" b="2"
<r><d a="1"++ b="2"++>x</d>
$ graphtage --no-status b2.xml b3.xml # a="1" -> a="2"
<r><d a="1" -> "2">x</d>
$ graphtage --no-status b2.xml b1.xml # a="1" -> (none)
<r><d~~ a="1"~~>x</d>
Cause
The differ is correct; the formatter drops the edit. The Insert is computed:
>>> for e in gx.build_tree("b1.xml").get_all_edits(gx.build_tree("b2.xml")): print(e)
Insert(to_insert=KeyValuePairNode(key=StringNode('a'), value=StringNode('1')), ...)
XMLFormatter.print_XMLElement (graphtage/xml.py:396) guards on the truthiness of the attribute node:
if node.attrib:
self.print(printer, node.attrib)
node.attrib is the from element's DictNode, and an empty DictNode is falsy (len() == 0). So when the source element has no attributes, the attribute node is never handed to the formatter, and the insert edits attached to it are never applied. This is the failure mode described in CLAUDE.md: insert and remove edits are applied inside SequenceFormatter.print_SequenceNode, so any path that skips that call drops them.
Fix
Removing the guard is sufficient. I tested it by patching print_XMLElement in a throwaway process:
| case | before | after |
|---|---|---|
| add to bare element | <r><d>x</d> |
<r><d++ a="1"++>x</d> |
| element with no attributes at all | <r><d>x</d> |
<r><d>x</d> |
| change a value | <r><d a="1" -> "2">x</d> |
<r><d a="1" -> "2">x</d> |
XMLElementAttribFormatter writes the leading space per key/value pair, so an element with genuinely no attributes still renders as <d> with no stray space, and the other cases are unchanged.
The same guard drops inserted child elements
The condition on the next line, if node._children._children or ... (xml.py:398), has the same shape and the same defect, and this one loses an entire subtree:
$ printf '<r><d>x</d></r>' > c1.xml
$ printf '<r><d>x<e>y</e></d></r>' > c2.xml
$ graphtage --no-status c1.xml c2.xml
<r><d>x</d>
</r>
<e>y</e> is gone from the output entirely. When the element starts out empty it is the same story, and the element even renders self-closing:
$ printf '<r><d></d></r>' > c3.xml
$ printf '<r><d><e>y</e></d></r>' > c4.xml
$ graphtage --no-status c3.xml c4.xml
<r><d />
</r>
Both guards need to consider the to side as well as the from side, or hand the node to the formatter unconditionally and let print_SequenceNode decide what to emit.
A regression test should assert on the rendered diff, not on str(edit) — the edit is present either way, so an assertion over the edit list passes even while the output is wrong.
Contributor guide
No contributing guide indexed for this repository
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
Start in graphtage/xml.py at XMLFormatter.print_XMLElement around lines 396-398, then run the XML and HTML reproducers from the issue. Add regression coverage that checks the rendered diff for inserted attributes and child elements, rather than only the edit list. Done means both insertions appear in the output while unchanged and existing-attribute cases remain unchanged.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- python
- Domain
- tooling
- Issue type
- Bug
- Difficulty
- 2/5
- Estimated time
- 1-3 hours
- Activity status
- Active
- Clarity
- Clearly specified
- Newbie friendliness
- 88/100