From 6aeba4ab39c37da2d2d8409795a587da35e8e2f4 Mon Sep 17 00:00:00 2001 From: Shinsuke Sugaya Date: Sun, 6 Sep 2026 11:47:42 +0900 Subject: [PATCH] feat(fs): let a file crawl leave symbolic links alone FileSystemClient walks a directory with java.io.File, which follows a symbolic link and offers no way not to. A link is therefore crawled as though it were the directory or the file it points at, and the target is indexed a second time -- under the crawling configuration that reached the link, not the one that covers the target. The second copy carries the permissions of the link. Where a share is readable by more people than the tree it links into, anyone who can create a link on that share can have documents from the other tree re-indexed under their own roles and read them back through search. The same file also ends up in the index twice, which distorts the hit count and the ranking. Until now the only way to avoid it was to name the link in the excluded paths of every configuration. A followSymlink init parameter is added instead. It defaults to true, so nothing changes for an existing crawl; set client.followSymlink=false in the configuration parameters of a file crawling configuration and a link is refused with 403 and left out of the child urls of the directory it sits in, with one INFO line naming what was skipped. --- .../crawler/client/fs/FileSystemClient.java | 59 +++++++++++++++++ .../client/fs/FileSystemClientTest.java | 64 +++++++++++++++++++ 2 files changed, 123 insertions(+) diff --git a/fess-crawler/src/main/java/org/codelibs/fess/crawler/client/fs/FileSystemClient.java b/fess-crawler/src/main/java/org/codelibs/fess/crawler/client/fs/FileSystemClient.java index f0180d93f..2885d4af6 100644 --- a/fess-crawler/src/main/java/org/codelibs/fess/crawler/client/fs/FileSystemClient.java +++ b/fess-crawler/src/main/java/org/codelibs/fess/crawler/client/fs/FileSystemClient.java @@ -76,6 +76,9 @@ public class FileSystemClient extends AbstractCrawlerClient { /** Key for file groups in metadata */ public static final String FS_FILE_GROUPS = "fsFileGroups"; + /** Init parameter deciding whether a symbolic link is followed. */ + public static final String FOLLOW_SYMLINK_PROPERTY = "followSymlink"; + /** Character encoding for files */ protected String charset = Constants.UTF_8; @@ -86,6 +89,14 @@ public class FileSystemClient extends AbstractCrawlerClient { /** Flag to track initialization status */ protected AtomicBoolean isInit = new AtomicBoolean(false); + /** + * Whether a symbolic link is followed. Following one indexes the file it points at a second + * time, under the configuration that reached the link rather than the one that covers the + * target, so the copy carries the roles of the link. Where a share is readable by more people + * than the tree it links into, that is a way around the roles on the target. + */ + protected boolean followSymlink = true; + /** * Constructs a new FileSystemClient. */ @@ -166,6 +177,12 @@ protected ResponseData getResponseData(final String uri, final boolean includeCo responseData.setHttpStatusCode(Constants.NOT_FOUND_STATUS_CODE); responseData.setCharSet(charset); responseData.setContentLength(0); + } else if (isSkippedSymlink(file)) { + logger.info("Skipped a symbolic link: file={}", file.getAbsolutePath()); + responseData.setHttpStatusCode(Constants.FORBIDDEN_STATUS_CODE); + responseData.setCharSet(charset); + responseData.setContentLength(0); + responseData.setMimeType(APPLICATION_OCTET_STREAM); } else if (file.isFile()) { // check file size responseData.setContentLength(file.length()); @@ -226,6 +243,10 @@ protected ResponseData getResponseData(final String uri, final boolean includeCo logger.debug("Found {} child entries in directory: directory={}", files.length, file.getAbsolutePath()); } for (final File f : files) { + if (isSkippedSymlink(f)) { + logger.info("Skipped a symbolic link: file={}", f.getAbsolutePath()); + continue; + } final String childUri = f.toURI().toASCIIString(); requestDataSet.add(RequestDataBuilder.newRequestData().get().url(childUri).build()); } @@ -288,6 +309,44 @@ protected FileOwnerAttributeView parseFileOwnerAttribute(final ResponseData resp } } + @Override + public void init() { + super.init(); + followSymlink = getInitParameter(FOLLOW_SYMLINK_PROPERTY, Boolean.valueOf(followSymlink), Boolean.class).booleanValue(); + if (logger.isDebugEnabled()) { + logger.debug("followSymlink={}", followSymlink); + } + } + + /** + * Determines whether the given file must be left alone because it is a symbolic link and + * links are not being followed. + * + * @param file the file to check + * @return true if the file is to be skipped + */ + protected boolean isSkippedSymlink(final File file) { + return !followSymlink && Files.isSymbolicLink(file.toPath()); + } + + /** + * Returns whether a symbolic link is followed. + * + * @return true if symbolic links are followed + */ + public boolean isFollowSymlink() { + return followSymlink; + } + + /** + * Sets whether a symbolic link is followed. + * + * @param followSymlink true to follow symbolic links + */ + public void setFollowSymlink(final boolean followSymlink) { + this.followSymlink = followSymlink; + } + /** * Preprocesses a URI to ensure it's in the correct format for file system access. * diff --git a/fess-crawler/src/test/java/org/codelibs/fess/crawler/client/fs/FileSystemClientTest.java b/fess-crawler/src/test/java/org/codelibs/fess/crawler/client/fs/FileSystemClientTest.java index 69848fa9a..7748be2b8 100644 --- a/fess-crawler/src/test/java/org/codelibs/fess/crawler/client/fs/FileSystemClientTest.java +++ b/fess-crawler/src/test/java/org/codelibs/fess/crawler/client/fs/FileSystemClientTest.java @@ -74,6 +74,70 @@ public void test_doGet_dir() { } + /** + * A symbolic link is followed by default, which is what java.io.File does and what every + * release so far has done. + */ + @Test + public void test_followSymlink_isOnByDefault() { + assertTrue(fsClient.isFollowSymlink()); + } + + /** + * With links turned off, a link is not read. Following one indexes the file it points at a + * second time, under the configuration that reached the link rather than the one that covers + * the target, so the copy carries the roles of the link: on a share that more people can read + * than the tree it links into, that is a way around the roles on the target. + */ + @Test + public void test_doGet_symlink_isRefusedWhenLinksAreNotFollowed() throws Exception { + final File target = ResourceUtil.getResourceAsFile("test/text1.txt"); + final File link = new File(tempDir(), "link1.txt"); + try { + Files.createSymbolicLink(link.toPath(), target.toPath()); + } catch (final UnsupportedOperationException | java.io.IOException e) { + return; // the file system does not support symbolic links + } + + fsClient.setFollowSymlink(false); + final ResponseData responseData = fsClient.doGet(link.toURI().toASCIIString()); + assertEquals(Constants.FORBIDDEN_STATUS_CODE, responseData.getHttpStatusCode()); + assertNull(responseData.getResponseBody()); + } + + /** + * A link inside a directory is left out of the child urls, so it is never queued either. + */ + @Test + public void test_doGet_dir_leavesOutSymlinkChildrenWhenLinksAreNotFollowed() throws Exception { + final File target = ResourceUtil.getResourceAsFile("test/text1.txt"); + final File dir = new File(tempDir(), "withlink"); + assertTrue(dir.mkdirs() || dir.isDirectory()); + final File link = new File(dir, "link2.txt"); + try { + Files.createSymbolicLink(link.toPath(), target.toPath()); + } catch (final UnsupportedOperationException | java.io.IOException e) { + return; // the file system does not support symbolic links + } + + fsClient.setFollowSymlink(false); + try { + fsClient.doGet(dir.toURI().toASCIIString()); + fail(); + } catch (final ChildUrlsException e) { + for (final RequestData requestData : e.getChildUrlList()) { + assertFalse(requestData.getUrl().contains("link2.txt")); + } + } + } + + private File tempDir() { + final File dir = new File(System.getProperty("java.io.tmpdir"), "fsClientSymlinkTest" + System.nanoTime()); + assertTrue(dir.mkdirs()); + dir.deleteOnExit(); + return dir; + } + @Test public void test_doGet_file() throws Exception { final File file = ResourceUtil.getResourceAsFile("test/text1.txt");