Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

escaping XPath expressions in attributes #29

Closed
cmarchand opened this issue Dec 19, 2016 · 19 comments · Fixed by #454
Closed

escaping XPath expressions in attributes #29

cmarchand opened this issue Dec 19, 2016 · 19 comments · Fixed by #454
Assignees
Milestone

Comments

@cmarchand
Copy link
Collaborator

cmarchand commented Dec 19, 2016

Such a scenario generates non-compilable XSL code :

<x:context>
  <file>
    <element path="/Q{http://www.w3.org/1999/XSL/Transform}stylesheet[1]/Q{http://www.w3.org/1999/XSL/Transform}template[1]">
    </element>
  </file>
</x:context>

Because of the curly brackets in the attribute value, when copied inside a <xsl:* /> element, XSL compiler tries to execute the XPath expression.

If in the context, I double the curly braces, then it works correctly. But is it xspec writer responsability to escape the curly braces, or it is xspec compiler responsability ?

@cirulls
Copy link
Member

cirulls commented Dec 23, 2016

Thanks for raising this question, I'll have a look at what is happening under the hood and let you know. Please allow me 7-10 days to get back to you as it is Christmas time.

@cirulls cirulls self-assigned this Dec 23, 2016
@cirulls
Copy link
Member

cirulls commented Dec 27, 2016

@cmarchand : can you give me the full XSpec test and XSLT you're testing so that I can reproduce the issue and see at which point it is causing problems?

@cmarchand
Copy link
Collaborator Author

xspec-attr-escape-bug.zip

Attached. The XSL is not important, all the problem is in the Xspec, when compiling the generated XSL.

@cirulls
Copy link
Member

cirulls commented Jan 2, 2017

I had a look at your zip file. First of all, I think there is an extra closing curly brace in the XSpec test as I can see this:

<simple attr="Q{http://www.jenitennison.com/xslt/xspec}scenario}"/>

I think you wanted to write this (note that there is no closing curly brace after scenario):

<simple attr="{http://www.jenitennison.com/xslt/xspec}scenario"/>

Also, I'm not sure what the Q stands for in here, am I missing something?

When running the XSpec test with this version of the simple element, a compilation error is generated:

