[xslt] Replacing the dict used by the transform context

[Stefan Behnel <behnel_ml gkec informatik tu-darmstadt de>]

Hi,

I'm the maintainer of lxml, a pythonic Python binding to the libxml2/libxslt
libraries.

In lxml, we use a global per-thread dictionary for all parsers. Recently, we
had reports about crashes in the XSLT code when moving subtrees of results to
other trees. The reason for this is that libxslt creates a new sub-dictionary
for each transformation context that is inherited by the result document. This
means that the dict disappears when the document is freed. The cleanup
procedure for this specific document and all of its elements therefore depends
on the source of the original document, and it's difficult to deal with this
in generic cleanup code (as required by a wrapper like lxml).

We would like to use the same approach as for parsers here, i.e. replace the
dict after creating the context. However, in the xsltNewTransformContext
function in transform.c it says right before the creation of the sub-dictionary:

   /*
    * setup of the dictionnary must be done early as some of the
    * processing later like key handling may need it.
    */

Now, I checked if this is actually true and found that the dict is not
accessed anywhere in that function or in any of the functions it calls. So
this comment is at least misleading.

I do not consider it common usage to replace the dict, so I would not request
an explicit API for this. However, for the case we need, it is sufficient to
free the dict that was created and replace it be the 'right' one.

I would like to propose making it a feature that the dictionary is *not*
changed in the xsltNewTransformContext, but can be safely replaced in user
code right after returning the new context. The attached patch does this. It
does not change the behaviour of the current libxslt at all, it mainly moves
the dict creation to the end of the function and adds a comment that it is
supposed to stay there to prevent any updates before returning the context. I
ran "make tests" with it and the tests passed nicely.

Hoping for approval,
Stefan

--- libxslt/transform.c.ORIG	2006-08-06 22:20:37.000000000 +0200
+++ libxslt/transform.c	2006-08-06 22:44:52.000000000 +0200
@@ -348,17 +348,6 @@
     memset(cur, 0, sizeof(xsltTransformContext));
 
     /*
-     * setup of the dictionnary must be done early as some of the
-     * processing later like key handling may need it.
-     */
-    cur->dict = xmlDictCreateSub(style->dict);
-    cur->internalized = ((style->internalized) && (cur->dict != NULL));
-#ifdef WITH_XSLT_DEBUG
-    xsltGenericDebug(xsltGenericDebugContext,
-	     "Creating sub-dictionary from stylesheet for transformation\n");
-#endif
-
-    /*
      * initialize the template stack
      */
     cur->templTab = (xsltTemplatePtr *)
@@ -467,6 +456,17 @@
     cur->traceCode = (unsigned long*) &xsltDefaultTrace;
     cur->xinclude = xsltGetXIncludeDefault();
 
+    /*
+     * setup of the dictionnary goes last. users might replace it, so we
+     * must not change it before returning the context.
+     */
+    cur->dict = xmlDictCreateSub(style->dict);
+    cur->internalized = ((style->internalized) && (cur->dict != NULL));
+#ifdef WITH_XSLT_DEBUG
+    xsltGenericDebug(xsltGenericDebugContext,
+	     "Creating sub-dictionary from stylesheet for transformation\n");
+#endif
+
     return(cur);
 
 internal_err: