Some Eclipse Foundation services are deprecated, or will be soon. Please ensure you've read this important communication.

Bug 143912

Summary:

StackOverFlowError happens with JSPTokenizer when I paste many JSP comments

Product:

[WebTools] WTP Source Editing

Reporter:

hoshiura <hoshiura>

Component:

jst.jsp

Assignee:

Nitin Dahyabhai <thatnitind>

Status:

CLOSED FIXED

QA Contact:

David Williams <david_williams>

Severity:

critical

Priority:

CC:

david_williams, jeffliu, patric, uchi

Version:

1.5

Target Milestone:

1.5 RC5

Hardware:

OS:

Windows XP

Whiteboard:

PMC_approved

Attachments:

Description	Flags
A JUnit test case in org.eclipse.jst.jsp.core.tests plugin	none
updated test for inclusion in our BVT	none
proposed changes for JSPTokenizer	none
patched jar to fix this problem	none

Description hoshiura

2006-05-26 05:34:03 EDT

When I paste JSP comment in *.jspf file, StackOverFlowError happens with JSPTokenizer. This might be side effect of 98258.patch. When I apply the patch as reverse one, the error seems to go way.

Could you fix this for RC4?

*Steps
1. Create empty a.jspf and open it with JSP source editor.
2. Type JSP comments(<%-- -->) in it.
3. Repeat step#2.
--> StackOverFlowError happens. When I saw it, the file size was about 6kbyte.

Comment 1 David Williams

2006-05-26 11:44:19 EDT

If the fix for this isn't obvious, we should revert the fix for 
bug 98258 for RC4 since this is worse that original problem. 

Hoshiura-san, if you have time and ability, it would be great to have 
a "headless" JUnit test that demonstrated the problem.

Comment 2 hoshiura

2006-05-29 07:07:10 EDT

Created attachment 42854 [details]
A JUnit test case in org.eclipse.jst.jsp.core.tests plugin

Hi, I attached a JUnit test case to reproduce StackOverFlowError. The patch contains a test case file and a test data.

Comment 3 Nitin Dahyabhai

2006-05-29 22:24:36 EDT

Created attachment 42915 [details]
updated test for inclusion in our BVT

Comment 4 Nitin Dahyabhai

2006-05-29 22:31:09 EDT

Created attachment 42916 [details]
proposed changes for JSPTokenizer

Thank you for the original Unit Test.  I've modified it so it can be added easily to our BVT and used it to track down the problem.  When a JSP comment exists in a standalone area, we were pushing the current state onto a stack to return to it after parsing the full comment.  The error was that the state was pushed at all--not being within a tag meant no state stack usage or other adjustment was needed.  The patch simply removes the pushing onto the stack and uses the current state (instead of the now-newest entry on the stack) to help decide what to do next.

Comment 5 Nitin Dahyabhai

2006-05-29 22:32:55 EDT

The attached unit test(s) and existing unit test suites pass with the patch.  CCing David for review.

Comment 6 David Williams

2006-05-29 22:53:40 EDT

Nitin, approved. 
As we discussed, the "peek" in original code was done in error. 
Please smoke test well :) 
but, let's put in RC4. 
please release.

Comment 7 David Williams

2006-06-01 03:52:49 EDT

+1

Comment 8 David Williams

2006-06-01 03:59:25 EDT

*** Bug 144807 has been marked as a duplicate of this bug. ***

Comment 9 Arthur Ryman

2006-06-01 14:37:02 EDT

+1 for WTP 1.5 RC5.

Comment 10 Jeffrey Liu

2006-06-02 10:53:29 EDT

Use the new PMC approval process.

Comment 11 Naci Dai

2006-06-02 13:35:46 EDT

+1 for RC5

If it is good for David it is good for me - he should definitely know this one :-)

Comment 12 Nitin Dahyabhai

2006-06-02 14:07:49 EDT

Released for 1.5 RC5

Comment 13 David Williams

2006-06-04 03:45:10 EDT

Created attachment 43416 [details]
patched jar to fix this problem

I kept getting a lot of these errors, just do to JSP indexing etc., using 
RC4 as my self hosting environment. This might just be because I have lots of "goofy" JSP's in my workspace for testing ... but, if others encounter this problem, you can try this "developer built" plugin, until RC5 comes out. 
You chould just be able to copy this into your plugins directory (and restart with -clean) .. no need to delete the old, bcause I make version number compatible.

Comment 14 hoshiura

2006-06-27 02:38:57 EDT

I verified this with RC6.