tomcat-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Christopher Schultz <ch...@christopherschultz.net>
Subject Re: On escaping of EL in attributes (BZ 57136)
Date Fri, 06 Nov 2015 12:24:32 GMT
Mark,

On 11/6/15 7:12 AM, Mark Thomas wrote:
> On 06/11/2015 11:47, Christopher Schultz wrote:
>> Mark,
>>
>> On 11/5/15 4:34 AM, Mark Thomas wrote:
>>> On 05/11/2015 08:48, Mark Thomas wrote:
>>>> On 05/11/2015 05:05, Konstantin Kolinko wrote:
>>>>> Hi!
>>>>>
>>>>> I happened to stumble on the following entry in changelog for 6.0.19:
>>>>>
>>>>>       <fix>
>>>>>         Fix various edge-cases when parsing EL, particularly inside attribute
>>>>>         values. Note that the Expert Group has confirmed that JSP.1.6
takes
>>>>>         precedence over JSP.1.3.10. Therefore EL in attributes must be
escaped
>>>>>         twice. (markt)
>>>>>       </fix>
>>>>
>>>> Wow. I have absolutely no memory of that at all.
>>>>
>>>> Let me see if I can dig up the discussion that provided that confirmation.
>>>
>>> OK, found it. Having a precise date range to work with made it a lot
>>> easier. Apologies in advance as I have the feeling that this is e-mail
>>> is going to be on the long side.
>>>
>>> Back in 2009, I, acting on behalf of the Tomcat community, raised this
>>> via a challenge to the JSP 2.1 TCK using the following examples:
>>>
>>> <test:echo text="${"hello world"}" />      <-- The spec requires this
>>> <test:echo text="${\"hello world\"}" />    <-- The TCK expects this
>>>
>>> To put this in the current context, the fix for BZ 57136 implements the
>>> first form.
>>>
>>> Our TCK contact discussed it with the JSP lead and the conclusion was
>>> that the second form was the correct one. The reason given was that the
>>> second form is valid XML whereas the first form is not.
>>>
>>> I queried this on the grounds that the grammar is explicit that the
>>> second form is correct and that the spec also states that the grammar
>>> takes precedence.
>>>
>>> The response was that a request would be made to clarify the spec.
>>>
>>> No such clarification was made in JSP 2.2 or JSP 2.3.
>>>
>>> Which brings us to where we are today. The spec says one thing, I assume
>>> the TCK tests for something else (I don't have access to the later JSP
>>> TCK versions), we have a private clarification from 7 years ago that the
>>> spec is wrong and the two versions of the spec since then have not
>>> included any related correction.
>>>
>>> In the past we have used the following order of precedence when the
>>> specs have been unclear:
>>> - what the EG intended based on their discussions
>>> - what the TCK tests for
>>> - spec language
>>>
>>> However, this order has only been used where we required clarification
>>> rather than when there were inconsistencies. Also, more recently, I have
>>> seen the view expressed with the EGs that it doesn't matter what the EG
>>> discussed, the specification language always takes priority even if the
>>> language does not reflect what the EG intended.
>>>
>>> To summarise:
>>>
>>> In favour of form 1:
>>> - it is consistent with the spec
>>> - EGs have recently expressed the spec takes precedence
>>> - There have been two releases of the JSP spec since the issue was
>>>   raised and the spec has not been updated
>>>
>>> In favour of form 2:
>>> - it is well-formed XML
>>> - it is what the TCK tested (tests?) for
>>> - the spec lead expressed the view this was the intended behaviour
>>> - Up until the BZ 57136 fix, Tomcat did it this way
>>
>> Neither of these are well-formed XML due to the presence of the embedded
>> quote characters in the attribute value. Or, are you talking about
>> attribute values *after* XML-un-escaping occurs (in which case they
>> would both be valid XML)? XML does not recognize the common \ character
>> as an escape.
>>
>> (This doesn't get us any closer to a decision, but it eliminates one of
>> the slight advantages to form 2.)
> 
> Fair point. I was just repeating a response I got back from Oracle. I
> suspect the test case was slightly different.

I'm surprised Oracle claimed that was valid XML. :/

>>> At this point, I don't see a clear argument one way or the other.
>>>
>>> I've looked through the open JSP spec issues:
>>> https://java.net/jira/browse/JSP_SPEC_PUBLIC
>>>
>>> and I don't see anything for this. I do see a lot of very old issues
>>> that don't appear to have been looked at for some time.
>>>
>>> Given the lack of clarity of the which behaviour is correct, I think we
>>> have little choice but to make this optional and that we should get this
>>> done before the next 8.0.x release. I intend to start working on that in
>>> trunk today.
>>
>> Well, the TCK behavior simply must be implemented or we won't pass it.
>> Are we actually under any obligation to pass the TCK?
> 
> No, because Oracle and the ASF have yet to agree a new license agreement
> for the TCK,

I just wasn't sure about whether we have to pass it in order to even
claim support for the various specs.

> If we did have the TCK we could challenge it again (on the grounds the
> spec was never updated so surely that must mean the spec is right and
> the TCK is wrong)
> 
>> On the other hand, nobody ready the TCK... only the spec.
> 
> Indeed.

Obviously, I meant "nobody READS", but it looks like you understood.

>> So most users will expect form 2. 
> 
> If they read the spec carefully enough (and to be fair it took me
> several days of reading and re-reading the relevant bits to get to the
> point I was happy that I understood what it meant) they should expect
> form 1.

I fat-fingered that. I'm typing without a left index finger this week,
and it's really interfering with my typing mojo.

>> So I guess we need to have a mode-switcher? It's
>> either that, or fail the TCK and/or anger some significant subset of users.
> 
> Things are sufficiently messy between the spec, the TCK, 'clarification'
> from Oracle and Tomcat's previous behaviour that I agree both forms need
> to be supported. The new quoteAttributeEL init param for the JSP servlet
> does exactly that.

Perfect. I wasn't able to follow the various acrobatics that you
performed with the switches... it looks like you actually removed
options that would have supported this. What have I missed?

-chris

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


Mime
View raw message