Bug 315436

Comment 1 Tommy McGuire 2010-06-02 13:48:23 EDT

Created attachment 170848 [details]
Patch to org.eclipse.virgo.util.io.FileSystemUtils to avoid following symlinks.

This patch has not actually been tested with Virgo, but has been used under the SpringSource dm Server 1.0 line.

Comment 2 Glyn Normington 2010-06-02 22:38:40 EDT

Thanks for raising this. I'll need to schedule some time to look at the patch again. We did look at it during the 2.0 development phase but IIRC it seemed to have some limitations. Perhaps in the context of Virgo we can help you improve the patch so that it can be folded in.

Meahwile I'll turn on the iplog flag on this bug so that we can do due IP diligence on your patch and any sequels.

Comment 3 Glyn Normington 2010-06-03 05:08:11 EDT

I had a look at the patch and the only part I would query is the expression:

file.getAbsoluteFile().getParentFile().getCanonicalPath()

which will NPE is file does not have a parent file. Since isSymlink is a private method rather than a general purpose utility, it is probably reasonable to add a precondition in a comment. If you can improve the method to make it robust when file does not have a parent file, e.g. by returning false if that's always correct, that would be better still.

Also, please check you have considered how the code is likely to behave on Windows as I believe there are constructs analogous to symbolic links in Windows but I'm not sure of their properties. I think the code is probably robust, but I'd like you to confirm.

Finally, it would be great to include a unit testcase, although this will probably have to be platform specific. I don't mind if such a test no-ops on Windows given the obscurity of symbolic links on Windows. Please give some thought to how this might be tested. Perhaps giving the isSymlink method package visibility and then testing that method against a suitable set of files created in the target directory would be the best approach. The only sticky point is I'm not sure how you would create a symbolic link in Java 6.

Comment 4 Glyn Normington 2010-09-16 06:00:46 EDT

Hi Tommy

I just noticed there has been no activity on this bug since June. Are you interested in continuing with contributing the fix?

Thanks,
Glyn

Comment 5 Tommy McGuire 2010-09-23 14:45:45 EDT

I'm sorry about the lack of communication. I got dragged off on a SAML project and am only now getting back to Virgo.

I agree with all of your comments, and spent some time trying to figure out what would happen on Windows, but I never managed to figure out how to create anything that looks like a symbolic link on XP (which I am told exist, somehow) and I don't have a later version of Windows to test on. As far as I could tell, the behavior is unchanged by the patch with normal files and directories.

I suppose the best way to create symlinks in Java is to call out to ln, My application makes a JNI call, but that is not any better. From what I hear, Java 7 is supposed to add something that may or may not be symlinks, but.... :-)

How do you want to proceed? I can fix the patch and submit it or just keep it handy.

Comment 6 Glyn Normington 2010-09-24 04:57:55 EDT

Hi Tommy

Welcome back!

If you are going to fix the patch anyway, why not attach it here for the benefit of the Virgo community?

I'm afraid it seems doubtful that we will be able to integrate the patch without significant extra investigation into Windows behaviour and testing on multiple platforms, neither of which the current committers are likely to be able to prioritise. However, this may be something that someone else with the necessary Windows expertise and test environments will want to take forward, especially if they run into the same problem.

Regards,
Glyn

Comment 7 Glyn Normington 2011-01-31 07:38:58 EST

Closing. If you decide to create a fresh patch, please re-open.