[Opensync-commits] r6007 - plugins/ldap-sync/src

[<svn...@op...>]

Author: scriptor
Date: Thu Jan 14 21:52:51 2010
New Revision: 6007
URL: http://www.opensync.org/changeset/6007

Log:
Workaround for the way osync_xmlformat_sort() in:

libopensync/opensync/xmlformat/opensync_xmlformat.c 

applies xmlAddChild().  xmlAddChild() frees the node, sometimes.
Consequently a subsequent call to osync_xmlformat_is_sorted() segfaults, 
occasionally.  

Please note, this is NOT a bug fix. Not at all.  Because it does NOT 
prevent the xmlformat struct from getting corrupted by libxml2.  


277   cur = osync_xmlformat_get_first_field(xmlformat);
278   for(; cur != NULL; cur = osync_xmlfield_get_next(cur)) {
279     list[index] = cur;
280     index++;
281     xmlUnlinkNode(cur->node);
282   }

290   for(index = 0; index < xmlformat->child_count; index++) {
291     cur = (OSyncXMLField *)list[index];
292     xmlAddChild(xmlDocGetRootElement(xmlformat->doc), cur->node);
293 

cur is a pointer to a field in the original xmlformat rather than
a copy.  And cur->node gets free'd in some cases (not in all cases) 
by xmlAddChild().


Quoting from 

http://xmlsoft.org/html/libxml-tree.html#xmlAddChild

"Add a new node to @parent, at the end of the child (or property) list 
merging adjacent TEXT nodes (in which case @cur is freed).

If the new node is ATTRIBUTE, it is added into properties instead of children. 

If there is an attribute with equal name, it is first destroyed."


The proper solution would be making use of the return value of xmlAddChild(),
I suppose.  Or make use of xmlCopyNode(). Not tested, though.  And in 
case of xmlCopyNode() I do not know how well it plays together with a
subsequent xmlFreeNode(), when the node has already been freed'd by libxml2...

Modified:
   plugins/ldap-sync/src/ldap_format.c

Modified: plugins/ldap-sync/src/ldap_format.c
==============================================================================
--- plugins/ldap-sync/src/ldap_format.c	Thu Jan 14 21:52:38 2010	(r6006)
+++ plugins/ldap-sync/src/ldap_format.c	Thu Jan 14 21:52:51 2010	(r6007)
@@ -2444,13 +2444,56 @@
 		goto error;
 	}
 
-	if (osync_xmlformat_is_sorted(xmlformat)) {
-		osync_trace(TRACE_INTERNAL, "%s:%i: xmlformat just produced IS sorted.\n", __FILE__, __LINE__);
+
+
+	/**
+	 * The following call to osync_xmlformat_copy() is a workaround for 
+	 * the problem, that libxml2 may or may not free some cur->node
+	 * pointers:
+	 *
+	 *  Quoting from http://localhost/usr_share_doc/libxml2-devel-2.7.6/html/libxml-tree.html
+	 * 
+	 * Function: xmlAddChild
+	 *
+	 * xmlNodePtr xmlAddChild (xmlNodePtr parent, xmlNodePtr cur)
+	 *
+	 * Add a new node to @parent, at the end of the child (or property) list 
+	 * merging adjacent TEXT nodes (in which case @cur is freed).
+	 *
+	 * If the new node is ATTRIBUTE, it is added into properties instead of 
+	 * children. 
+	 *
+	 * If there is an attribute with equal name, it is first destroyed.
+ 	 *
+ 	 * See also osync_xmlformat_sort() in opensync_xmlformat.c
+ 	 *          around line 292.
+ 	 *
+ 	 *          Small wonder, that a subsequent call to osync_xmlformat_is_sorted()
+ 	 *          segfaults: cur->node may have been freed by libxml2. 
+ 	 *          Possibly, not necessarily.  Resulting in a double-free.
+ 	 *
+ 	 */
+	OSyncXMLFormat *xmlformat2 = NULL;
+	if (osync_xmlformat_copy(xmlformat, &xmlformat2, error)) {
+		if (xmlformat2) {
+			if (osync_xmlformat_is_sorted(xmlformat2)) {
+				osync_trace(TRACE_INTERNAL, "%s:%i: xmlformat just produced IS sorted.\n", __FILE__, __LINE__);
+			} else {
+				osync_trace(TRACE_INTERNAL, "%s:%i: xmlformat just produced is NOT sorted.\n", __FILE__, __LINE__);
+			}
+
+			osync_xmlformat_unref(xmlformat2);
+		} else {
+			osync_trace(TRACE_ERROR, "%s:%i: WARNING: xmlformat2 = NULL. osync_xmlformat_is_sorted() cannot be called, therefore. Ignoring.", __FILE__, __LINE__);
+		}
 	} else {
-		osync_trace(TRACE_INTERNAL, "%s:%i: xmlformat just produced is NOT sorted.\n", __FILE__, __LINE__);
+		if (!osync_error_is_set(error)) {
+			osync_error_set(error, OSYNC_ERROR_GENERIC, "%s:%i: ERROR: osync_xmlformat_copy() has failed.", __FILE__, __LINE__);
+		}
+	
+		goto error;
 	}
 
-
 	// Report the results to the calling function
 	*output = (char *) xmlformat;
 	*outpsize = osync_xmlformat_size(); // Returns the size of the OSyncXMLFormat struct.