From cbf0b17ba5fdc9c28cac167b2b9929aeabc6da0f Mon Sep 17 00:00:00 2001 From: Lum Date: Mon, 26 Oct 2026 12:53:12 -0700 Subject: [PATCH 1/5] Custom file roots should be considered valid server managed roots. --- .../labkey/api/files/FileContentService.java | 13 +- .../controllers/exp/ExperimentController.java | 4 +- .../filecontent/FileContentServiceImpl.java | 112 ++++++++++++------ .../labkey/filecontent/FileRootManager.java | 6 + 4 files changed, 93 insertions(+), 42 deletions(-) diff --git a/api/src/org/labkey/api/files/FileContentService.java b/api/src/org/labkey/api/files/FileContentService.java index 9ae586b1fe7..8ef4771131d 100644 --- a/api/src/org/labkey/api/files/FileContentService.java +++ b/api/src/org/labkey/api/files/FileContentService.java @@ -68,10 +68,19 @@ static void setInstance(FileContentService impl) } /** - * Returns a list of Container in which the path resides. + * Returns the container whose file root holds the path, or null if none does. */ + @Nullable + Container getContainerForFilePath(java.nio.file.Path path); + + /** @deprecated use {@link #getContainerForFilePath(java.nio.file.Path)} */ + @Deprecated @NotNull - List getContainersForFilePath(java.nio.file.Path path); + default List getContainersForFilePath(java.nio.file.Path path) + { + Container c = getContainerForFilePath(path); + return c == null ? List.of() : List.of(c); + } /** * Returns the file root of the specified container. If not explicitly defined, diff --git a/experiment/src/org/labkey/experiment/controllers/exp/ExperimentController.java b/experiment/src/org/labkey/experiment/controllers/exp/ExperimentController.java index 10bc1c7caeb..62a17af3a5a 100644 --- a/experiment/src/org/labkey/experiment/controllers/exp/ExperimentController.java +++ b/experiment/src/org/labkey/experiment/controllers/exp/ExperimentController.java @@ -2452,8 +2452,8 @@ protected ModelAndView getDataView(DataForm form, BindException errors) throws I if (!underFileRoot) { // Issue 35649: ImmPort module "publish" creates exp.data object in this container for paths that originate in a different container - List containers = fileSvc.getContainersForFilePath(dataPath); - if (containers.isEmpty() || containers.stream().noneMatch(c -> c.hasPermission(getUser(), ReadPermission.class))) + Container fileContainer = fileSvc.getContainerForFilePath(dataPath); + if (fileContainer == null || !fileContainer.hasPermission(getUser(), ReadPermission.class)) throw new UnauthorizedException("Data file is not under the pipeline root for this folder"); } } diff --git a/filecontent/src/org/labkey/filecontent/FileContentServiceImpl.java b/filecontent/src/org/labkey/filecontent/FileContentServiceImpl.java index ca6ef515ee3..21b1df803e9 100644 --- a/filecontent/src/org/labkey/filecontent/FileContentServiceImpl.java +++ b/filecontent/src/org/labkey/filecontent/FileContentServiceImpl.java @@ -193,57 +193,66 @@ private FileContentServiceImpl() } @Override - @NotNull - public List getContainersForFilePath(java.nio.file.Path path) + public @Nullable Container getContainerForFilePath(java.nio.file.Path path) { // Ignore cloud files for now if (FileUtil.hasCloudScheme(path)) - return Collections.emptyList(); + return null; - // If the path is under the default root, do optimistic simple match for containers under the default root - File defaultRoot = getSiteDefaultRoot(); - java.nio.file.Path defaultRootPath = defaultRoot.toPath(); + // Anchor on the most specific root holding the path: the site default root or a custom file root + Container anchor = null; + java.nio.file.Path anchorPath = null; + java.nio.file.Path defaultRootPath = getSiteDefaultRoot().toPath(); if (path.startsWith(defaultRootPath)) { - java.nio.file.Path rel = defaultRootPath.relativize(path); - if (rel.getNameCount() > 0) + anchor = ContainerManager.getRoot(); + anchorPath = defaultRootPath; + } + + for (FileRoot root : FileRootManager.get().getFileRoots()) + { + Container c = ContainerManager.getForId(root.getContainer()); + if (c == null || c.isRoot() || isUseDefaultRoot(c)) + continue; + java.nio.file.Path rootPath = getExistingFileRootPath(c); + if (rootPath != null && !FileUtil.hasCloudScheme(rootPath) && path.startsWith(rootPath) && (anchorPath == null || rootPath.startsWith(anchorPath))) { - Container root = ContainerManager.getRoot(); - Container next = root; - while (rel.getNameCount() > 0) - { - // check if there exists a child container that matches the next path segment - java.nio.file.Path top = rel.subpath(0, 1); - assert top != null; - Container child = next.getChild(top.getFileName().toString()); - if (child == null) - break; + anchor = c; + anchorPath = rootPath; + } + } - next = child; + if (anchor == null) + return null; - if(rel.getNameCount() > 1) - { - rel = rel.subpath(1, rel.getNameCount()); - } - else - { - break; - } - } + // Child folder roots default to a matching subdirectory, so walk the path's segments down the container tree + List candidates = new ArrayList<>(List.of(anchor)); + Container next = anchor; - if (next != null && !next.equals(root)) - { - // verify our naive file path is correct for the container -- it may have a file root other than the default - java.nio.file.Path fileRoot = getFileRootPath(next); - if (fileRoot != null && path.startsWith(fileRoot)) - return Collections.singletonList(next); - } - } + for (java.nio.file.Path segment : anchorPath.relativize(path)) + { + next = next.getChild(segment.toString()); + if (next == null) + break; + candidates.add(next); + } + + // A folder may have its own root elsewhere, so take the deepest candidate whose actual root holds the path + for (Container c : candidates.reversed()) + { + if (c.isRoot()) + continue; + java.nio.file.Path fileRoot = getExistingFileRootPath(c); + if (fileRoot != null && path.startsWith(fileRoot)) + return c; } - // TODO: Create cache of file root and pipeline root paths -> list of containers + return null; + } - return Collections.emptyList(); + private @Nullable java.nio.file.Path getExistingFileRootPath(@NotNull Container c) + { + return isFileRootDisabled(c) ? null : resolveFileRootPath(c, false); } @Override @@ -1849,6 +1858,33 @@ public void fileRootsTest() } + @Test + public void testContainerForFilePath() + { + //pre-clean + cleanup(); + + FileContentService svc = FileContentService.get(); + Assert.assertNotNull(svc); + + Container project1 = ContainerManager.createContainer(ContainerManager.getRoot(), PROJECT1, TestContext.get().getUser()); + Container subfolder1 = ContainerManager.createContainer(project1, PROJECT1_SUBFOLDER1, TestContext.get().getUser()); + Container project2 = ContainerManager.createContainer(ContainerManager.getRoot(), PROJECT2, TestContext.get().getUser()); + + java.nio.file.Path project2File = svc.getFileRootPath(project2, ContentType.files).resolve(TXT_FILE); + Assert.assertEquals("Default root path", project2, svc.getContainerForFilePath(project2File)); + + File testRoot = getTestRoot(); + svc.setFileRoot(project1, testRoot); + + java.nio.file.Path subfolderFile = svc.getFileRootPath(subfolder1, ContentType.files).resolve(TXT_FILE); + Assert.assertEquals("Inherited custom root path", subfolder1, svc.getContainerForFilePath(subfolderFile)); + Assert.assertEquals("Custom root path", project1, svc.getContainerForFilePath(testRoot.toPath().resolve("unmatched").resolve(TXT_FILE))); + + java.nio.file.Path unmanaged = svc.getSiteDefaultRoot().toPath().resolve(FILE_ROOT_SUFFIX + "Unmanaged").resolve(TXT_FILE); + Assert.assertNull("Unmanaged path", svc.getContainerForFilePath(unmanaged)); + } + private void assertPathsEqual(String msg, File expected, File actual) { String expectedPath = FileUtil.getAbsoluteCaseSensitiveFile(expected).getPath(); diff --git a/filecontent/src/org/labkey/filecontent/FileRootManager.java b/filecontent/src/org/labkey/filecontent/FileRootManager.java index 03eec39df27..7f4c307482a 100644 --- a/filecontent/src/org/labkey/filecontent/FileRootManager.java +++ b/filecontent/src/org/labkey/filecontent/FileRootManager.java @@ -28,6 +28,7 @@ import org.labkey.api.files.FileRoot; import org.labkey.api.security.User; +import java.util.Collection; import java.util.Collections; import java.util.HashMap; import java.util.Map; @@ -72,6 +73,11 @@ public FileRoot getFileRoot(Container c) return null == root ? new FileRoot(c) : root; } + public Collection getFileRoots() + { + return CACHE.get(CACHE_KEY).values(); + } + public void deleteFileRoot(Container c) { SimpleFilter filter = SimpleFilter.createContainerFilter(c); From 69e68d50145ad27e1777cf8ef0082558623736de Mon Sep 17 00:00:00 2001 From: Lum Date: Mon, 26 Oct 2026 13:34:02 -0700 Subject: [PATCH 2/5] simplify --- .../src/org/labkey/filecontent/FileContentServiceImpl.java | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/filecontent/src/org/labkey/filecontent/FileContentServiceImpl.java b/filecontent/src/org/labkey/filecontent/FileContentServiceImpl.java index 21b1df803e9..76260156372 100644 --- a/filecontent/src/org/labkey/filecontent/FileContentServiceImpl.java +++ b/filecontent/src/org/labkey/filecontent/FileContentServiceImpl.java @@ -211,11 +211,14 @@ private FileContentServiceImpl() for (FileRoot root : FileRootManager.get().getFileRoots()) { + // Skip cloud roots before resolving: resolution goes through CloudStoreService, which may be absent or misconfigured + if (isCloudFileRoot(root.getPath())) + continue; Container c = ContainerManager.getForId(root.getContainer()); if (c == null || c.isRoot() || isUseDefaultRoot(c)) continue; java.nio.file.Path rootPath = getExistingFileRootPath(c); - if (rootPath != null && !FileUtil.hasCloudScheme(rootPath) && path.startsWith(rootPath) && (anchorPath == null || rootPath.startsWith(anchorPath))) + if (rootPath != null && path.startsWith(rootPath) && (anchorPath == null || rootPath.startsWith(anchorPath))) { anchor = c; anchorPath = rootPath; From 375f8e015caf9b4623824ddbd0edf2175605ffb0 Mon Sep 17 00:00:00 2001 From: Lum Date: Tue, 27 Oct 2026 16:36:34 -0700 Subject: [PATCH 3/5] use URIUtil.isDescendant --- .../src/org/labkey/filecontent/FileContentServiceImpl.java | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/filecontent/src/org/labkey/filecontent/FileContentServiceImpl.java b/filecontent/src/org/labkey/filecontent/FileContentServiceImpl.java index 76260156372..dcfedfdf70c 100644 --- a/filecontent/src/org/labkey/filecontent/FileContentServiceImpl.java +++ b/filecontent/src/org/labkey/filecontent/FileContentServiceImpl.java @@ -203,7 +203,7 @@ private FileContentServiceImpl() Container anchor = null; java.nio.file.Path anchorPath = null; java.nio.file.Path defaultRootPath = getSiteDefaultRoot().toPath(); - if (path.startsWith(defaultRootPath)) + if (URIUtil.isDescendant(defaultRootPath.toUri(), path.toUri())) { anchor = ContainerManager.getRoot(); anchorPath = defaultRootPath; @@ -218,7 +218,7 @@ private FileContentServiceImpl() if (c == null || c.isRoot() || isUseDefaultRoot(c)) continue; java.nio.file.Path rootPath = getExistingFileRootPath(c); - if (rootPath != null && path.startsWith(rootPath) && (anchorPath == null || rootPath.startsWith(anchorPath))) + if (rootPath != null && URIUtil.isDescendant(rootPath.toUri(), path.toUri()) && (anchorPath == null || URIUtil.isDescendant(rootPath.toUri(), anchorPath.toUri()))) { anchor = c; anchorPath = rootPath; @@ -246,7 +246,7 @@ private FileContentServiceImpl() if (c.isRoot()) continue; java.nio.file.Path fileRoot = getExistingFileRootPath(c); - if (fileRoot != null && path.startsWith(fileRoot)) + if (fileRoot != null && URIUtil.isDescendant(fileRoot.toUri(), path.toUri())) return c; } From c8f108d0c7c62aa1370c869d6e7a5079e7d64181 Mon Sep 17 00:00:00 2001 From: Lum Date: Tue, 27 Oct 2026 16:47:59 -0700 Subject: [PATCH 4/5] typo --- .../src/org/labkey/filecontent/FileContentServiceImpl.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/filecontent/src/org/labkey/filecontent/FileContentServiceImpl.java b/filecontent/src/org/labkey/filecontent/FileContentServiceImpl.java index dcfedfdf70c..473b31884fa 100644 --- a/filecontent/src/org/labkey/filecontent/FileContentServiceImpl.java +++ b/filecontent/src/org/labkey/filecontent/FileContentServiceImpl.java @@ -218,7 +218,7 @@ private FileContentServiceImpl() if (c == null || c.isRoot() || isUseDefaultRoot(c)) continue; java.nio.file.Path rootPath = getExistingFileRootPath(c); - if (rootPath != null && URIUtil.isDescendant(rootPath.toUri(), path.toUri()) && (anchorPath == null || URIUtil.isDescendant(rootPath.toUri(), anchorPath.toUri()))) + if (rootPath != null && URIUtil.isDescendant(rootPath.toUri(), path.toUri()) && (anchorPath == null || URIUtil.isDescendant(anchorPath.toUri(), rootPath.toUri()))) { anchor = c; anchorPath = rootPath; From 439bfb342701f02739dea82515fd008d71ed0bd6 Mon Sep 17 00:00:00 2001 From: lum Date: Thu, 29 Oct 2026 11:44:33 -0700 Subject: [PATCH 5/5] Protect against falling back to the parent for custom file roots. --- .../filecontent/FileContentServiceImpl.java | 32 ++++++++++--------- 1 file changed, 17 insertions(+), 15 deletions(-) diff --git a/filecontent/src/org/labkey/filecontent/FileContentServiceImpl.java b/filecontent/src/org/labkey/filecontent/FileContentServiceImpl.java index 473b31884fa..974a993daf2 100644 --- a/filecontent/src/org/labkey/filecontent/FileContentServiceImpl.java +++ b/filecontent/src/org/labkey/filecontent/FileContentServiceImpl.java @@ -229,28 +229,22 @@ private FileContentServiceImpl() return null; // Child folder roots default to a matching subdirectory, so walk the path's segments down the container tree - List candidates = new ArrayList<>(List.of(anchor)); - Container next = anchor; + Container owner = anchor; for (java.nio.file.Path segment : anchorPath.relativize(path)) { - next = next.getChild(segment.toString()); - if (next == null) + Container child = owner.getChild(segment.toString()); + if (child == null) break; - candidates.add(next); + owner = child; } - // A folder may have its own root elsewhere, so take the deepest candidate whose actual root holds the path - for (Container c : candidates.reversed()) - { - if (c.isRoot()) - continue; - java.nio.file.Path fileRoot = getExistingFileRootPath(c); - if (fileRoot != null && URIUtil.isDescendant(fileRoot.toUri(), path.toUri())) - return c; - } + // A child's default root sits beside the parent's @files, so a moved child's old location belongs to no folder + if (owner.isRoot()) + return null; - return null; + java.nio.file.Path fileRoot = getExistingFileRootPath(owner); + return fileRoot != null && URIUtil.isDescendant(fileRoot.toUri(), path.toUri()) ? owner : null; } private @Nullable java.nio.file.Path getExistingFileRootPath(@NotNull Container c) @@ -1886,6 +1880,14 @@ public void testContainerForFilePath() java.nio.file.Path unmanaged = svc.getSiteDefaultRoot().toPath().resolve(FILE_ROOT_SUFFIX + "Unmanaged").resolve(TXT_FILE); Assert.assertNull("Unmanaged path", svc.getContainerForFilePath(unmanaged)); + + File subfolderRoot = new File(testRoot, "CustomSubfolder"); + subfolderRoot.mkdirs(); + svc.setFileRoot(subfolder1, subfolderRoot); + + java.nio.file.Path movedSubfolderFile = svc.getFileRootPath(subfolder1, ContentType.files).resolve(TXT_FILE); + Assert.assertEquals("Moved custom root path", subfolder1, svc.getContainerForFilePath(movedSubfolderFile)); + Assert.assertNull("Stale default root path", svc.getContainerForFilePath(subfolderFile)); } private void assertPathsEqual(String msg, File expected, File actual)