Skip to content
This repository was archived by the owner on Aug 31, 2021. It is now read-only.

Commit a6b6f8b

Browse files
runrevmarkrunrevmark
committed
[[ Bug 12628 ]] Use appropriate libxml2 API call to move XML nodes from one document to another.
1 parent 264107f commit a6b6f8b

File tree

4 files changed

+65
-23
lines changed

4 files changed

+65
-23
lines changed

‎docs/notes/bugfix-12628.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
# Instability when using revAppendXML and revCopy/MoveRemoteXMLNode.

‎revxml/src/cxml.h‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -139,6 +139,8 @@ int ChildCount(char *childname, int maxdepth);
139139
void CopyElement(CXMLElement *telement,Bool truecopy = False);
140140
void AddElement(CXMLElement *telement);
141141
bool MoveElement(CXMLElement *tsrcelement, bool p_sibling, bool p_before);
142+
// MW-2014-06-12: [[ Bug 12628 ]] Uses appropriate libxml calls to move srclement from another document
143+
bool MoveRemoteElement(CXMLElement *tsrcelement, bool p_sibling, bool p_before);
142144
char *GetContent(Bool isbuffered = True);
143145
void SetContent(char *tdata);
144146
Bool GoChild(char *ename, bool inc_text = false);

‎revxml/src/revxml.cpp‎

Lines changed: 39 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -892,17 +892,25 @@ char *XML_ManipulateElement(int p_source_tree, char *p_source_node, int p_destin
892892
CXMLElement t_destination_element;
893893
if (!t_destination_doc -> GetElementByPath(&t_destination_element, p_destination_node))
894894
return istrdup(xmlerrors[XMLERR_BADELEMENT]);
895+
896+
// If we are copying, then we first duplicate the node before moving it
897+
if (p_copy)
898+
t_source_element . CopyElement(&t_source_element, true);
899+
900+
if (p_source_tree == p_destination_tree)
901+
{
902+
// Now move the source element to the new location, returning an error if fails
903+
if (!t_destination_element . MoveElement(&t_source_element, p_sibling, p_before))
904+
return istrdup(xmlerrors[XMLERR_BADCOPY]);
905+
}
906+
else
907+
{
908+
if (!t_destination_element . MoveRemoteElement(&t_source_element, p_sibling, p_before))
909+
return istrdup(xmlerrors[XMLERR_BADCOPY]);
910+
}
895911

896-
// If we are copying, then we first duplicate the node before moving it
897-
if (p_copy)
898-
t_source_element . CopyElement(&t_source_element, true);
899-
900-
// Now move the source element to the new location, returning an error if fails
901-
if (!t_destination_element . MoveElement(&t_source_element, p_sibling, p_before))
902-
return istrdup(xmlerrors[XMLERR_BADCOPY]);
903-
904-
// If succeeded, we return the new path of the moved element.
905-
return t_source_element . GetPath();
912+
// If succeeded, we return the new path of the moved element.
913+
return t_source_element . GetPath();
906914
}
907915

908916

@@ -1136,31 +1144,39 @@ void XML_AddXML(char *args[], int nargs, char **retstring,
11361144
*pass = False;
11371145
*error = False;
11381146
char *result = NULL;
1139-
if (nargs != 3){
1147+
if (nargs != 3)
1148+
{
11401149
*error = True;
11411150
result = istrdup(xmlerrors[XMLERR_BADARGUMENTS]);
11421151
}
1143-
else{
1152+
else
1153+
{
11441154
int docid = atoi(args[0]);
11451155
CXMLDocument *tdoc = doclist.find(docid);
11461156
CXMLElement telement;
11471157
if (!tdoc)
1148-
11491158
result = istrdup(xmlerrors[XMLERR_BADDOCID]);
1150-
else {
1159+
else
1160+
{
11511161
if (!tdoc->GetElementByPath(&telement,args[1]))
11521162
result = istrdup(xmlerrors[XMLERR_BADELEMENT]);
1153-
else {
1154-
CXMLElement newelement;
1155-
CXMLDocument tdoc;
1156-
if (!tdoc.Read(args[2],strlen(args[2]),False)){
1163+
else
1164+
{
1165+
CXMLDocument tnewdoc;
1166+
if (!tnewdoc.Read(args[2],strlen(args[2]),False))
1167+
{
11571168
result = (char *)malloc(1024);
1158-
sprintf(result,"%s\n%s",xmlerrors[XMLERR_BADXML],tdoc.GetError());
1169+
sprintf(result,"%s\n%s",xmlerrors[XMLERR_BADXML],tnewdoc.GetError());
11591170
}
1160-
else {
1161-
tdoc.GetRootElement(&newelement);
1162-
telement.AddElement(&newelement);
1163-
result = newelement.GetPath();
1171+
else
1172+
{
1173+
// MW-2014-06-12: [[ Bug 12628 ]] Use Remote operation to copy the root out of the new document.
1174+
CXMLElement newelement;
1175+
tnewdoc.GetRootElement(&newelement);
1176+
if (!telement.MoveRemoteElement(&newelement, false, false))
1177+
result = istrdup(xmlerrors[XMLERR_BADCOPY]);
1178+
else
1179+
result = newelement.GetPath();
11641180
}
11651181
}
11661182
}

‎revxml/src/xmlelement.cpp‎

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -460,6 +460,29 @@ bool CXMLElement::MoveElement(CXMLElement* p_element, bool p_sibling, bool p_bef
460460
return true;
461461
}
462462

463+
// MW-2014-06-12: [[ Bug 12628 ]] Use Remote operation to copy the root out of the new document.
464+
bool CXMLElement::MoveRemoteElement(CXMLElement *p_element, bool p_sibling, bool p_before)
465+
{
466+
// DOMWrapAdoptNode only allows you to move a node to under a parent, so first
467+
// we compute the parent node. In non-sibling mode, this is the parent node. In sibling
468+
// mode the parent of this is the parent node.
469+
xmlNodePtr t_parent;
470+
t_parent = element;
471+
if (p_sibling)
472+
t_parent = t_parent -> parent;
473+
474+
// If this has no parent then badness.
475+
if (t_parent == NULL)
476+
return false;
477+
478+
// Now make our doc tree adopt the source node.
479+
if (xmlDOMWrapAdoptNode(NULL, p_element -> element -> doc, p_element -> element, element -> doc, t_parent, 0) != 0)
480+
return false;
481+
482+
// Now that p_element is in this's xmlTree we can use MoveElement to place it appropriately.
483+
return MoveElement(p_element, p_sibling, p_before);
484+
}
485+
463486
/*GetContent - return value of element
464487
isbuffered - if true returns copy of element data
465488
- must be disposed by caller.

0 commit comments

Comments
 (0)