[PATCH] Re: xsl:use-attribute-sets in libxslt not conforming to the XSLT spec
Alexey Neyman <[email protected]> Tue, 12 Feb 2019 01:05:10 -0800
| Newsgroups | gmane.comp.gnome.lib.xslt |
|---|---|
| Message-ID | <[email protected]> |
This is a multi-part message in MIME format.
--------------B0BA0A0778B8409CE798B04D
Content-Type: text/plain; charset=utf-8; format=flowed
Content-Transfer-Encoding: 8bit
[CC'ing the Nick Wellnhofer, the author of the commit that introduced
this issue]
So, I found what the problem with that commit is.
The commit removed the `xsltAttributeInternal` function which
distinguished between attributes added as a part of an attribute set
(which were not allowed to overwrite the attributes from the LREs,
literal result elements) and attributes added using <xsl:attribute/>
(which overwrite the attributes from the LREs). This is the removed
block - which was explicitly handling this case:
if (fromAttributeSet) {
/*
* This tries to ensure that xsl:attribute(s) coming
* from an xsl:attribute-set won't override attribute of
* literal result elements or of explicit xsl:attribute(s).
* URGENT TODO: This might be buggy, since it will miss to
* overwrite two equal attributes both from attribute sets.
*/
attr = xmlHasNsProp(targetElem, name, nsName);
if (attr != NULL)
return;
}
The assumption stated in the comment in xsltAttrListTemplateProcess()
/*
* Apply attribute-sets.
* The creation of such attributes will not overwrite any existing
* attribute.
*/
no longer holds because xsltAttribute now always overwrites the existing
attributes.
I don't think this removed block needs to be restored; instead, I think,
the xsltAttrListTemplateProcess() should process the attribute sets
first and then, iterating over LRE attributes, override the values if
there is a duplicate attribute. Instead of constructing the attribute
list itself, it can use xmlSetNsProp() which will either create a new
attribute, or set the value on an existing one. xsltAttribute() already
uses xmlSetNsProp() for that purpose.
Patch attached; fixes the test case I sent before. I am not sure how to
interpret the output of `make check` - if it causes any regressions or
not. Would appreciate a review from the maintainers.
Best regards,
Alexey.
On 2/11/19 4:34 PM, Alexey Neyman wrote:
> Hi,
>
> I noticed since version 1.1.30 of libxslt, the attributes on the
> literal element are not merged correctly with the attributes specified
> on the element itself. The language in the spec [1] ("7.1.4 Named
> Attribute Sets") is as follows:
>
> > Thus, for a literal result element, attributes from attribute sets
> named in an xsl:use-attribute-sets attribute will be added first, in
> the order listed in the attribute; next, attributes specified on the
> literal result element will be added; finally, any attributes
> specified by xsl:attribute elements will be added. Since adding an
> attribute to an element replaces any existing attribute of that
> element with the same name, this means that attributes specified in
> attribute sets can be overridden by attributes specified on the
> literal result element itself.
>
> Below is a small test case that has two attribute sets ("as1", "as2";
> used in that order) + attributes specified on the element itself
> ("element") and attributes specified via <xsl:attribute/> in the
> content of the literal element. Per spec, the output of the test case
> should be:
>
> <bar a1="attr" a2="element" a3="as2" a4="as1"/>
>
> With 1.1.30 and newer, the output is:
>
> <bar a1="attr" a2="as2" a3="as2" a4="as1"/>
>
> Before 1.1.30, the output was:
>
> <bar a1="attr" a2="element" a3="as1" a4="as1"/>
>
> The commit [2] that changed this behavior apparently fixed the order
> of attributes merged from multiple attribute sets (see a3/a4
> attributes above) but in doing so, broke the handling of the
> attributes from the literal element (see a2 attribute above).
>
> This, for example, breaks the docbook stylesheet customizations for
> the titlepage (which auto-generate an XSLT stylesheet using both an
> attribute sets and attributes on a literal element).
>
> The test case:
>
> ---- a.xsl ----
> <?xml version="1.0"?>
> <xsl:stylesheet xmlns:xsl="http://www.w3.org/1999/XSL/Transform"
> version="1.0">
> <xsl:template match="*">
> <xsl:apply-templates/>
> </xsl:template>
>
> <xsl:attribute-set name="as1">
> <xsl:attribute name="a1">as1</xsl:attribute>
> <xsl:attribute name="a2">as1</xsl:attribute>
> <xsl:attribute name="a3">as1</xsl:attribute>
> <xsl:attribute name="a4">as1</xsl:attribute>
> </xsl:attribute-set>
>
> <xsl:attribute-set name="as2">
> <xsl:attribute name="a1">as2</xsl:attribute>
> <xsl:attribute name="a2">as2</xsl:attribute>
> <xsl:attribute name="a3">as2</xsl:attribute>
> </xsl:attribute-set>
>
> <xsl:template match="foo">
> <bar xsl:use-attribute-sets="as1 as2" a1="element" a2="element">
> <xsl:attribute name="a1">attr</xsl:attribute>
> </bar>
> </xsl:template>
> </xsl:stylesheet>
>
> ---- a.xml ----
> <?xml version="1.0"?>
> <foo/>
>
> [1] https://www.w3.org/TR/1999/REC-xslt-19991116
> [2]
> https://gitlab.gnome.org/GNOME/libxslt/commit/05f70130433478c1075ce8b6fdc4c4dadf51d33e
>
> Regards,
> Alexey.
>
--------------B0BA0A0778B8409CE798B04D
Content-Type: text/x-patch;
name="0001-LRE-attribute-must-have-precedence-over-attribute-se.patch"
Content-Transfer-Encoding: 7bit
Content-Disposition: attachment;
filename*0="0001-LRE-attribute-must-have-precedence-over-attribute-se.pa";
filename*1="tch"
From e72c507fac55a4a6d608b616231c9fd247dd18ac Mon Sep 17 00:00:00 2001
From: Alexey Neyman <[email protected]>
Date: Tue, 12 Feb 2019 00:56:50 -0800
Subject: [PATCH] LRE attribute must have precedence over attribute sets
Process attribute sets first. Then, use xmlSetNsProp to create
properties copied from the literal result elements (LREs) - which
overwrites the values from attribute sets.
Signed-off-by: Alexey Neyman <[email protected]>
---
libxslt/templates.c | 90 ++++++++++++++-------------------------------
1 file changed, 28 insertions(+), 62 deletions(-)
diff --git a/libxslt/templates.c b/libxslt/templates.c
index 42559210..12a93951 100644
--- a/libxslt/templates.c
+++ b/libxslt/templates.c
@@ -645,7 +645,7 @@ xmlAttrPtr
xsltAttrListTemplateProcess(xsltTransformContextPtr ctxt,
xmlNodePtr target, xmlAttrPtr attrs)
{
- xmlAttrPtr attr, copy, last;
+ xmlAttrPtr attr, copy;
xmlNodePtr oldInsert, text;
xmlNsPtr origNs = NULL, copyNs = NULL;
const xmlChar *value;
@@ -659,15 +659,29 @@ xsltAttrListTemplateProcess(xsltTransformContextPtr ctxt,
ctxt->insert = target;
/*
- * Instantiate LRE-attributes.
+ * Apply attribute-sets.
+ * The creation of such attributes will not overwrite any existing
+ * attribute.
*/
- if (target->properties) {
- last = target->properties;
- while (last->next != NULL)
- last = last->next;
- } else {
- last = NULL;
- }
+ attr = attrs;
+ do {
+#ifdef XSLT_REFACTORED
+ if ((attr->psvi == xsltXSLTAttrMarker) &&
+ xmlStrEqual(attr->name, (const xmlChar *)"use-attribute-sets"))
+ {
+ xsltApplyAttributeSet(ctxt, ctxt->node, (xmlNodePtr) attr, NULL);
+ }
+#else
+ if ((attr->ns != NULL) &&
+ xmlStrEqual(attr->name, (const xmlChar *)"use-attribute-sets") &&
+ xmlStrEqual(attr->ns->href, XSLT_NAMESPACE))
+ {
+ xsltApplyAttributeSet(ctxt, ctxt->node, (xmlNodePtr) attr, NULL);
+ }
+#endif
+ attr = attr->next;
+ } while (attr != NULL);
+
attr = attrs;
do {
/*
@@ -702,34 +716,6 @@ xsltAttrListTemplateProcess(xsltTransformContextPtr ctxt,
} else
value = xmlDictLookup(ctxt->dict, BAD_CAST "", 0);
- /*
- * Create a new attribute.
- */
- copy = xmlNewDocProp(target->doc, attr->name, NULL);
- if (copy == NULL) {
- if (attr->ns) {
- xsltTransformError(ctxt, NULL, attr->parent,
- "Internal error: Failed to create attribute '{%s}%s'.\n",
- attr->ns->href, attr->name);
- } else {
- xsltTransformError(ctxt, NULL, attr->parent,
- "Internal error: Failed to create attribute '%s'.\n",
- attr->name);
- }
- goto error;
- }
- /*
- * Attach it to the target element.
- */
- copy->parent = target;
- if (last == NULL) {
- target->properties = copy;
- last = copy;
- } else {
- last->next = copy;
- copy->prev = last;
- last = copy;
- }
/*
* Set the namespace. Avoid lookups of same namespaces.
*/
@@ -748,7 +734,11 @@ xsltAttrListTemplateProcess(xsltTransformContextPtr ctxt,
} else
copyNs = NULL;
}
- copy->ns = copyNs;
+
+ // xmlSetNsProp will take care of the duplicates
+ copy = xmlSetNsProp(ctxt->insert, copyNs, attr->name, NULL);
+ if (copy == NULL)
+ goto error;
/*
* Set the value.
@@ -803,30 +793,6 @@ next_attribute:
attr = attr->next;
} while (attr != NULL);
- /*
- * Apply attribute-sets.
- * The creation of such attributes will not overwrite any existing
- * attribute.
- */
- attr = attrs;
- do {
-#ifdef XSLT_REFACTORED
- if ((attr->psvi == xsltXSLTAttrMarker) &&
- xmlStrEqual(attr->name, (const xmlChar *)"use-attribute-sets"))
- {
- xsltApplyAttributeSet(ctxt, ctxt->node, (xmlNodePtr) attr, NULL);
- }
-#else
- if ((attr->ns != NULL) &&
- xmlStrEqual(attr->name, (const xmlChar *)"use-attribute-sets") &&
- xmlStrEqual(attr->ns->href, XSLT_NAMESPACE))
- {
- xsltApplyAttributeSet(ctxt, ctxt->node, (xmlNodePtr) attr, NULL);
- }
-#endif
- attr = attr->next;
- } while (attr != NULL);
-
ctxt->insert = oldInsert;
return(target->properties);
--
2.19.1
--------------B0BA0A0778B8409CE798B04D
Content-Type: text/plain; charset="us-ascii"
MIME-Version: 1.0
Content-Transfer-Encoding: 7bit
Content-Disposition: inline
_______________________________________________
xslt mailing list, project page http://xmlsoft.org/XSLT/
[email protected]
https://mail.gnome.org/mailman/listinfo/xslt
--------------B0BA0A0778B8409CE798B04D--