Re: [xslt] [XSLT] [PATCH] unglobalize xsltMaxDepth variable - take 2

[Daniel Veillard <veillard redhat com>]

On Sun, Mar 20, 2011 at 10:25:29PM -0400, Jérôme Carretero wrote:
> Hi,
> 
> A few weeks (months?) ago I submitted a patch to unglobalize the maximum template recursion parameter in libxslt.

  hum seems I missed it, probably during Dec-Feb while I didn't
got all my mail.

> This small patch has been cleaned-up, and a new version is attached.

  thanks,

> Comments are welcome.
[...]
> diff --git a/libxslt/libxslt.syms b/libxslt/libxslt.syms
> index 45fa74b..63cacf0 100644
> --- a/libxslt/libxslt.syms
> +++ b/libxslt/libxslt.syms
> @@ -306,7 +306,6 @@ LIBXML2_1.0.24 {
>  # xslt
>    xsltLibxmlVersion; # variable
>    xsltLibxsltVersion; # variable
> -  xsltMaxDepth; # variable
>  
>  # xsltInternals
>    xsltParseStylesheetImportedDoc;
> diff --git a/libxslt/transform.c b/libxslt/transform.c
> index 948d7d0..a7eb035 100644
> --- a/libxslt/transform.c
> +++ b/libxslt/transform.c
> @@ -63,8 +63,6 @@ static int xsltGetHTMLIDs(const xmlChar *version, const xmlChar **publicID,
>  			  const xmlChar **systemID);
>  #endif
>  
> -int xsltMaxDepth = 3000;
> -

  We can't do that, it breaks the library ABI. The symbol has to stay
and its default behaviour must be preserved.

>   * Useful macros
>   */
> @@ -504,6 +502,7 @@ xsltNewTransformContext(xsltStylesheetPtr style, xmlDocPtr doc) {
>      cur->templNr = 0;
>      cur->templMax = 5;
>      cur->templ = NULL;
> +    cur->maxTemplateDepth = 10000;

  initialize from xsltMaxDepth instead

>      /*
>       * initialize the variables stack
> @@ -519,6 +518,7 @@ xsltNewTransformContext(xsltStylesheetPtr style, xmlDocPtr doc) {
>      cur->varsMax = 10;
>      cur->vars = NULL;
>      cur->varsBase = 0;
> +    cur->maxTemplateVars = 50000;
>  
>      /*
>       * the profiling stack is not initialized by default
> @@ -2983,20 +2983,31 @@ xsltApplyXSLTTemplate(xsltTransformContextPtr ctxt,
>      * Check for infinite recursion: stop if the maximum of nested templates
>      * is excceeded. Adjust xsltMaxDepth if you need more.
>      */
> -    if (((ctxt->templNr >= xsltMaxDepth) ||
> -        (ctxt->varsNr >= 5 * xsltMaxDepth)))
> +    if (ctxt->templNr >= ctxt->maxTemplateDepth)
>      {
>          xsltTransformError(ctxt, NULL, list,
>  	    "xsltApplyXSLTTemplate: A potential infinite template recursion "
>  	    "was detected.\n"
> -	    "You can adjust xsltMaxDepth (--maxdepth) in order to "
> -	    "raise the maximum number of nested template calls and "
> -	    "variables/params (currently set to %d).\n",
> -	    xsltMaxDepth);
> +	    "You can adjust maxTemplateDepth (--maxdepth) in order to "
> +	    "raise the maximum number of nested template calls "
> +	    " (currently set to %d).\n",
> +	    ctxt->maxTemplateDepth);

  the error messages will have to be preserved I think.

>          return;
>      }
>  
> +    if (ctxt->varsNr >= ctxt->maxTemplateVars)
> +	{
> +        xsltTransformError(ctxt, NULL, list,
> +	    "xsltApplyXSLTTemplate: A potential infinite template recursion "
> +	    "was detected.\n"
> +	    "You can adjust maxTemplateVars (--maxvars) in order to "
> +	    "raise the maximum number of variables/params (currently set to %d).\n",
> +	    ctxt->maxTemplateVars);
> +        xsltDebug(ctxt, contextNode, list, NULL);
> +        return;
> +	}
> +
>      oldUserFragmentTop = ctxt->tmpRVT;
>      ctxt->tmpRVT = NULL;
>      oldLocalFragmentTop = ctxt->localRVT;
> diff --git a/libxslt/xslt.h b/libxslt/xslt.h
> index 849b03c..fa8bb32 100644
> --- a/libxslt/xslt.h
> +++ b/libxslt/xslt.h
> @@ -55,13 +55,6 @@ extern "C" {
>   XML_PARSE_NOENT | XML_PARSE_DTDLOAD | XML_PARSE_DTDATTR | XML_PARSE_NOCDATA
>  
>  /**
> - * xsltMaxDepth:
> - *
> - * This value is used to detect templates loops.
> - */
> -XSLTPUBVAR int xsltMaxDepth;
> -
> -/**
>   * xsltEngineVersion:

  Not possible

>   * The version string for libxslt.
> diff --git a/libxslt/xsltInternals.h b/libxslt/xsltInternals.h
> index 764fe8c..afb2a2c 100644
> --- a/libxslt/xsltInternals.h
> +++ b/libxslt/xsltInternals.h
> @@ -1780,6 +1780,8 @@ struct _xsltTransformContext {
>      xmlDocPtr localRVTBase;
>      int keyInitLevel;   /* Needed to catch recursive keys issues */
>      int funcLevel;      /* Needed to catch recursive functions issues */
> +    int maxTemplateDepth;
> +    int maxTemplateVars;
>  };

  still a good idea to make this tunable dynamically

>  /**
> diff --git a/xsltproc/xsltproc.c b/xsltproc/xsltproc.c
> index e978a63..df582fe 100644
> --- a/xsltproc/xsltproc.c
> +++ b/xsltproc/xsltproc.c
> @@ -103,6 +103,8 @@ static int nbpaths = 0;
>  static char *output = NULL;
>  static int errorno = 0;
>  static const char *writesubtree = NULL;
> +static int xsltMaxDepth = 3000;
> +static int xsltMaxVars = 15000;
>  
>  /*
>   * Entity loading control and customization.
> @@ -466,6 +468,9 @@ xsltProcess(xmlDocPtr doc, xsltStylesheetPtr cur, const char *filename) {
>  	if (xinclude)
>  	    ctxt->xinclude = 1;
>  #endif
> +	ctxt->maxTemplateDepth = xsltMaxDepth;
> +	ctxt->maxTemplateVars = xsltMaxVars;
> +
>  	if (profile) {
>  	    ret = xsltRunStylesheetUser(cur, doc, params, output,
>  		                        NULL, NULL, stderr, ctxt);
> @@ -502,6 +507,7 @@ static void usage(const char *name) {
>      printf("\t--nodtdattr do not default attributes from the DTD\n");
>      printf("\t--noout: do not dump the result\n");
>      printf("\t--maxdepth val : increase the maximum depth\n");
> +    printf("\t--maxvars val : increase the maximum variables\n");

  Okay

>      printf("\t--maxparserdepth val : increase the maximum parser depth\n");
>  #ifdef LIBXML_HTML_ENABLED
>      printf("\t--html: the input document is(are) an HTML file(s)\n");
> @@ -721,6 +727,15 @@ main(int argc, char **argv)
>                  if (value > 0)
>                      xsltMaxDepth = value;
>              }
> +        } else if ((!strcmp(argv[i], "-maxvars")) ||
> +                   (!strcmp(argv[i], "--maxvars"))) {
> +            int value;
> +
> +            i++;
> +            if (sscanf(argv[i], "%d", &value) == 1) {
> +                if (value > 0)
> +                    xsltMaxVars = value;
> +            }
>          } else if ((!strcmp(argv[i], "-maxparserdepth")) ||
>                     (!strcmp(argv[i], "--maxparserdepth"))) {
>              int value;

  Weird, I would have expected the xsltproc part to remove a chunk
where --maxdepth uses xsltMaxDepth, but it's still there. Seems you
didn't compile this or didn't compiled it against the local code,

Daniel

-- 
Daniel Veillard      | libxml Gnome XML XSLT toolkit  http://xmlsoft.org/
daniel veillard com  | Rpmfind RPM search engine http://rpmfind.net/
http://veillard.com/ | virtualization library  http://libvirt.org/