Syntax error at char 6 in {Q{http://} in expression in simple/@attr on line 37 column 79 of 29.xsl:
  XPST0003: Unexpected token ":" beyond end of expression
Syntax error at char 6 in {Q{http://} in expression in simple/@attr on line 47 column 85 of 29.xsl:
  XPST0003: Unexpected token ":" beyond end of expression
Errors were reported during stylesheet compilation

I believe this occurs because the curly braces identify attribute value templates. Curly braces inside attribute values allow to evaluate the result of an expression. Therefore what is inside the curly braces gets evaluated into an XPath expression, hence the error. If you don't want to evaluate it, you need to use the double curly braces as you correctly suggest. I believe this is the expected behaviour in XPath/XSLT and it is not linked to the XSpec implementation.

There are other ways to get the same result and this may help you in case you need to evaluate the value of a variable inside an attribute:

  1. You can force the value of the attribute to be a string by surrounding it with single quotes as follows:
    <x:context>
        <simple attr="{'http://www.jenitennison.com/xslt/xspec'}scenario"/>
    </x:context>
    In this case the URL is a string and gets evaluated as a string. This XSpec runs successfully.
  2. You can use <xsl:variable> to provide the first part of the URL and then evaluate the value of the variable using curly braces:
    <x:description xmlns:x="http://www.jenitennison.com/xslt/xspec" stylesheet="29.xsl" xslt-version="3.0">
        <x:variable name="url" select="'http://www.jenitennison.com/xslt/xspec/'"/>
        <x:scenario label="To show the escape of curly braces problem">
        <x:context>
            <simple attr="{$url}scenario"/>
        </x:context>
        <x:expect label="Not empty" test="not(empty(.))"/>
        </x:scenario>
    </x:description>
    You can have multiple variables for different URLs and you can compose URLs from previously declared variables using the variable name (e.g. $url) inside the select attribute.

I hope this addresses you question or helps to clarify it.

@AirQuick
Copy link
Member

AirQuick commented Jan 2, 2017

Q is a new face who represents the qualified names: http://www.w3.org/TR/xpath-30/#doc-xpath30-EQName
Maybe XSpec has not taken it into account yet.

@cmarchand
Copy link
Collaborator Author

You are right, the last closing curly brace shouldn't be there.

If you escape the curly braces as XSLT expect - doubling them - , then there is no more compilation error, and the value is what I expect :

    <x:context>
        <simple attr="{{http://www.jenitennison.com/xslt/xspec}}scenario"/>
    </x:context>

@cirulls
Copy link
Member

cirulls commented Jan 3, 2017

Thanks for letting me know about Q, I was aware of QName but not of Q.

Is the solution with double curly braces fine with you? Can I close this issue?

@cmarchand
Copy link
Collaborator Author

All the solutions provided in this thread are fine.

The question is: is it a xspec writer's responsability to escape de curly braces, or should the xspec compiler do it ?

Whatever the answer, it have to be written in the documentation.

For me, it's fine to say the xspec writer knows enough XSL stuff to be able to escape the curly braces in x:context and x:expect

@cirulls
Copy link
Member

cirulls commented Jan 7, 2017

I added it to the documentation at the end of the section Matching Scenarios.

Are you happy with this change in the documentation?

@cirulls cirulls added this to the v0.5.0 milestone Jan 7, 2017
@cmarchand
Copy link
Collaborator Author

cmarchand commented Jan 7, 2017 via email

@AirQuick
Copy link
Member

AirQuick commented Jan 8, 2017

context and assert seem to have been introduced rather quietly in haste.
Unfortunately they're removed from this repository at the moment due to regressions, lack of test and scarce documentation. (#38)
I guess it would not be recommended to rely on them now.

@cirulls
Copy link
Member

cirulls commented Jan 8, 2017

Thanks @cmarchand and @AirQuick for your input.

@cmarchand : I edited your last comment and removed my reply at the end so that it is easier to read. I also removed the notation @ followed by attribute name as this is automatically converted into a GitHub handle which notifies GitHub users. Please do not use this common XML notation as GitHub users have already complained in the past.

I suggest to close this issue as I updated the documentation. If you want to discuss further the removal of attributes assert and context feel free to open another issue.

@cirulls cirulls closed this as completed Jan 8, 2017
@AirQuick
Copy link
Member

While I was looking into other issues, I found this issue to be just one of similar bugs.
Documentation added to Wiki should be considered as a workaround.

@AirQuick
Copy link
Member

Fix submitted: #454

@cmarchand
Please test it if you have a chance.

@AirQuick
Copy link
Member

Updated Wiki to reflect the fact.

@cmarchand
Copy link
Collaborator Author

cmarchand commented Nov 21, 2018

Wiki change describes precisely the problem, and solution is clear. Perfect.

@AirQuick
Copy link
Member

@cmarchand
Summing up just in case. The previous update to Wiki stated that you need to escape curly braces. But in fact, it's a bug and you don't need to escape curly braces. I fixed it in #454. So I removed the statement from Wiki.

@cmarchand
Copy link
Collaborator Author

Ok, I didn't look at the PR.
I prefer this compared to escape.
Thanks a lot @AirQuick

@AirQuick
Copy link
Member

Sorry!
Looking into the history further, I found that the current behavior was actually introduced as a feature, although it was not discussed and not documented and not tested at all and many seemingly related bugs are scattered. (See expath#9 and a thread)

So you need to escape curly braces.
I updated Wiki to make it clear. Also I'll revise #454.

@AirQuick AirQuick removed the bug label Dec 21, 2018
AirQuick added a commit that referenced this issue Apr 28, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment