SOLR-12429: Prevent symbolic links from being uploaded as part of a configset #2651
Conversation
| @@ -60,6 +60,14 @@ public static void assertConfigSetFolderLegal(Path confPath) throws IOException | |||
| new SimpleFileVisitor<>() { | |||
| @Override | |||
| public FileVisitResult visitFile(Path file, BasicFileAttributes attrs) { | |||
| if (Files.isSymbolicLink(file)) { | |||
There was a problem hiding this comment.
Ah so much easier then when I had to detect symlinks for Apache Ant back in the day :) (https://bz.apache.org/bugzilla/show_bug.cgi?id=1550#c14)
However, I'm not sure it's valid to assume the user never wants to follow symbolic link. I can certainly imagine cases where folks would use a symlink for things they wanted to keep synced across config sets. It seems to me the correct behavior is to A) check for loops and fail if the symlinks are circular, and B) unroll the symlink (possibly emitting a warning) so that the contents are represented as the same tree that a person navigating with a shell would perceive.
There was a problem hiding this comment.
^ me on wrong browser again
There was a problem hiding this comment.
So, right now symbolic links don't work, they blow up with a cryptic error. This PR 's goal is to replace the cryptic error with a nicer error! I think your suggestion is interesting, and if we wanted to support symbolic link lookup, that sounds like a nice addition to do in a separate PR that removes the limitation. It isn't an immediate itch for me to scratch however. Does that sound reasonable?
| @@ -60,6 +60,14 @@ public static void assertConfigSetFolderLegal(Path confPath) throws IOException | |||
| new SimpleFileVisitor<>() { | |||
| @Override | |||
| public FileVisitResult visitFile(Path file, BasicFileAttributes attrs) { | |||
| if (Files.isSymbolicLink(file)) { | |||
There was a problem hiding this comment.
^ me on wrong browser again
|
I am going to merge this PR, however if we want to pair on supporting symbolic links in another PR, all for supporting that! |
https://issues.apache.org/jira/browse/SOLR-12429
Description
Preventing the upload of a configset via bin/solr zk upload that contains symbolic links to files and dirs. This prevents a obscure error message, and tells the user what went wrong.
Solution
I added a check in
FileTypeMagicUtilwhich maybe not quite perfect, but let me tap into a custom error message. Maybe the class should be renamed to "ConfigSetFileValidator" ???Because of java security manager limitations, my attempt to create a symbolic file and directory in a unit test failed, so I backed out and added a BATS style test instead.
I did notice some other places where MAGIC file checking is done, BackupManager, ZkConfigSetService.. But not sure in those situations you can actually get a symbolic link in those places, since you are "inside solr" not on a file system...
Tests
Bats and manual.
Checklist
Please review the following and check all that apply:
mainbranch../gradlew check.