Merge branch 'main' into empty-dir-zip

com-lihaoyi · Nov 14, 2024 · 88c7a22 · 88c7a22
2 parents a02fbab + 8d4bf10
commit 88c7a22
Show file tree

Hide file tree

Showing 4 changed files with 77 additions and 30 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(" /", " "))

diff --git a/os/test/src/ZipOpTests.scala b/os/test/src/ZipOpTests.scala
@@ -10,29 +10,30 @@ import java.util.zip.{ZipEntry, ZipOutputStream}
 object ZipOpTests extends TestSuite {
 
   def tests = Tests {
-    test("level") - prep { wd =>
-      val zipsForLevel = for (i <- Range.inclusive(0, 9)) yield {
-        os.write.over(wd / "File.txt", Range(0, 1000).map(x => x.toString * x))
-        os.zip(
-          dest = wd / s"archive-$i.zip",
-          sources = Seq(
-            wd / "File.txt",
-            wd / "folder1"
-          ),
-          compressionLevel = i
-        )
-      }
-
-      // We can't compare every level because compression isn't fully monotonic,
-      // but we compare some arbitrary levels just to sanity check things
-
-      // Uncompressed zip is definitely bigger than first level of compression
-      assert(os.size(zipsForLevel(0)) > os.size(zipsForLevel(1)))
-      // First level of compression is bigger than middle compression
-      assert(os.size(zipsForLevel(1)) > os.size(zipsForLevel(5)))
-      // Middle compression is bigger than best compression
-      assert(os.size(zipsForLevel(5)) > os.size(zipsForLevel(9)))
-    }
+    // This test seems really flaky for some reason
+    // test("level") - prep { wd =>
+    //   val zipsForLevel = for (i <- Range.inclusive(0, 9)) yield {
+    //     os.write.over(wd / "File.txt", Range(0, 1000).map(x => x.toString * x))
+    //     os.zip(
+    //       dest = wd / s"archive-$i.zip",
+    //       sources = Seq(
+    //         wd / "File.txt",
+    //         wd / "folder1"
+    //       ),
+    //       compressionLevel = i
+    //     )
+    //   }
+
+    //   // We can't compare every level because compression isn't fully monotonic,
+    //   // but we compare some arbitrary levels just to sanity check things
+
+    //   // Uncompressed zip is definitely bigger than first level of compression
+    //   assert(os.size(zipsForLevel(0)) > os.size(zipsForLevel(1)))
+    //   // First level of compression is bigger than middle compression
+    //   assert(os.size(zipsForLevel(1)) > os.size(zipsForLevel(5)))
+    //   // Middle compression is bigger than best compression
+    //   assert(os.size(zipsForLevel(5)) > os.size(zipsForLevel(9)))
+    // }
     test("renaming") - prep { wd =>
       val zipFileName = "zip-file-test.zip"
       val zipFile1: os.Path = os.zip(