From 80c7a54ad4d236d610139c04174b6d0ff46eee9a Mon Sep 17 00:00:00 2001
From: Jonathan Coates <git@squiddev.cc>
Date: Sun, 18 Aug 2024 12:49:33 +0100
Subject: [PATCH] Test path manipulation methods sanitise correctly

There's some nuance here with pattern vs non-pattern characters, so
useful to test for that.
---
 .../dan200/computercraft/core/apis/FSAPI.java |  2 +-
 .../core/filesystem/FileSystem.java           |  8 +++----
 .../resources/test-rom/spec/apis/fs_spec.lua  | 24 +++++++++++++++++++
 3 files changed, 28 insertions(+), 6 deletions(-)

diff --git a/projects/core/src/main/java/dan200/computercraft/core/apis/FSAPI.java b/projects/core/src/main/java/dan200/computercraft/core/apis/FSAPI.java
index 794d8b7c4..09c37ba9c 100644
--- a/projects/core/src/main/java/dan200/computercraft/core/apis/FSAPI.java
+++ b/projects/core/src/main/java/dan200/computercraft/core/apis/FSAPI.java
@@ -101,7 +101,7 @@ private FileSystem getFileSystem() {
      * }</pre>
      */
     @LuaFunction
-    public final String[] list(String path) throws LuaException {
+    public final List<String> list(String path) throws LuaException {
         try (var ignored = environment.time(Metrics.FS_OPS)) {
             return getFileSystem().list(path);
         } catch (FileSystemException e) {
diff --git a/projects/core/src/main/java/dan200/computercraft/core/filesystem/FileSystem.java b/projects/core/src/main/java/dan200/computercraft/core/filesystem/FileSystem.java
index 75a4ce1f8..509ee63f8 100644
--- a/projects/core/src/main/java/dan200/computercraft/core/filesystem/FileSystem.java
+++ b/projects/core/src/main/java/dan200/computercraft/core/filesystem/FileSystem.java
@@ -149,7 +149,7 @@ public synchronized BasicFileAttributes getAttributes(String path) throws FileSy
         return getMount(sanitizePath(path)).getAttributes(sanitizePath(path));
     }
 
-    public synchronized String[] list(String path) throws FileSystemException {
+    public synchronized List<String> list(String path) throws FileSystemException {
         path = sanitizePath(path);
         var mount = getMount(path);
 
@@ -165,10 +165,8 @@ public synchronized String[] list(String path) throws FileSystemException {
         }
 
         // Return list
-        var array = new String[list.size()];
-        list.toArray(array);
-        Arrays.sort(array);
-        return array;
+        list.sort(Comparator.naturalOrder());
+        return list;
     }
 
     public synchronized boolean exists(String path) throws FileSystemException {
diff --git a/projects/core/src/test/resources/test-rom/spec/apis/fs_spec.lua b/projects/core/src/test/resources/test-rom/spec/apis/fs_spec.lua
index e6af8d55c..084511173 100644
--- a/projects/core/src/test/resources/test-rom/spec/apis/fs_spec.lua
+++ b/projects/core/src/test/resources/test-rom/spec/apis/fs_spec.lua
@@ -158,6 +158,14 @@ describe("The fs library", function()
             expect(fs.combine("", "a")):eq("a")
             expect(fs.combine("a", "", "b", "c")):eq("a/b/c")
         end)
+
+        it("preserves pattern characters", function()
+            expect(fs.combine("foo*?")):eq("foo*?")
+        end)
+
+        it("sanitises paths", function()
+            expect(fs.combine("foo\":<>|")):eq("foo")
+        end)
     end)
 
     describe("fs.getName", function()
@@ -175,6 +183,14 @@ describe("The fs library", function()
         it("returns '..' for parent directories", function()
             expect(fs.getName("..")):eq("..")
         end)
+
+        it("preserves pattern characters", function()
+            expect(fs.getName("foo*?")):eq("foo*?")
+        end)
+
+        it("sanitises paths", function()
+            expect(fs.getName("foo\":<>|")):eq("foo")
+        end)
     end)
 
     describe("fs.getDir", function()
@@ -193,6 +209,14 @@ describe("The fs library", function()
             expect(fs.getDir("..")):eq("../..")
             expect(fs.getDir("../..")):eq("../../..")
         end)
+
+        it("preserves pattern characters", function()
+            expect(fs.getDir("foo*?/x")):eq("foo*?")
+        end)
+
+        it("sanitises paths", function()
+            expect(fs.getDir("foo\":<>|/x")):eq("foo")
+        end)
     end)
 
     describe("fs.getSize", function()