This is an automated email from the ASF dual-hosted git repository. jsinovassin pushed a commit to branch UNOMI-973-review-followups in repository https://gitbox.apache.org/repos/asf/unomi.git
commit e76ed1e0d7b249abb80ef88bca6f6fbd279da431 Author: jsinovassin <[email protected]> AuthorDate: Tue Sep 1 18:31:35 2026 +0200 UNOMI-973: Resolve a path the way the file system does, one component at a time canonicalize() normalized the whole path before following its symbolic links. Collapsing parent segments first erases the component they cancel, symbolic link included: /base/link/.. read as /base, while the file system expands the link and applies the segment to its target, landing wherever that link points. A source written that way was accepted and its route was built. The path is now walked from the root: a link is expanded where it stands, and a parent segment applies to what the walk has resolved so far. What does not exist yet is kept as written, so an export destination is still decided on before it is created, and an existing part that cannot be resolved is still a refusal. --- .../apache/unomi/router/api/EndpointValidator.java | 66 ++++++++++++++++------ .../core/route/FileEndpointContainmentTest.java | 19 +++++++ 2 files changed, 69 insertions(+), 16 deletions(-) diff --git a/extensions/router/router-api/src/main/java/org/apache/unomi/router/api/EndpointValidator.java b/extensions/router/router-api/src/main/java/org/apache/unomi/router/api/EndpointValidator.java index aedf0b1e4..ad18305ed 100644 --- a/extensions/router/router-api/src/main/java/org/apache/unomi/router/api/EndpointValidator.java +++ b/extensions/router/router-api/src/main/java/org/apache/unomi/router/api/EndpointValidator.java @@ -93,6 +93,10 @@ public final class EndpointValidator { /** Stands in for a token that expands to a name: one path component, never a parent segment. */ private static final String NAME_PLACEHOLDER = "_"; + /** The path segments the walk interprets rather than resolves against the file system. */ + private static final String CURRENT_DIRECTORY = "."; + private static final String PARENT_DIRECTORY = ".."; + private EndpointValidator() { } @@ -189,29 +193,59 @@ public final class EndpointValidator { } /** - * Resolves a path to the one the file system would actually use: made absolute, stripped of its - * parent segments, and with the symbolic links of its existing part followed. A path that does not - * exist yet is canonicalized through its deepest existing ancestor — an export destination is - * created on first write, and must be decided on before it exists. + * Resolves a path to the one the file system would actually use, walking it one component at a + * time from the root the way the file system does: a symbolic link is expanded where it stands, + * and a parent segment is applied to what the walk has resolved so far. + * + * <p>The order is what makes this correct. Collapsing parent segments first — {@code normalize()} + * on the whole path — erases the component they cancel, symbolic link included, so + * {@code /base/link/..} reads as {@code /base} while the file system resolves it to the parent of + * the link's target. Only a walk sees the link before the segment that cancels it. + * + * <p>A path that does not exist yet is resolved as far as it exists and kept as it stands from + * there — an export destination is created on first write, and must be decided on before it + * exists. * * <p>A path whose existing part cannot be resolved is refused rather than accepted as it stands: a * dangling symbolic link inside a permitted directory would otherwise be taken for a child of it, * and would leave it as soon as its target is created. */ private static Path canonicalize(Path path) throws Refusal { - Path normalized = path.toAbsolutePath().normalize(); - Path existing = normalized; - while (existing != null && !Files.exists(existing, LinkOption.NOFOLLOW_LINKS)) { - existing = existing.getParent(); - } - if (existing == null) { - return normalized; - } - try { - return existing.toRealPath().resolve(existing.relativize(normalized)); - } catch (IOException e) { - throw new Refusal("path '" + normalized + "' cannot be resolved on the file system: " + e); + Path absolute = path.toAbsolutePath(); + Path resolved = absolute.getRoot(); + if (resolved == null) { + // an absolute path always has a root; without one there is nothing to decide on + throw new Refusal("path '" + path + "' cannot be made absolute"); + } + // once a component is missing, nothing below it can exist: the rest is kept as written + boolean belowWhatExists = false; + for (Path component : absolute) { + String name = component.toString(); + if (CURRENT_DIRECTORY.equals(name)) { + continue; + } + if (PARENT_DIRECTORY.equals(name)) { + Path parent = resolved.getParent(); + if (parent != null) { + resolved = parent; + } + continue; + } + Path candidate = resolved.resolve(component); + if (belowWhatExists || !Files.exists(candidate, LinkOption.NOFOLLOW_LINKS)) { + belowWhatExists = true; + resolved = candidate; + } else if (Files.isSymbolicLink(candidate)) { + try { + resolved = candidate.toRealPath(); + } catch (IOException e) { + throw new Refusal("path '" + candidate + "' cannot be resolved on the file system: " + e); + } + } else { + resolved = candidate; + } } + return resolved; } /** diff --git a/extensions/router/router-core/src/test/java/org/apache/unomi/router/core/route/FileEndpointContainmentTest.java b/extensions/router/router-core/src/test/java/org/apache/unomi/router/core/route/FileEndpointContainmentTest.java index 0d065f9db..2f749687a 100644 --- a/extensions/router/router-core/src/test/java/org/apache/unomi/router/core/route/FileEndpointContainmentTest.java +++ b/extensions/router/router-core/src/test/java/org/apache/unomi/router/core/route/FileEndpointContainmentTest.java @@ -143,6 +143,25 @@ public class FileEndpointContainmentTest { assertRouteRefused("symlink", "the source leaves the permitted base directory once symbolic links are resolved"); } + @Test + public void importRouteIsRefusedWhenSourceLeavesPermittedBaseDirThroughASymlinkAndAParentSegment() throws Exception { + File outsideChild = new File(arbitraryDir, "child"); + assertTrue("could not prepare the test fixture", outsideChild.mkdir()); + File link = new File(permittedImportDir, "link"); + try { + Files.createSymbolicLink(link.toPath(), outsideChild.toPath()); + } catch (IOException | UnsupportedOperationException e) { + Assume.assumeNoException("this file system does not support symbolic links", e); + } + + // the file system expands the link, then applies the parent segment to its target: the source + // is the arbitrary directory. Collapsing the segment first would read it as the base directory. + addImportRoutes(recurrentImport("symlink-parent", fileUri(new File(link, ".."), "?fileName=profiles.csv"))); + + assertRouteRefused("symlink-parent", + "a parent segment applies to the target of the link that precedes it, not to the link's own parent"); + } + @Test public void importRouteIsRefusedWhenSourceIsOutsidePermittedBaseDir() throws Exception { addImportRoutes(recurrentImport("arbitrary-dir", fileUri(arbitraryDir, "?fileName=profiles.csv")));
