[xslt] [PATCH] Re: xsl:use-attribute-sets in libxslt not conforming to the XSLT spec



[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.

Attachment: 0001-LRE-attribute-must-have-precedence-over-attribute-se.patch
Description: Text Data



[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]