Bug 49297 - Whitespace absence is allowed before attribute name in a custom tag
Whitespace absence is allowed before attribute name in a custom tag
Status: RESOLVED FIXED
Product: Tomcat 7
Classification: Unclassified
Component: Jasper
trunk
PC Windows XP
: P2 minor (vote)
: ---
Assigned To: Tomcat Developers Mailing List
:
Depends on:
Blocks:
  Show dependency tree
 
Reported: 2010-05-15 15:58 UTC by Konstantin Kolinko
Modified: 2011-03-19 06:29 UTC (History)
0 users



Attachments
2010-05-15_tc7_attribute_name_whitespace-test.patch (2.40 KB, application/octet-stream)
2010-05-15 15:58 UTC, Konstantin Kolinko
Details
Fix the spec broken code in related code of this bug. (9.67 KB, patch)
2010-11-09 22:52 UTC, Shawn Jiang
Details | Diff
Fix the spec broken code in related code of this bug. (9.66 KB, patch)
2010-11-09 22:57 UTC, Shawn Jiang
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Konstantin Kolinko 2010-05-15 15:58:53 UTC
Created attachment 25441 [details]
2010-05-15_tc7_attribute_name_whitespace-test.patch

The /test/webapp-3.0/bug45nnn/bug45015b.jsp page of trunk contains the following text:
<tags:echo echo="hello "wo"rld" />

Running the test (testBug45015b of org.apache.jasper.compiler.TestGenerator) produces the following message:

org.apache.jasper.JasperException: /bug45nnn/bug45015b.jsp(21,30) equal symbol expected


It means, that the parser interprets "wo" as an attribute name, ignoring the fact that there is no whitespace before it.


If I write
<tags:echo echo="hello "wo="rld" />
the test complains that attribute "wo" is invalid for the tag,

and finally if I write
<tags:echo echo="hello "echo="rld" />
the test succeeds, while I would expect it to fail.

I am attaching a patch that adds a testcase based on the last example, though a better testcase is needed that does not use the same attribute name twice.

I think it is a minor issue, because this issue does not affect valid JSP pages, though allows to run some invalid ones.


It is reproducible in the current 6.0 and 5.5 as well (thus 6.0.26, 5.5.29):
if I add the following to /webapps/examples/jsp/simpletag/foo.jsp of TC6 (or the respective page in TC5.5):
<ul>
<eg:foo att1="1"att2="2"att3="3">
<li><%= member %></li>
</eg:foo>
</ul>

and call http://localhost:8080/examples/jsp/simpletag/foo.jsp
the page runs successfully and displays "1, 2, 3" list items among its output.

