bug fix: silently fails overwriting symlinks

[mkarg]

This is a fix MNG-6048

When A is an existing symlink to B, then createSymbolicLink(A,C) does
neither overwrite A->B by A->C (as expected in analogy to the behavior
of copy(A,C)) nor does it throw an exception nor does it return A->B to
indicate the failure, but it actually "silently fails", i. e. it returns
A->C!

This certainly is heavily problematic, unsymmetric to what
copy(File,File) and Files.createSymbolicLink(Path,Path) do, and
certainly unwanted and buggy behavior.

The solution is to delete any existing target before creating the
symlic, hence copying the behavior of copy(File,File).

[khmarbaise]

Do we have JIRA issue for that ? I don't see a reference in the PR. If i think about that. Is it possible to identify this situation and produce a warning or error ? And what about a test ?

[mkarg]

Opened JIRA MNG-6048 to describe this issue.

I could change to code to add a warning, but it would make the code less stable, as the solution currently uses an atomic OS operation (delete if exists), while splitting in two operations (if exists then delete) imposes the situation of a stale delete in case somebody else removed the file in-between.

About tests: Do you always add an integration test for each bug or is this a particular demand for this very bug report?

[michael-o]

Interesting issue. How does ln -s behave in this situation? Does it fail with an appropriate exit code?

[mkarg]

Yes, ln -s fails in that case:

$ touch a
$ touch b
$ ln -s a c
$ ln -s b c
ln: failed to create symbolic link "c": file exists
$ echo $?
1

[michael-o]

Just read and your code. This is a good idea but some things are still don't clear. If ln -s fails we should fail too instead of deleting the link and recreating from a new source. I would expect an exception. Can you write a simple Java demo which depicts the issue. I certain that this has to be raised with Oracle.

[michael-o]

Based on this, I get this result on FreeBS 11.0-RELEASE:

Exception in thread "main" java.nio.file.FileAlreadyExistsException: /tmp/ln1
        at sun.nio.fs.UnixException.translateToIOException(UnixException.java:88)
        at sun.nio.fs.UnixException.rethrowAsIOException(UnixException.java:102)
        at sun.nio.fs.UnixException.rethrowAsIOException(UnixException.java:107)
        at sun.nio.fs.UnixFileSystemProvider.createSymbolicLink(UnixFileSystemProvider.java:457)
        at java.nio.file.Files.createSymbolicLink(Files.java:1043)
        at Test.main(Test.java:15)

[mkarg]

I think this discussion deviates a bit too far from the origin: The current code silently fails, so the caller assumes the link was overwritten, while it is not. That is what I solved. Not more. What you ask for is a different feature: Do not overwrite existing files and links without explicit grant. Feel free to open a different feature proposal for that. :-)

[mkarg]

Committers: If there is any reason why my contribution is not considered for merge, then please tell me, but do not further stay silent.

[michael-o]

Thinking about this one more time, I do think that we should fail just like ln -s who do. Silent overwrite is not default and we shouldn't do it. @khmarbaise what do you think?

[mkarg]

@michael-o @khmarbaise Any news?