diff --git a/doc/stub/fs.lua b/doc/stub/fs.lua index a9e76a352..41e1db8e1 100644 --- a/doc/stub/fs.lua +++ b/doc/stub/fs.lua @@ -19,6 +19,18 @@ function getFreeSpace(path) end function find(pattern) end function getDir(path) end +--- Returns true if a path is mounted to the parent filesystem. +-- +-- The root filesystem "/" is considered a mount, along with disk folders and +-- the rom folder. Other programs (such as network shares) can exstend this to +-- make other mount types by correctly assigning their return value for getDrive. +-- +-- @tparam string path The path to check. +-- @treturn boolean If the path is mounted, rather than a normal file/folder. +-- @throws If the path does not exist. +-- @see getDrive +function isDriveRoot(path) end + --- Get the capacity of the drive at the given path. -- -- This may be used in conjunction with @{getFreeSpace} to determine what diff --git a/src/main/resources/assets/computercraft/lua/bios.lua b/src/main/resources/assets/computercraft/lua/bios.lua index cef90b633..55688a4f2 100644 --- a/src/main/resources/assets/computercraft/lua/bios.lua +++ b/src/main/resources/assets/computercraft/lua/bios.lua @@ -797,6 +797,12 @@ function fs.complete(sPath, sLocation, bIncludeFiles, bIncludeDirs) return tEmpty end +function fs.isDriveRoot(sPath) + expect(1, sPath, "string") + -- Force the root directory to be a mount. + return fs.getDir(sPath) == ".." or fs.getDrive(sPath) ~= fs.getDrive(fs.getDir(sPath)) +end + -- Load APIs local bAPIError = false local tApis = fs.list("rom/apis") diff --git a/src/main/resources/assets/computercraft/lua/rom/programs/delete.lua b/src/main/resources/assets/computercraft/lua/rom/programs/delete.lua index 91f2e0c23..620cdd629 100644 --- a/src/main/resources/assets/computercraft/lua/rom/programs/delete.lua +++ b/src/main/resources/assets/computercraft/lua/rom/programs/delete.lua @@ -9,9 +9,18 @@ for i = 1, args.n do local files = fs.find(shell.resolve(args[i])) if #files > 0 then for _, file in ipairs(files) do - local ok, err = pcall(fs.delete, file) - if not ok then - printError((err:gsub("^pcall: ", ""))) + if fs.isReadOnly(file) then + printError("Cannot delete read-only file /" .. file) + elseif fs.isDriveRoot(file) then + printError("Cannot delete mount /" .. file) + if fs.isDir(file) then + print("To delete its contents run rm /" .. fs.combine(file, "*")) + end + else + local ok, err = pcall(fs.delete, file) + if not ok then + printError((err:gsub("^pcall: ", ""))) + end end end else diff --git a/src/main/resources/assets/computercraft/lua/rom/programs/move.lua b/src/main/resources/assets/computercraft/lua/rom/programs/move.lua index 3273b1ba9..254592200 100644 --- a/src/main/resources/assets/computercraft/lua/rom/programs/move.lua +++ b/src/main/resources/assets/computercraft/lua/rom/programs/move.lua @@ -7,12 +7,35 @@ end local sSource = shell.resolve(tArgs[1]) local sDest = shell.resolve(tArgs[2]) local tFiles = fs.find(sSource) + +local function sanity_checks(source, dest) + if fs.exists(dest) then + printError("Destination exists") + return false + elseif fs.isReadOnly(dest) then + printError("Destination is read-only") + return false + elseif fs.isDriveRoot(source) then + printError("Cannot move mount /" .. source) + return false + elseif fs.isReadOnly(source) then + printError("Cannot move read-only file /" .. source) + return false + end + return true +end + if #tFiles > 0 then for _, sFile in ipairs(tFiles) do if fs.isDir(sDest) then - fs.move(sFile, fs.combine(sDest, fs.getName(sFile))) + local dest = fs.combine(sDest, fs.getName(sFile)) + if sanity_checks(sFile, dest) then + fs.move(sFile, dest) + end elseif #tFiles == 1 then - fs.move(sFile, sDest) + if sanity_checks(sFile, sDest) then + fs.move(sFile, sDest) + end else printError("Cannot overwrite file multiple times") return diff --git a/src/main/resources/assets/computercraft/lua/rom/programs/rename.lua b/src/main/resources/assets/computercraft/lua/rom/programs/rename.lua index a90c1f8ad..8b491abcd 100644 --- a/src/main/resources/assets/computercraft/lua/rom/programs/rename.lua +++ b/src/main/resources/assets/computercraft/lua/rom/programs/rename.lua @@ -10,6 +10,12 @@ local sDest = shell.resolve(tArgs[2]) if not fs.exists(sSource) then printError("No matching files") return +elseif fs.isDriveRoot(sSource) then + printError("Can't rename mounts") + return +elseif fs.isReadOnly(sSource) then + printError("Source is read-only") + return elseif fs.exists(sDest) then printError("Destination exists") return diff --git a/src/test/resources/test-rom/spec/apis/fs_spec.lua b/src/test/resources/test-rom/spec/apis/fs_spec.lua index 5288aa9c8..fd16599de 100644 --- a/src/test/resources/test-rom/spec/apis/fs_spec.lua +++ b/src/test/resources/test-rom/spec/apis/fs_spec.lua @@ -12,6 +12,21 @@ describe("The fs library", function() end) end) + describe("fs.isDriveRoot", function() + it("validates arguments", function() + fs.isDriveRoot("") + + expect.error(fs.isDriveRoot, nil):eq("bad argument #1 (expected string, got nil)") + end) + + it("correctly identifies drive roots", function() + expect(fs.isDriveRoot("/rom")):eq(true) + expect(fs.isDriveRoot("/")):eq(true) + expect(fs.isDriveRoot("/rom/startup.lua")):eq(false) + expect(fs.isDriveRoot("/rom/programs/delete.lua")):eq(false) + end) + end) + describe("fs.list", function() it("fails on files", function() expect.error(fs.list, "rom/startup.lua"):eq("/rom/startup.lua: Not a directory") diff --git a/src/test/resources/test-rom/spec/programs/delete_spec.lua b/src/test/resources/test-rom/spec/programs/delete_spec.lua index 1eec95cc4..18a01c5c0 100644 --- a/src/test/resources/test-rom/spec/programs/delete_spec.lua +++ b/src/test/resources/test-rom/spec/programs/delete_spec.lua @@ -42,7 +42,15 @@ describe("The rm program", function() it("errors when trying to delete a read-only file", function() expect(capture(stub, "rm /rom/startup.lua")) - :matches { ok = true, output = "", error = "/rom/startup.lua: Access denied\n" } + :matches { ok = true, output = "", error = "Cannot delete read-only file /rom/startup.lua\n" } + end) + + it("errors when trying to delete the root mount", function() + expect(capture(stub, "rm /")):matches { + ok = true, + output = "To delete its contents run rm /*\n", + error = "Cannot delete mount /\n", + } end) it("errors when a glob fails to match", function() diff --git a/src/test/resources/test-rom/spec/programs/move_spec.lua b/src/test/resources/test-rom/spec/programs/move_spec.lua index de1a36569..664010118 100644 --- a/src/test/resources/test-rom/spec/programs/move_spec.lua +++ b/src/test/resources/test-rom/spec/programs/move_spec.lua @@ -1,11 +1,13 @@ local capture = require "test_helpers".capture_program describe("The move program", function() + local function cleanup() fs.delete("/test-files/move") end local function touch(file) io.open(file, "w"):close() end it("move a file", function() + cleanup() touch("/test-files/move/a.txt") shell.run("move /test-files/move/a.txt /test-files/move/b.txt") @@ -14,11 +16,57 @@ describe("The move program", function() expect(fs.exists("/test-files/move/b.txt")):eq(true) end) - it("try to move a not existing file", function() + it("moves a file to a directory", function() + cleanup() + touch("/test-files/move/a.txt") + fs.makeDir("/test-files/move/a") + + expect(capture(stub, "move /test-files/move/a.txt /test-files/move/a")) + :matches { ok = true } + + expect(fs.exists("/test-files/move/a.txt")):eq(false) + expect(fs.exists("/test-files/move/a/a.txt")):eq(true) + end) + + it("fails when moving a file which doesn't exist", function() expect(capture(stub, "move nothing destination")) :matches { ok = true, output = "", error = "No matching files\n" } end) + it("fails when overwriting an existing file", function() + cleanup() + touch("/test-files/move/a.txt") + + expect(capture(stub, "move /test-files/move/a.txt /test-files/move/a.txt")) + :matches { ok = true, output = "", error = "Destination exists\n" } + end) + + it("fails when moving to read-only locations", function() + cleanup() + touch("/test-files/move/a.txt") + + expect(capture(stub, "move /test-files/move/a.txt /rom/test.txt")) + :matches { ok = true, output = "", error = "Destination is read-only\n" } + end) + + it("fails when moving from read-only locations", function() + expect(capture(stub, "move /rom/startup.lua /test-files/move/not-exist.txt")) + :matches { ok = true, output = "", error = "Cannot move read-only file /rom/startup.lua\n" } + end) + + it("fails when moving mounts", function() + expect(capture(stub, "move /rom /test-files/move/rom")) + :matches { ok = true, output = "", error = "Cannot move mount /rom\n" } + end) + + it("fails when moving a file multiple times", function() + cleanup() + touch("/test-files/move/a.txt") + touch("/test-files/move/b.txt") + expect(capture(stub, "move /test-files/move/*.txt /test-files/move/c.txt")) + :matches { ok = true, output = "", error = "Cannot overwrite file multiple times\n" } + end) + it("displays the usage with no arguments", function() expect(capture(stub, "move")) :matches { ok = true, output = "Usage: mv \n", error = "" } diff --git a/src/test/resources/test-rom/spec/programs/rename_spec.lua b/src/test/resources/test-rom/spec/programs/rename_spec.lua index 2ab955dab..437e940d3 100644 --- a/src/test/resources/test-rom/spec/programs/rename_spec.lua +++ b/src/test/resources/test-rom/spec/programs/rename_spec.lua @@ -26,13 +26,23 @@ describe("The rename program", function() :matches { ok = true, output = "", error = "Destination exists\n" } end) - it("fails when copying to read-only locations", function() + it("fails when renaming to read-only locations", function() touch("/test-files/rename/d.txt") expect(capture(stub, "rename /test-files/rename/d.txt /rom/test.txt")) :matches { ok = true, output = "", error = "Destination is read-only\n" } end) + it("fails when renaming from read-only locations", function() + expect(capture(stub, "rename /rom/startup.lua /test-files/rename/d.txt")) + :matches { ok = true, output = "", error = "Source is read-only\n" } + end) + + it("fails when renaming mounts", function() + expect(capture(stub, "rename /rom /test-files/rename/rom")) + :matches { ok = true, output = "", error = "Can't rename mounts\n" } + end) + it("displays the usage when given no arguments", function() expect(capture(stub, "rename")) :matches { ok = true, output = "Usage: rename \n", error = "" }