Expected result: show some compilation error, e.g. ">, /> or whitespace was expected".
Comment 1 Mark Thomas 2010-05-15 18:39:34 UTC
(In reply to comment #0)
> It means, that the parser interprets "wo" as an attribute name, ignoring the
> fact that there is no whitespace before it.

Which spec requires there to be white-space before an attribute name? I can't see why it would be necessary.

> If I write
> <tags:echo echo="hello "wo="rld" />
> the test complains that attribute "wo" is invalid for the tag,

I don't see this issue here. That is what I would expect to happen.


> and finally if I write
> <tags:echo echo="hello "echo="rld" />
> the test succeeds, while I would expect it to fail.

That is a little more odd. I'm not sure what I would expect in that case. I'd need to do some spec research on that one.
Comment 2 Konstantin Kolinko 2010-05-15 19:01:53 UTC
(In reply to comment #1)
> (In reply to comment #0)
> > It means, that the parser interprets "wo" as an attribute name, ignoring the
> > fact that there is no whitespace before it.
> 
> Which spec requires there to be white-space before an attribute name? I can't
> see why it would be necessary.
> 

From jsp-2_1-fr-spec.pdf:

JSP.1.3.2 Element Syntax
"Action elements follow the syntax of an XML element."

JSP.1.3.10.1 EBNF Grammar for JSP Syntax

AllBody ::= ...
    ( ‘<‘ CustomAction CustomActionBody )

CustomAction ::= TagPrefix ’:’ CustomActionName

CustomActionBody ::= ( Attributes CustomActionEnd )

Attributes ::= ( S Attribute )* S?
[ vc: UniqueAttSpec ]

The 'S' before Attribute means that a space is required, and UniqueAttSpec says that the same attribute name cannot appear more than once.
Comment 3 Mark Thomas 2010-05-15 19:21:40 UTC
(In reply to comment #2)
> The 'S' before Attribute means that a space is required

Thanks for the reference. It still seems pointless, but we should fix it. Not something I think we need to worry about for 7.0.0.

>UniqueAttSpec says that the same attribute name cannot appear more than once.

Now that I can see the point of. More important, but still not a show stopper for 7.0.0 in my view.
Comment 4 Mark Thomas 2010-10-19 09:21:56 UTC
Fixed in 7.0.x and will be included in 7.0.5 onwards.
Comment 5 Shawn Jiang 2010-11-09 22:52:42 UTC
Created attachment 26275 [details]
Fix the spec broken code in related code of this bug.

Hello, the fix of this bug recently result in some JSP TCK failures of geronimo. According to jsp spec :

"A translation error will occur if the page directive defines duplicate attribute/values within a given   translation unit, unless the values for the duplicate attributes are identical for all occurrences. The   import and pageEncoding attributes are exempt from this rule and can appear multiple times."

Adding a patch with testcases to fix this.
Comment 6 Shawn Jiang 2010-11-09 22:53:37 UTC
Reopen it because we have regressions here.
Comment 7 Shawn Jiang 2010-11-09 22:57:39 UTC
Created attachment 26276 [details]
Fix the spec broken code in related code of this bug.

ops, update the patch to include a missed block.
Comment 8 Mark Thomas 2010-11-12 08:45:02 UTC
The quote above along with some comments in the patch suggests an ambiguity in the spec. The JSP 2.2 language is quite clear:
<quote>
There shall be only one occurrence of any attribute/value pair defined by this directive in a given translation unit, unless the values for the duplicate attributes are identical for all occurrences. The import and pageEncoding
attributes are exempt from this rule and can appear multiple times. Multiple uses of the import attribute are cumulative (with ordered set union semantics). The pageEncoding attribute can occur at most once per file (or a translation error will result), and applies only to the file in which it appears.
</quote>
Comment 9 Mark Thomas 2010-11-12 11:53:09 UTC
Thanks for the patch. I didn't use it exactly but I did base the final fix on your patch.

The fix is in 7.0.x and will be included in 7.0.5 onwards.
Comment 10 Shawn Jiang 2010-11-14 02:18:15 UTC
Thank Mark, The fix works like a charm with pageDirective, but we should include the same logic for tag Directive according to the spec.  

Per JSP.8.5.1 The tag Directive

"The tag directive is similar to the page directive, but applies to tag files instead
of JSPs. Like the page directive, a translation unit can contain more than one
instance of the tag directive, all the attributes will apply to the complete translation unit (i.e. tag directives are position independent). There shall be only one occurrence of any attribute/value defined by this directive in a given translation unit,unless the values for the duplicate attributes are identical for all occurrences. The import and pageEncoding attributes are exempt from this rule and can appear multiple times. Multiple uses of the import attribute are cumulative (with ordered set union semantics). Other such multiple attribute/value (re)definitions result in a fatal translation error if the values do not match."
Comment 11 Mark Thomas 2010-11-19 13:22:37 UTC
Tag directives fixed too
Comment 12 Arif Chowdhury 2011-03-19 06:07:04 UTC
I have tried on apache-tomcat-7.0.11 problem is still there and not yet fixed. if fixed by this time. please guide us how to resolve.
Comment 13 Mark Thomas 2011-03-19 06:29:53 UTC
This issue is fixed.

Bugzilla is not a support forum. Please direct your enquiry to the Tomcat users mailing list.