fix(ivorysql_ora): prevent double free in APPENDCHILDXML with prolog comments

[zjncs]

Fixes #2134

Problem

APPENDCHILDXML aborts the backend when the input document has a comment or processing instruction before the root element:

SELECT appendchildxml(XMLType('<!-- c --><a><b/></a>'), '/a', XMLType('<c/>'));

free(): double free detected in tcache 2
LOG:  client backend (PID 17119) was terminated by signal 6: Aborted

Both the 3-argument form and the 4-argument namespace form (ivy_appendchildxml / ivy_appendchildxml2) are affected.

Root cause

After serializing the result, both functions ran:

ret = (xmltype *)ivy_xml_xmlnode2xmltype((xmlNodePtr)ws.doc);
xmlUnlinkNode((xmlNodePtr)ws.xpathctx->node->children);
xmlFreeNode((xmlNodePtr)ws.xpathctx->node->children);

ws.xpathctx->node is the document node, and with a prolog comment node->children is that comment. xmlUnlinkNode() unlinks the comment and advances doc->children to the root element, so the following xmlFreeNode() frees the root element while it is still linked in the document. cleanup_ws() later calls xmlFreeDoc(ws.doc), which walks the child list starting at the already-freed root element — double free.

Without a prolog node the same two lines unlinked the root element, freed nothing (xmlFreeNode(NULL)), and left xmlFreeDoc() with an empty document: the whole modified tree leaked on every call.

Fix

Delete the two lines in both functions. ret is already a serialized copy and cleanup_ws() frees ws.doc exactly once, so nothing else is needed. This fixes both the crash and the per-call document leak.

After the fix the statement above returns:

 <!-- c -->
 <a>
   <b/>
   <c/>
 </a>

Verification

Debug build (--enable-cassert --enable-debug --with-libxml --with-icu --with-uuid=e2fs), oracle compatible mode, server encoding UTF8, on master (63fb0bf):

• Before: the repro aborts the backend with free(): double free detected in tcache 2 (signal 6); the same happens for a prolog processing instruction and for the 4-argument namespace variant.
• After: all three forms return the modified document with the prolog node preserved.
• make -C contrib/ivorysql_ora oracle-installcheck: 31/31 tests in ORA_REGRESS behave identically before and after the change (ora_xml_functions green both times; the standalone-run ora_sysview environment diff is unchanged and unrelated).
• New regression cases in contrib/ivorysql_ora/sql/ora_xml_functions.sql cover the prolog comment, the prolog processing instruction, and the namespace variant; expected output updated.

Summary by CodeRabbit

• Bug Fixes

  • Fixed a backend crash when appending XML to documents containing comments or processing instructions before the root element.
  • Corrected XML append behavior for namespace-qualified documents and ensured appended content remains available in the resulting document.

• Tests

  • Added regression coverage for XML documents containing pre-root comments and processing instructions.

[Copilot]

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Repository: IvorySQL/IvorySQL/.coderabbit.yaml

Review profile: CHILL

Plan: Advanced

Run ID: 2a19b8c7-30b1-4d76-9010-e83ee7c00b59

📥 Commits

Reviewing files that changed from the base of the PR and between 63fb0bf and 4bcfba6.

📒 Files selected for processing (3)

• contrib/ivorysql_ora/expected/ora_xml_functions.out
• contrib/ivorysql_ora/sql/ora_xml_functions.sql
• contrib/ivorysql_ora/src/xml_functions/ora_xml_functions.c

💤 Files with no reviewable changes (1)

• contrib/ivorysql_ora/src/xml_functions/ora_xml_functions.c

Included review availability: Your plan provides up to 4 included reviews per hour; 3 remain after this review.

---

📝 Walkthrough

Walkthrough

The change removes redundant XML node cleanup from both APPENDCHILDXML implementations. It adds regression tests for documents with pre-root comments or processing instructions, including a namespace-qualified case.

Changes

APPENDCHILDXML double-free fix

Layer / File(s)	Summary
Remove redundant XML cleanup contrib/ivorysql_ora/src/xml_functions/ora_xml_functions.c	Both APPENDCHILDXML implementations no longer unlink and free document children after serialization.
Add regression coverage contrib/ivorysql_ora/sql/ora_xml_functions.sql, contrib/ivorysql_ora/expected/ora_xml_functions.out	Adds tests and expected results for comment-prefixed, processing-instruction-prefixed, and namespace-qualified documents.

Priority: ➖ Normal

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix · Severity of issue fixed: Medium

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly identifies the main change: preventing double-free failures in APPENDCHILDXML when prolog comments are present. It is concise and directly related to the pull request.
Linked Issues check	✅ Passed	The change satisfies #2134. It removes the redundant xmlUnlinkNode and xmlFreeNode calls from both ivy_appendchildxml() and ivy_appendchildxml2(). This prevents the documented double-free path…
Out of Scope Changes check	✅ Passed	All changes stay within #2134. The source change fixes cleanup in the two affected functions. The SQL and expected-output changes add regression coverage for the reported 3-argument and namespace case…
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 0…

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create a new PR

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[NotHimmel]

Thanks for contributing to IvorySQL!