Bug 325818

Comment 1 Rakesh 2010-09-21 04:34:25 EDT

When we are trying to parse embedded container, if encounter a inappropriate value we are returning UNDEFINED region. Problem with UNDEFINED  is , it will cause function assembleEmbeddedContainer() to exit prematurely.For  label="previous,<<   Back|next,Next>>" it will exit after '|'.But if we are in ST_XML_ATTRIBUTE_VALUE_DQUOTED(or single quote) state , then most probably it is part of XML_TAG_ATTRIBUTE_VALUE. I am reverting part of patch(only jflex file) for Bug 316223.

Comment 2 Rakesh 2010-09-21 04:43:05 EDT

Created attachment 179293 [details]
patch

Comment 3 Nick Sandonato 2010-09-22 18:15:55 EDT

Patch looks appropriate. Thanks, Rakesh.

Comment 4 Nick Sandonato 2010-09-27 16:22:06 EDT

Actually, on a second go with this defect, I've noticed that it fails a single unit test in JSP UI's test suite. When investigating, I noticed it was fairly important in that it was causing the document past the end of the double quote terminating the attribute value to be considered part of the attribute value. The example is this:

<a type="a<4"/>

You'll notice if you bring that up in an editor with these changes that the /> becomes part of the attribute value.

Please check out org.eclipse.jst.jsp.ui.tests.other.ScannerUnitTests.testJSPInvalidTagNameInAttValue() for the unit test failure.

Thanks.

Comment 5 Rakesh 2010-09-28 02:07:23 EDT

Created attachment 179709 [details]
patch

Looks like idea to popup last character was just for cases like this.

Comment 6 Nick Sandonato 2010-09-28 11:25:53 EDT

I think we're getting close, Rakesh. Unfortunately, I found a fairly simple scenario that's failing (and probably why the code was written to return UNDEFINED and then the assembleEmbeddedContainer looked for the last region in the list to find if it was UNDEFINED to terminate the assembly).

<button value="<"></button>

will cause the rest of the document to be treated as an attribute value.

Comment 7 Rakesh 2010-09-29 06:44:27 EDT

(In reply to comment #6)
> I think we're getting close, Rakesh. Unfortunately, I found a fairly simple
> scenario that's failing (and probably why the code was written to return
> UNDEFINED and then the assembleEmbeddedContainer looked for the last region in
> the list to find if it was UNDEFINED to terminate the assembly).
> 
> <button value="<"></button>
> 
> will cause the rest of the document to be treated as an attribute value.

Hi, Nick.It is getting complicated now :).From my analysis  of problem in hand, we need more specific information than UNDEFINED(not to exit assembleEmbeddedContainer  on characters like '|') and XML_ATTRIBUTE_VALUE(to exit assembleEmbeddedContainer  on '\"', or '\'',this problem is mentioned in your last comment), to get out of assembleEmbeddedContainer  properly.So instead of returning general XML_ATTRIBUTE_VALUE, i am returning more specific XML_TAG_ATTRIBUTE_VALUE_DQUOTE if last character in "inapproriate tag name" is '\"'(similar for single quote).I am not too familiar with Tokenizer class.Please let me know if my analysis is wrong.

I have taken following sample to do analysis and testing :

<siteedit:navbar
	label='previous,<<   Back|next,Next>>' spec="horizontal-button.jsp" />
	
<a href=\"<c:out value="test.html"></c:out>\">Test</a>



<button value="<<Previous"></button>

<button value='<page>'></button>

<button value = 'Next>>'></button>

<a type="a<4"/>

<button value='<'></button>

Comment 8 Rakesh 2010-09-29 06:45:22 EDT

Created attachment 179825 [details]
patch

Comment 9 Nick Sandonato 2010-09-29 11:17:15 EDT

I think you've got the right fix now, Rakesh. I think the route you took is the appropriate one. I'm updating your patch slightly, though. I think we need to check the state stack to verify that we pair the correct quote type.

Comment 10 Nick Sandonato 2010-09-29 11:17:46 EDT

Created attachment 179858 [details]
tweak to Rakesh's patch

Comment 11 Carl Anderson 2010-10-04 14:41:00 EDT

Committed to R3_2_2_patches