Community
Participate
Working Groups
A bad key can be used to create a CodeReader entry in the CodeReaderCache that can not be modified. steps to reproduce: - create a standard make C++ project named "proj" - create a test.h header file with these contents: #ifndef _TEST_H_ #define _TEST_H_ int x; #endif /*_TEST_H_*/ - create a test.cpp source file with these contents: #include "/proj/test.h" - create a file named "makefile" with these contents: test.exe : test.o g++ -o test.exe test.o test.o : test.cpp g++ -c test.cpp all : test.exe clean : -rm test.exe test.o - build the project - close and reopen the project to pick up the discovered #includes - add the root project path to the list of project #includes - open the test.cpp source file in the DOM AST View - analyze the DOM AST View to ensure the int x declaration is there - update test.h file so that int x is int y instead - refresh the DOM AST View by clicking the "Refresh DOM AST" button - analyze the DOM AST View to see that the int x declaration did not go away This seems to be caused by the fix for 91086 at BaseScanner:line 2902. The BaseScanner uses an empty string to prefix the filename assuming that the filename is an absolute path. The cache finds the local file to be created and creates a CodeReader corresponding to the following bad key: "\proj\test.h" The empty string causes a bad key to be used (and continues to be used due to the bad #include in test.cpp).
Created attachment 23571 [details] proposed fix for this PR
I'm not sure if this patch is the completely right answer. We may need to put some logic in BaseScanner as well. Talk more when I get in.
#include "/proj/test.h" is supposed to not use the search path. pls update BaseScanner for this use case. Submit a patch for this & 100858 together if you can. Thanks
Also, we may be able to add a check in somewhere that ensures that a path provided is a canonical path. i.e. instead of using the generated path in the code reader constructor, you can take that path and do something like path = new File( path ).getCanonicalPath()
FYI RC1 starts next Monday. Pls have these bugs fixed by then or moved off to a later milestone.
Created attachment 23891 [details] updated patch for this PR updated so that the base scanner doesn't use the search path if the #included file starts with "/" or "\"... can you take a look at this and let me know what you think? What do you want to do with the canonical path? How would it differ from File.exists()? I think that the CodeReaderCache.get() should make sure that the file exists since the path being used as a key should always be the proper path to a file that exists.
Comment on attachment 23891 [details] updated patch for this PR Invalid patch, fails for windows absolute paths, i.e. C:\temp\file.c... will update in a bit.
Comment on attachment 23571 [details] proposed fix for this PR old patch
Patch is OK ... I would change the following: 1. CodeReaderCache shouldn't check File.exists() on get, but only on put() (the add function). 2. In ScannerUtility, use File.separatorChar rather than '\\' or '/' explicitly, it makes sure that on the right file system invalid characters are not being accepted. 3. Ditto issue for BaseScanner & File.separatorChar.
I know what you mean about canonical path checks and File.separatorChar now, bad mistakes on my behalf. Although I disagree with the "only on put instead of get" comment. If that was done then the cache will still always return a valid CodeReader (when it shouldn't because the file doesn't exist for the key). So the problem will continue, even though nothing is actually stored in the cache.
Q: Is File.exists() an expensive operation? If so, the code needs to be structured so that it's called less often than at the begining of every get().
Created attachment 23936 [details] fix for this PR This should be it... I can feel it :)
Nope. In BaseScanner you add the following import. +import org.eclipse.core.runtime.Path; This will not compile using our loadbuild scripts. The parser/ src folder cannot depend on anything other than the jre. Pls retry.
Created attachment 23956 [details] replaced Path#isAbsolute with File#isAbsolute
Patch applied.
FIXED in HEAD.
