Bugzilla – Full Text Bug Listing
| Summary: | [deployables] ArrayIndexOutOfBoundsException in ComponentDeployable | ||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|
| Product: | [WebTools] WTP Java EE Tools | Reporter: | Gunnar Wagenknecht <gunnar> | ||||||||
| Component: | wst.web | Assignee: | John Lanuti <jlanuti> | ||||||||
| Status: | CLOSED FIXED | QA Contact: | Chuck Bridgham <cbridgha> | ||||||||
| Severity: | critical | ||||||||||
| Priority: | P2 | CC: | cbridgha, david_williams | ||||||||
| Version: | 1.5 | ||||||||||
| Target Milestone: | 1.5 RC6 | ||||||||||
| Hardware: | All | ||||||||||
| OS: | All | ||||||||||
| Whiteboard: | PMC_approved | ||||||||||
| Attachments: |
|
||||||||||
|
Description
Gunnar Wagenknecht
I looked into the code and the problematic line contains: pathSegments[moduleSegments.length > 0 ? moduleSegments.length : 0] However, the code does not check if pathSegments.length > moduleSegments.length. I'm also curious if the last check-in broke it. http://dev.eclipse.org/viewcvs/index.cgi/wst/components/web/plugins/org.eclipse.wst.web/static_web_project/org/eclipse/wst/web/internal/deployables/ComponentDeployable.java.diff?r1=1.10&r2=1.11&cvsroot=WebTools_Project According to the check-in comment the suspicious change was made to fix bug 119286. Created attachment 45084 [details]
Proposed patch
Here is a proposed patch to add the greater than check. Can you test with this patch?
Patch attached. Added a safe check to ensure we don't get an array index out of bounds exception. This code was recently reworked to improve performance and passed our scenarios, so not sure what is different about Gunnar's. At any rate, this change makes sense and is very safe and contained. I know about the performance improvements and that's why I think the code needs some additional changes instead of just avoiding the AIOOBE.
I debugged through my case when the AIOOBE is thrown and the interesting thing is that pathSegments.length == moduleSegments.length. If this happens and startsWith(...) returns true then the paths are equal. That also means that the method must return this resource because it was explicitly asked for it.
Just for the record, the path#toString() is "WEB-INF/classes/aSubfolder" and moduleResource#toString() is "ModuleFolder [aSubfolder, WEB-INF/classes/aSubfolder]".
So, the additional check to avoid the AIOOBE is ok. But there must be a problem in the preceding code (line 238-244).
// If the last segment in passed in path equals the module resource name
// and segment count is the same and the path segments start with the module path segments
// then we have a match and we return the existing moduleResource
if (pathSegments[pathSegments.length - 1].equals(moduleResource.getName()) &&
(moduleSegments.length + 1) == pathSegments.length &&
startsWith(moduleSegments, pathSegments))
return moduleResource;
The comment clearly stated "and segment count is the same". This clashes with the check "(moduleSegments.length + 1) == pathSegments.length". Either I have a suspicious moduleResource or the check should be simply moduleSegments.length == pathSegments.length.
Some more debugging info about the module resource: ModuleResource#container = F/myProject/build/war/classes/aSubFolder ModuleResource#members = null ModuleResource#name = aSubFolder ModuleResource#path = WEB-INF/classes/aSubFolder "build/war/classes" is the output directory from the JDT compiler. Thus, "aSubFolder" is actually a start folder of package hierarchy. I'm not sure if I have a bogus module resource. For most resources the expression (moduleSegments.length + 1) == pathSegments.length is true. "ModuleFolder [aSubfolder, WEB-INF/classes/aSubfolder]" means that there is a folder called "aSubFolder" nested in the "aSubFolder" folder in "WEB-INF/classes". So, this case should fail. It is trying to find "WEB-INF/classes/aSubfolder/aSubFolder". Does this resource exist? That's what it is trying to do. (In reply to comment #8) > "ModuleFolder [aSubfolder, WEB-INF/classes/aSubfolder]" means that there is a > folder called "aSubFolder" nested in the "aSubFolder" folder in > "WEB-INF/classes". So, this case should fail. It is trying to find > "WEB-INF/classes/aSubfolder/aSubFolder". Does this resource exist? That's what > it is trying to do. A folder (or file) with the path "WEB-INF/classes/aSubfolder/aSubFolder" does not exist. According to the path passed to getExistingModuleResource it is trying to find "WEB-INF/classes/aSubfolder". During debugging I realized that this happens for different sub folders, too. Created attachment 45095 [details]
screeenshot of variables view with bogus module folders
See the attached screenshot. I created a simple detail formatter to print out the hierarchy. It looks like someone generates bogus module folders.
(In reply to comment #10) > Created an attachment (id=45095) [edit] > screeenshot of variables view with bogus module folders BTW, this screenshot is from a stack frame before the one that processes the list of resources and throws the exception. Gunnar, Can you attach a zip of your project or a similar project exposing this issue? I am not able to reproduce this behaviour on various web projects. (In reply to comment #12) > Gunnar, Can you attach a zip of your project or a similar project exposing this > issue? I sent you a workspace to your mail address. I was able to reproduce the problem there. Gunnar, can you just export the project and send it to me or attach here? It's not just a project. You need the workspace (contains two projects) I sent you. It's a stripped down version (without confidential code, just a few test classes) that is exposing the problem. I can't attach the workspace here, sorry. Created attachment 45116 [details]
Proposed patch
Hey Gunnar, I was able to reproduce with your test and you're right, it looks like a pretty basic problem with creating the module resources. I added an updated patch which works in my testing, can you give it a shot?
Thanks for the effort here!
(In reply to comment #17) > Created an attachment (id=45116) [edit] > Proposed patch The patch works fine. I could also verify that the bogus folders are gone. Thanks! Great, thanks Gunnar. The problem here was that the java folder module resources were getting create with a path with an extra segment. That is, its parent container was set to be itself. That created some confusion. The fix is trivial, set the parent path to be the parent. It is just a one word change and myself and Gunnar have verified it works. I have tested other scenarios and junits. This bug is ready for PMC approval. We will also add this particular test to our junit test bucket for deployables. +1 As I understand it, the main problem was a "typo" where 'newpath' was used instead of 'path' Plus, the patch includes some bullet proofing to gaurd the range can not get out of bound. Thanks all. This is released for the WTP 1.5 RC6 062206 driver. Dropped. verified 20060925 Verified, closing. |