From fa5466f675a381861e20dcbb8c2e7ba33288f557 Mon Sep 17 00:00:00 2001 From: ostermine Date: Sat, 10 Oct 2026 15:54:28 +0300 Subject: [PATCH 1/3] fix zip archives paths and add zip files test - create_zip from a subdirectory wrote entry names with a leading '/', so the mounted archive looked empty - memory_ostream did not support tellp(), so create_zip into a memory device wrote invalid offsets (-1) to the archive - ZipFileDevice added missing parent directories for the first entry only - ZipFileDevice::exists returned false for the archive root - MemoryDevice::mkdirs recreated existing directories, dropping their content (mkdir("a/b") cleared "a"); overwriting a file duplicated its name in the directory listing Added dev/tests/zip_files.lua: builds archives with file.create_zip from memory device root, subdirectories and the real filesystem, mounts them and checks reading, listing and read-only access. memory_filesystem.lua got checks for the MemoryDevice issues. Closes #544 --- dev/tests/memory_filesystem.lua | 11 +++ dev/tests/zip_files.lua | 118 +++++++++++++++++++++++++++++++ src/io/devices/MemoryDevice.cpp | 13 ++++ src/io/devices/ZipFileDevice.cpp | 18 ++++- src/io/memory_ostream.hpp | 13 ++++ 5 files changed, 170 insertions(+), 3 deletions(-) create mode 100644 dev/tests/zip_files.lua diff --git a/dev/tests/memory_filesystem.lua b/dev/tests/memory_filesystem.lua index e246279a7..20d8e9700 100644 --- a/dev/tests/memory_filesystem.lua +++ b/dev/tests/memory_filesystem.lua @@ -41,6 +41,17 @@ assert(entries[1] == "memtest:dir/subdir/a.txt") assert(entries[2] == "memtest:dir/subdir/b.txt") assert(entries[3] == "memtest:dir/subdir/other") +debug.log("create sibling directory") +file.mkdir("memtest:dir/subdir/sibling") +assert(file.isfile("memtest:dir/subdir/a.txt")) +assert(file.isdir("memtest:dir/subdir/other")) +asserts.equals(4, #file.list("memtest:dir/subdir")) + +debug.log("overwrite file") +file.write("memtest:dir/subdir/a.txt", "overwritten") +asserts.equals("overwritten", file.read("memtest:dir/subdir/a.txt")) +asserts.equals(4, #file.list("memtest:dir/subdir")) + debug.log("remove tree") file.remove_tree("memtest:dir") assert(not file.isdir("memtest:dir")) diff --git a/dev/tests/zip_files.lua b/dev/tests/zip_files.lua new file mode 100644 index 000000000..ad5f4e7e0 --- /dev/null +++ b/dev/tests/zip_files.lua @@ -0,0 +1,118 @@ +local function sorted_list(path) + local entries = file.list(path) + table.sort(entries) + return entries +end + +local function check_list(expected, path) + local entries = sorted_list(path) + asserts.equals(#expected, #entries) + for i, name in ipairs(expected) do + asserts.equals(name, entries[i]) + end +end + +local function join(root, name) + if root:sub(-1) == ":" then + return root..name + end + return root.."/"..name +end + +local bytes = {0xDE, 0xAD, 0x00, 0xC0, 0xDE} + +local function create_tree(root) + file.mkdirs(join(root, "sub/deep")) + file.mkdir(join(root, "empty")) + file.write(join(root, "root.txt"), "root file") + file.write(join(root, "sub/a.txt"), "example, пример") + file.write_bytes(join(root, "sub/deep/binary"), bytes) +end + +local function check_tree(m) + debug.log("check directories") + assert(file.exists(m..":")) + assert(file.isdir(m..":")) + assert(file.isdir(m..":sub")) + assert(file.isdir(m..":sub/deep")) + assert(file.isdir(m..":empty")) + assert(not file.isfile(m..":sub")) + + debug.log("check files") + assert(file.isfile(m..":root.txt")) + assert(file.isfile(m..":sub/a.txt")) + assert(not file.isdir(m..":sub/a.txt")) + assert(not file.exists(m..":missing.txt")) + assert(not file.exists(m..":sub/missing.txt")) + + debug.log("read files") + asserts.equals("root file", file.read(m..":root.txt")) + asserts.equals("example, пример", file.read(m..":sub/a.txt")) + asserts.equals(#"root file", file.length(m..":root.txt")) + + local rbytes = file.read_bytes(m..":sub/deep/binary") + asserts.equals(#bytes, #rbytes) + for i, b in ipairs(bytes) do + asserts.equals(b, rbytes[i]) + end + + debug.log("read file with io_stream") + local stream = file.open(m..":root.txt", "r") + asserts.equals("root file", stream:read_line()) + stream:close() + + debug.log("list directories") + check_list({m..":empty", m..":root.txt", m..":sub"}, m..":") + check_list({m..":sub/a.txt", m..":sub/deep"}, m..":sub") + check_list({m..":sub/deep/binary"}, m..":sub/deep") + check_list({}, m..":empty") + + debug.log("check read-only") + assert(not file.is_writeable(m..":")) + assert(not pcall(file.write, m..":new.txt", "text")) + local ok, created = pcall(file.mkdir, m..":newdir") + assert(not (ok and created)) + assert(not file.exists(m..":newdir")) +end + +local function check_zip(src, zipfile) + debug.log("create zip "..zipfile.." from "..src) + file.create_zip(src, zipfile) + assert(file.isfile(zipfile)) + + debug.log("mount "..zipfile) + local m = file.mount(zipfile) + check_tree(m) + + debug.log("unmount "..m) + file.unmount(m) + assert(not pcall(file.read, m..":root.txt")) + assert(not pcall(file.unmount, m)) +end + +local mem = file.create_memory_device() +local out = file.create_memory_device() + +debug.log("zip memory device root") +create_tree(mem..":") +check_zip(mem..":", out..":root.zip") + +debug.log("zip memory device subdirectory") +create_tree(mem..":pack") +check_zip(mem..":pack", out..":pack.zip") + +debug.log("zip nested subdirectory") +create_tree(mem..":a/b/pack") +check_zip(mem..":a/b/pack", out..":nested.zip") + +debug.log("zip real filesystem directory") +if file.exists("config:ziptest") then + file.remove_tree("config:ziptest") +end +create_tree("config:ziptest/pack") +check_zip("config:ziptest/pack", "config:ziptest/pack.zip") +file.remove_tree("config:ziptest") + +debug.log("mount non-zip file") +file.write(out..":text.txt", "not an archive") +assert(not pcall(file.mount, out..":text.txt")) diff --git a/src/io/devices/MemoryDevice.cpp b/src/io/devices/MemoryDevice.cpp index f9f4760ee..8cf440c4e 100644 --- a/src/io/devices/MemoryDevice.cpp +++ b/src/io/devices/MemoryDevice.cpp @@ -180,6 +180,10 @@ std::unique_ptr io::MemoryDevice::list(std::string_view path } io::MemoryDevice::Dir* io::MemoryDevice::createDir(std::string path) { + const auto& found = nodes.find(path); + if (found != nodes.end()) { + return found->second.get_if(); + } io::path filePath = path; io::path parent = filePath.parent(); auto parentDir = getDir(parent.string()); @@ -195,6 +199,15 @@ io::MemoryDevice::Dir* io::MemoryDevice::createDir(std::string path) { io::MemoryDevice::Node* io::MemoryDevice::createFile( std::string path, util::Buffer&& content ) { + const auto& found = nodes.find(path); + if (found != nodes.end()) { + auto& node = found->second; + if (!node.holds_alternative()) { + return nullptr; + } + node.data = File {std::move(content)}; + return &node; + } io::path filePath = path; io::path parent = filePath.parent(); auto dir = getDir(parent.string()); diff --git a/src/io/devices/ZipFileDevice.cpp b/src/io/devices/ZipFileDevice.cpp index dce053887..2c52439ae 100644 --- a/src/io/devices/ZipFileDevice.cpp +++ b/src/io/devices/ZipFileDevice.cpp @@ -173,7 +173,13 @@ ZipFileDevice::ZipFileDevice( entries[entry.fileName] = std::move(entry); } - for (auto& [name, _] : entries) { + // Add missing parent directories entries + std::vector names; + names.reserve(entries.size()); + for (const auto& [name, _] : entries) { + names.push_back(name); + } + for (const auto& name : names) { io::path path = name; while (!(path = path.parent()).pathPart().empty()) { @@ -184,7 +190,6 @@ ZipFileDevice::ZipFileDevice( entry.isDirectory = true; entries[path.pathPart()] = entry; } - break; } for (auto& [_, entry] : entries) { @@ -253,6 +258,9 @@ file_time_type ZipFileDevice::lastWriteTime(std::string_view path) { } bool ZipFileDevice::exists(std::string_view path) { + if (path.empty()) { + return true; + } return entries.find(std::string(path)) != entries.end(); } @@ -441,7 +449,11 @@ static size_t write_zip( void io::write_zip(const path& folder, const path& file) { ByteBuilder central_dir; auto out = io::write(file); - size_t entries = write_zip(folder.pathPart(), folder, *out, central_dir); + auto root = folder.pathPart(); + if (!root.empty() && root.back() != '/') { + root += '/'; + } + size_t entries = write_zip(root, folder, *out, central_dir); size_t central_dir_offset = out->tellp(); out->write(reinterpret_cast(central_dir.data()), central_dir.size()); diff --git a/src/io/memory_ostream.hpp b/src/io/memory_ostream.hpp index 3ff14a994..329277aff 100644 --- a/src/io/memory_ostream.hpp +++ b/src/io/memory_ostream.hpp @@ -50,6 +50,19 @@ protected: return c; } + pos_type seekoff( + off_type off, + std::ios_base::seekdir way, + std::ios_base::openmode mode = std::ios_base::out + ) override { + // only position query (tellp) is supported + if (off == 0 && way == std::ios_base::cur && + (mode & std::ios_base::out)) { + return pos_type(off_type(size())); + } + return pos_type(off_type(-1)); + } + std::streamsize xsputn(const char* s, std::streamsize count) override { const std::streamsize avail = epptr() - pptr(); From a56158b78e3ffcb7be7af2951375adb5dc4ac2e0 Mon Sep 17 00:00:00 2001 From: ostermine Date: Sat, 10 Oct 2026 16:39:20 +0300 Subject: [PATCH 2/3] fix crash in create_zip for files without modification time MemoryDevice::lastWriteTime returns file_time_type::min(). Converting it to the system clock overflows, and on Windows std::localtime returns nullptr for the resulting time, which was dereferenced (access violation in zip_files test on MSVC). Unknown or out of range time is now written as 1980-01-01 00:00, the earliest MS-DOS date. --- src/io/devices/ZipFileDevice.cpp | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/src/io/devices/ZipFileDevice.cpp b/src/io/devices/ZipFileDevice.cpp index 2c52439ae..025c1c335 100644 --- a/src/io/devices/ZipFileDevice.cpp +++ b/src/io/devices/ZipFileDevice.cpp @@ -58,11 +58,22 @@ namespace { } uint32_t to_ms_dos_timestamp(const file_time_type& fileTime) { + // 1980-01-01 00:00, the earliest MS-DOS date; used for unknown time + // (devices without timestamps return file_time_type::min()) + constexpr uint32_t DOS_EPOCH = (0 << 9 | 1 << 5 | 1) << 16; + if (fileTime == file_time_type::min()) { + return DOS_EPOCH; + } auto timePoint = time_point_cast( fileTime - file_time_type::clock::now() + system_clock::now() ); std::time_t timeT = system_clock::to_time_t(timePoint); - std::tm tm = *std::localtime(&timeT); + // localtime returns nullptr for out of range time on Windows + const std::tm* tmPtr = std::localtime(&timeT); + if (tmPtr == nullptr || tmPtr->tm_year < 80) { + return DOS_EPOCH; + } + const std::tm& tm = *tmPtr; uint16_t date = (tm.tm_year - 80) << 9 | (tm.tm_mon + 1) << 5 | tm.tm_mday; uint16_t time = (tm.tm_hour << 11) | (tm.tm_min << 5) | (tm.tm_sec / 2); return (date << 16) | time; From 0efabc1883b70deb12cd2de3815e2391e950fceb Mon Sep 17 00:00:00 2001 From: ostermine Date: Sat, 10 Oct 2026 18:41:37 +0300 Subject: [PATCH 3/3] remove comments only relevant to the PR --- src/io/devices/ZipFileDevice.cpp | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/io/devices/ZipFileDevice.cpp b/src/io/devices/ZipFileDevice.cpp index 025c1c335..8420e1e1d 100644 --- a/src/io/devices/ZipFileDevice.cpp +++ b/src/io/devices/ZipFileDevice.cpp @@ -58,8 +58,7 @@ namespace { } uint32_t to_ms_dos_timestamp(const file_time_type& fileTime) { - // 1980-01-01 00:00, the earliest MS-DOS date; used for unknown time - // (devices without timestamps return file_time_type::min()) + // 1980-01-01 00:00, the earliest MS-DOS date constexpr uint32_t DOS_EPOCH = (0 << 9 | 1 << 5 | 1) << 16; if (fileTime == file_time_type::min()) { return DOS_EPOCH; @@ -68,7 +67,6 @@ namespace { fileTime - file_time_type::clock::now() + system_clock::now() ); std::time_t timeT = system_clock::to_time_t(timePoint); - // localtime returns nullptr for out of range time on Windows const std::tm* tmPtr = std::localtime(&timeT); if (tmPtr == nullptr || tmPtr->tm_year < 80) { return DOS_EPOCH;