Add compile-time validation of literal paths containing ".." (#329)

Addresses #327 This PR adds additional validation of literal path segments containing `..`. It throws compiletime error, suggesting canonical form of a path, when literal path is not canonical. Eg. "../foo/../bar" suggests "../bar" "foo/.." suggests removing literal --------- Co-authored-by: Li Haoyi <[email protected]>
com-lihaoyi · Nov 12, 2024 · 8d4bf10 · 8d4bf10
1 parent 1ddbb25
commit 8d4bf10
Show file tree

Hide file tree

Showing 3 changed files with 53 additions and 7 deletions.
diff --git a/os/src/Path.scala b/os/src/Path.scala
@@ -26,10 +26,17 @@ object PathChunk extends PathChunkMacros {
     val splitted = strNoTrailingSeps.split('/')
     splitted ++ Array.fill(trailingSeparatorsCount)("")
   }
-
+  private def reduceUps(in: Array[String]): List[String] =
+    in.foldLeft(List.empty[String]) { case (acc, x) =>
+      acc match {
+        case h :: t if h == ".." => x :: acc
+        case h :: t if x == ".." => t
+        case _ => x :: acc
+      }
+    }.reverse
   private[os] def segmentsFromStringLiteralValidation(literal: String): Array[String] = {
     val stringSegments = segmentsFromString(literal)
-    val validSegmnts = validLiteralSegments(stringSegments)
+    val validSegmnts = reduceUps(validLiteralSegments(stringSegments))
     val sanitizedLiteral = validSegmnts.mkString("/")
     if (validSegmnts.isEmpty) throw InvalidSegment(
       literal,

diff --git a/os/test/src-jvm/ExpectyIntegration.scala b/os/test/src-jvm/ExpectyIntegration.scala
@@ -14,19 +14,20 @@ object ExpectyIntegration extends TestSuite {
       }
       test("literals with [..]") {
         expect(rel / "src" / ".." == rel / "src" / os.up)
-        expect(root / "src/.." == root / "src" / os.up)
         expect(root / "src" / ".." == root / "src" / os.up)
         expect(root / "hello" / ".." / "world" == root / "hello" / os.up / "world")
         expect(root / "hello" / "../world" == root / "hello" / os.up / "world")
-        expect(root / "hello/../world" == root / "hello" / os.up / "world")
       }
       test("from issue") {
         expect(Seq(os.pwd / "foo") == Seq(os.pwd / "foo"))
         val path = os.Path("/") / "tmp" / "foo"
         expect(path.startsWith(os.Path("/") / "tmp"))
       }
       test("multiple args") {
-        expect(rel / "src" / ".." == rel / "src" / os.up, root / "src/.." == root / "src" / os.up)
+        expect(
+          rel / "src" / ".." == rel / "src" / os.up,
+          root / "src" / "../foo" == root / "src" / os.up / "foo"
+        )
       }
     }
   }

diff --git a/os/test/src/PathTests.scala b/os/test/src/PathTests.scala
@@ -23,14 +23,52 @@ object PathTests extends TestSuite {
       test("literals with [..]") {
 
         assert(rel / "src" / ".." == rel / "src" / os.up)
-        assert(root / "src/.." == root / "src" / os.up)
         assert(root / "src" / ".." == root / "src" / os.up)
         assert(root / "hello" / ".." / "world" == root / "hello" / os.up / "world")
         assert(root / "hello" / "../world" == root / "hello" / os.up / "world")
-        assert(root / "hello/../world" == root / "hello" / os.up / "world")
       }
 
       test("Compile errors") {
+
+        compileError("""root / "src/../foo"""").check("", nonCanonicalLiteral("src/../foo", "foo"))
+        compileError("""root / "hello/../world"""").check(
+          "",
+          nonCanonicalLiteral("hello/../world", "world")
+        )
+        compileError("""root / "src/../foo/bar"""").check(
+          "",
+          nonCanonicalLiteral("src/../foo/bar", "foo/bar")
+        )
+        compileError("""root / "src/../foo/bar/.."""").check(
+          "",
+          nonCanonicalLiteral("src/../foo/bar/..", "foo")
+        )
+        compileError("""root / "src/../foo/../bar/."""").check(
+          "",
+          nonCanonicalLiteral("src/../foo/../bar/.", "bar")
+        )
+        compileError("""root / "src/foo/./.."""").check(
+          "",
+          nonCanonicalLiteral("src/foo/./..", "src")
+        )
+        compileError("""root / "src/foo//./.."""").check(
+          "",
+          nonCanonicalLiteral("src/foo//./..", "src")
+        )
+
+        compileError("""root / "src/.."""").check("", removeLiteralErr("src/.."))
+        compileError("""root / "src/../foo/.."""").check("", removeLiteralErr("src/../foo/.."))
+        compileError("""root / "src/foo/../.."""").check("", removeLiteralErr("src/foo/../.."))
+        compileError("""root / "src/foo/./../.."""").check("", removeLiteralErr("src/foo/./../.."))
+        compileError("""root / "src/./foo/./../.."""").check(
+          "",
+          removeLiteralErr("src/./foo/./../..")
+        )
+        compileError("""root / "src///foo/./../.."""").check(
+          "",
+          removeLiteralErr("src///foo/./../..")
+        )
+
         compileError("""root / "/" """).check("", removeLiteralErr("/"))
         compileError("""root / "/ " """).check("", nonCanonicalLiteral("/ ", " "))
         compileError("""root / " /" """).check("", nonCanonicalLiteral(" /", " "))