diff --git a/.github/workflows/linux.yml b/.github/workflows/linux.yml index 392bbfc6f9..cff3642f5e 100644 --- a/.github/workflows/linux.yml +++ b/.github/workflows/linux.yml @@ -86,7 +86,7 @@ jobs: - name: Install deps run: | source ./util/ci/common.sh - install_linux_deps clang-7 valgrind + install_linux_deps clang-7 llvm - name: Build run: | @@ -94,15 +94,12 @@ jobs: env: CC: clang-7 CXX: clang++-7 + CMAKE_FLAGS: '-DCMAKE_C_FLAGS="-fsanitize=address" -DCMAKE_CXX_FLAGS="-fsanitize=address"' - name: Unittest run: | ./bin/minetest --run-unittests - - name: Valgrind - run: | - valgrind --leak-check=full --leak-check-heuristics=all --undef-value-errors=no --error-exitcode=9 ./bin/minetest --run-unittests - # Current clang version clang_14: runs-on: ubuntu-22.04 diff --git a/doc/lua_api.md b/doc/lua_api.md index ea1bc3c34d..bb213b7c5c 100644 --- a/doc/lua_api.md +++ b/doc/lua_api.md @@ -5874,8 +5874,11 @@ handler. * `minetest.add_entity(pos, name, [staticdata])`: Spawn Lua-defined entity at position. * Returns `ObjectRef`, or `nil` if failed + * Entities with `static_save = true` can be added also + to unloaded and non-generated blocks. * `minetest.add_item(pos, item)`: Spawn item * Returns `ObjectRef`, or `nil` if failed + * Items can be added also to unloaded and non-generated blocks. * `minetest.get_player_by_name(name)`: Get an `ObjectRef` to a player * `minetest.get_objects_inside_radius(pos, radius)`: returns a list of ObjectRefs. diff --git a/src/client/clientmedia.cpp b/src/client/clientmedia.cpp index f78fe9e353..9d3e9fedc0 100644 --- a/src/client/clientmedia.cpp +++ b/src/client/clientmedia.cpp @@ -29,6 +29,7 @@ with this program; if not, write to the Free Software Foundation, Inc., #include "util/serialize.h" #include "util/sha1.h" #include "util/string.h" +#include static std::string getMediaCacheDir() { @@ -41,7 +42,16 @@ bool clientMediaUpdateCache(const std::string &raw_hash, const std::string &file std::string sha1_hex = hex_encode(raw_hash); if (!media_cache.exists(sha1_hex)) return media_cache.update(sha1_hex, filedata); - return true; + return false; +} + +bool clientMediaUpdateCacheCopy(const std::string &raw_hash, const std::string &path) +{ + FileCache media_cache(getMediaCacheDir()); + std::string sha1_hex = hex_encode(raw_hash); + if (!media_cache.exists(sha1_hex)) + return media_cache.updateCopyFile(sha1_hex, path); + return false; } /* @@ -189,10 +199,6 @@ void ClientMediaDownloader::initialStep(Client *client) assert(m_uncached_received_count == 0); - // Create the media cache dir if we are likely to write to it - if (m_uncached_count != 0) - createCacheDirs(); - // If we found all files in the cache, report this fact to the server. // If the server reported no remote servers, immediately start // conventional transfers. Note: if cURL support is not compiled in, @@ -511,18 +517,6 @@ IClientMediaDownloader::IClientMediaDownloader(): { } -void IClientMediaDownloader::createCacheDirs() -{ - if (!m_write_to_cache) - return; - - std::string path = getMediaCacheDir(); - if (!fs::CreateAllDirs(path)) { - errorstream << "Client: Could not create media cache directory: " - << path << std::endl; - } -} - bool IClientMediaDownloader::tryLoadFromCache(const std::string &name, const std::string &sha1, Client *client) { @@ -726,8 +720,6 @@ void SingleMediaDownloader::initialStep(Client *client) if (isDone()) return; - createCacheDirs(); - // If the server reported no remote servers, immediately fall back to // conventional transfer. if (!USE_CURL || m_remotes.empty()) { diff --git a/src/client/clientmedia.h b/src/client/clientmedia.h index c297d737fc..27af86e4b0 100644 --- a/src/client/clientmedia.h +++ b/src/client/clientmedia.h @@ -22,7 +22,6 @@ with this program; if not, write to the Free Software Foundation, Inc., #include "irrlichttypes.h" #include "filecache.h" #include "util/basic_macros.h" -#include #include #include #include @@ -35,10 +34,15 @@ struct HTTPFetchResult; #define MTHASHSET_FILE_NAME "index.mth" // Store file into media cache (unless it exists already) -// Validating the hash is responsibility of the caller +// Caller should check the hash. +// return true if something was updated bool clientMediaUpdateCache(const std::string &raw_hash, const std::string &filedata); +// Copy file on disk(!) into media cache (unless it exists already) +bool clientMediaUpdateCacheCopy(const std::string &raw_hash, + const std::string &path); + // more of a base class than an interface but this name was most convenient... class IClientMediaDownloader { @@ -81,8 +85,6 @@ protected: virtual bool loadMedia(Client *client, const std::string &data, const std::string &name) = 0; - void createCacheDirs(); - bool tryLoadFromCache(const std::string &name, const std::string &sha1, Client *client); diff --git a/src/client/filecache.cpp b/src/client/filecache.cpp index 46bbe40595..f3d7bf34ad 100644 --- a/src/client/filecache.cpp +++ b/src/client/filecache.cpp @@ -28,6 +28,14 @@ with this program; if not, write to the Free Software Foundation, Inc., #include #include +void FileCache::createDir() +{ + if (!fs::CreateAllDirs(m_dir)) { + errorstream << "Could not create cache directory: " + << m_dir << std::endl; + } +} + bool FileCache::loadByPath(const std::string &path, std::ostream &os) { std::ifstream fis(path.c_str(), std::ios_base::binary); @@ -40,8 +48,8 @@ bool FileCache::loadByPath(const std::string &path, std::ostream &os) bool bad = false; for(;;){ - char buf[1024]; - fis.read(buf, 1024); + char buf[4096]; + fis.read(buf, sizeof(buf)); std::streamsize len = fis.gcount(); os.write(buf, len); if(fis.eof()) @@ -61,6 +69,7 @@ bool FileCache::loadByPath(const std::string &path, std::ostream &os) bool FileCache::updateByPath(const std::string &path, const std::string &data) { + createDir(); std::ofstream file(path.c_str(), std::ios_base::binary | std::ios_base::trunc); @@ -95,3 +104,11 @@ bool FileCache::exists(const std::string &name) std::ifstream fis(path.c_str(), std::ios_base::binary); return fis.good(); } + +bool FileCache::updateCopyFile(const std::string &name, const std::string &src_path) +{ + std::string path = m_dir + DIR_DELIM + name; + + createDir(); + return fs::CopyFileContents(src_path, path); +} diff --git a/src/client/filecache.h b/src/client/filecache.h index ea6afc4b2a..c8d5a781e7 100644 --- a/src/client/filecache.h +++ b/src/client/filecache.h @@ -35,9 +35,13 @@ public: bool load(const std::string &name, std::ostream &os); bool exists(const std::string &name); + // Copy another file on disk into the cache + bool updateCopyFile(const std::string &name, const std::string &src_path); + private: std::string m_dir; + void createDir(); bool loadByPath(const std::string &path, std::ostream &os); bool updateByPath(const std::string &path, const std::string &data); }; diff --git a/src/client/game.cpp b/src/client/game.cpp index 546dfa7ec3..b0890a8448 100644 --- a/src/client/game.cpp +++ b/src/client/game.cpp @@ -33,6 +33,7 @@ with this program; if not, write to the Free Software Foundation, Inc., #include "client/mapblock_mesh.h" #include "client/sound.h" #include "clientmap.h" +#include "clientmedia.h" // For clientMediaUpdateCacheCopy #include "clouds.h" #include "config.h" #include "content_cao.h" @@ -769,6 +770,7 @@ protected: bool initSound(); bool createSingleplayerServer(const std::string &map_dir, const SubgameSpec &gamespec, u16 port); + void copyServerClientCache(); // Client creation bool createClient(const GameStartData &start_data); @@ -1453,9 +1455,31 @@ bool Game::createSingleplayerServer(const std::string &map_dir, false, nullptr, error_message); server->start(); + copyServerClientCache(); + return true; } +void Game::copyServerClientCache() +{ + // It would be possible to let the client directly read the media files + // from where the server knows they are. But aside from being more complicated + // it would also *not* fill the media cache and cause slower joining of + // remote servers. + // (Imagine that you launch a game once locally and then connect to a server.) + + assert(server); + auto map = server->getMediaList(); + u32 n = 0; + for (auto &it : map) { + assert(it.first.size() == 20); // SHA1 + if (clientMediaUpdateCacheCopy(it.first, it.second)) + n++; + } + infostream << "Copied " << n << " files directly from server to client cache" + << std::endl; +} + bool Game::createClient(const GameStartData &start_data) { showOverlayMessage(N_("Creating client..."), 0, 10); diff --git a/src/filesys.cpp b/src/filesys.cpp index 4fc51a5da9..5ea71b8fdc 100644 --- a/src/filesys.cpp +++ b/src/filesys.cpp @@ -33,6 +33,13 @@ with this program; if not, write to the Free Software Foundation, Inc., #include #include #endif +#ifdef __linux__ +#include +#include +#ifndef FICLONE +#define FICLONE _IOW(0x94, 9, int) +#endif +#endif namespace fs { @@ -216,6 +223,31 @@ std::string CreateTempFile() return path; } +bool CopyFileContents(const std::string &source, const std::string &target) +{ + BOOL ok = CopyFileEx(source.c_str(), target.c_str(), nullptr, nullptr, + nullptr, COPY_FILE_ALLOW_DECRYPTED_DESTINATION); + if (!ok) { + errorstream << "copying " << source << " to " << target + << " failed: " << GetLastError() << std::endl; + return false; + } + + // docs: "File attributes for the existing file are copied to the new file." + // This is not our intention so get rid of unwanted attributes: + DWORD attr = GetFileAttributes(target.c_str()); + if (attr == INVALID_FILE_ATTRIBUTES) { + errorstream << target << ": file disappeared after copy" << std::endl; + return false; + } + attr &= ~(FILE_ATTRIBUTE_READONLY | FILE_ATTRIBUTE_HIDDEN); + SetFileAttributes(target.c_str(), attr); + + tracestream << "copied " << source << " to " << target + << " using CopyFileEx" << std::endl; + return true; +} + #else /********* @@ -414,6 +446,101 @@ std::string CreateTempFile() return path; } +namespace { + struct FileDeleter { + void operator()(FILE *stream) { + fclose(stream); + } + }; + + typedef std::unique_ptr FileUniquePtr; +} + +bool CopyFileContents(const std::string &source, const std::string &target) +{ + FileUniquePtr sourcefile, targetfile; + +#ifdef __linux__ + // Try to clone using Copy-on-Write (CoW). This is instant but supported + // only by some filesystems. + + int srcfd, tgtfd; + srcfd = open(source.c_str(), O_RDONLY); + if (srcfd == -1) { + errorstream << source << ": can't open for reading: " + << strerror(errno) << std::endl; + return false; + } + tgtfd = open(target.c_str(), O_WRONLY | O_CREAT | O_TRUNC, 0644); + if (tgtfd == -1) { + errorstream << target << ": can't open for writing: " + << strerror(errno) << std::endl; + close(srcfd); + return false; + } + + if (ioctl(tgtfd, FICLONE, srcfd) == 0) { + tracestream << "copied " << source << " to " << target + << " using FICLONE" << std::endl; + close(srcfd); + close(tgtfd); + return true; + } + + // fallback to normal copy, but no need to reopen the files + sourcefile.reset(fdopen(srcfd, "rb")); + targetfile.reset(fdopen(tgtfd, "wb")); + goto fallback; + +#endif + + sourcefile.reset(fopen(source.c_str(), "rb")); + targetfile.reset(fopen(target.c_str(), "wb")); + +fallback: + + if (!sourcefile) { + errorstream << source << ": can't open for reading: " + << strerror(errno) << std::endl; + return false; + } + if (!targetfile) { + errorstream << target << ": can't open for writing: " + << strerror(errno) << std::endl; + return false; + } + + size_t total = 0; + bool done = false; + char readbuffer[BUFSIZ]; + while (!done) { + size_t readbytes = fread(readbuffer, 1, + sizeof(readbuffer), sourcefile.get()); + total += readbytes; + if (ferror(sourcefile.get())) { + errorstream << source << ": IO error: " + << strerror(errno) << std::endl; + return false; + } + if (readbytes > 0) + fwrite(readbuffer, 1, readbytes, targetfile.get()); + if (feof(sourcefile.get())) { + // flush destination file to catch write errors (e.g. disk full) + fflush(targetfile.get()); + done = true; + } + if (ferror(targetfile.get())) { + errorstream << target << ": IO error: " + << strerror(errno) << std::endl; + return false; + } + } + tracestream << "copied " << total << " bytes from " + << source << " to " << target << std::endl; + + return true; +} + #endif /**************************** @@ -488,60 +615,6 @@ bool CreateAllDirs(const std::string &path) return true; } -bool CopyFileContents(const std::string &source, const std::string &target) -{ - FILE *sourcefile = fopen(source.c_str(), "rb"); - if(sourcefile == NULL){ - errorstream< 0){ - fwrite(readbuffer, 1, readbytes, targetfile); - } - if(feof(sourcefile) || ferror(sourcefile)){ - // flush destination file to catch write errors - // (e.g. disk full) - fflush(targetfile); - done = true; - } - if(ferror(targetfile)){ - errorstream< Server::getMediaList() +{ + MutexAutoLock env_lock(m_env_mutex); + + std::unordered_map ret; + for (auto &it : m_media) { + if (it.second.no_announce) + continue; + ret.emplace(base64_decode(it.second.sha1_digest), it.second.path); + } + return ret; +} + ModStorageDatabase *Server::openModStorageDatabase(const std::string &world_path) { std::string world_mt_path = world_path + DIR_DELIM + "world.mt"; diff --git a/src/server.h b/src/server.h index 5a6b6b7cdc..a7fc126b6c 100644 --- a/src/server.h +++ b/src/server.h @@ -388,6 +388,10 @@ public: // Get or load translations for a language Translations *getTranslationLanguage(const std::string &lang_code); + // Returns all media files the server knows about + // map key = binary sha1, map value = file path + std::unordered_map getMediaList(); + static ModStorageDatabase *openModStorageDatabase(const std::string &world_path); static ModStorageDatabase *openModStorageDatabase(const std::string &backend, diff --git a/src/serverenvironment.cpp b/src/serverenvironment.cpp index 7b188f60fc..753899fb83 100644 --- a/src/serverenvironment.cpp +++ b/src/serverenvironment.cpp @@ -1044,7 +1044,8 @@ void ServerEnvironment::activateBlock(MapBlock *block, u32 additional_dtime) <m_static_objects.clearStored(); // do not set changed flag to avoid unnecessary mapblock writes } @@ -1892,11 +1893,6 @@ u16 ServerEnvironment::addActiveObjectRaw(std::unique_ptr ob return 0; } - // Register reference in scripting api (must be done before post-init) - m_script->addObjectReference(object); - // Post-initialize object - object->addedToEnvironment(dtime_s); - // Add static data to block if (object->isStaticAllowed()) { // Add static object to active static list of the block @@ -1915,12 +1911,20 @@ u16 ServerEnvironment::addActiveObjectRaw(std::unique_ptr ob MOD_REASON_ADD_ACTIVE_OBJECT_RAW); } else { v3s16 p = floatToInt(objectpos, BS); - errorstream<<"ServerEnvironment::addActiveObjectRaw(): " - <<"could not emerge block for storing id="<getId() - <<" statically (pos="<getId()); + return 0; } } + // Register reference in scripting api (must be done before post-init) + m_script->addObjectReference(object); + // Post-initialize object + object->addedToEnvironment(dtime_s); + return object->getId(); } diff --git a/src/unittest/test_datastructures.cpp b/src/unittest/test_datastructures.cpp index a7e24b989a..cd3755a1fd 100644 --- a/src/unittest/test_datastructures.cpp +++ b/src/unittest/test_datastructures.cpp @@ -108,6 +108,8 @@ void TestDataStructures::testMap1() break; } UASSERT(once); + + map.clear(); // ASan complains about stack-use-after-scope otherwise } void TestDataStructures::testMap2() @@ -121,6 +123,8 @@ void TestDataStructures::testMap2() UASSERT(t0.deleted); UASSERT(!t1.copied); UASSERT(!t1.deleted); + + map.clear(); } void TestDataStructures::testMap3() diff --git a/src/unittest/test_filesys.cpp b/src/unittest/test_filesys.cpp index dde541a7cc..3023713615 100644 --- a/src/unittest/test_filesys.cpp +++ b/src/unittest/test_filesys.cpp @@ -40,6 +40,7 @@ public: void testRemoveLastPathComponentWithTrailingDelimiter(); void testRemoveRelativePathComponent(); void testSafeWriteToFile(); + void testCopyFileContents(); }; static TestFileSys g_test_instance; @@ -52,6 +53,7 @@ void TestFileSys::runTests(IGameDef *gamedef) TEST(testRemoveLastPathComponentWithTrailingDelimiter); TEST(testRemoveRelativePathComponent); TEST(testSafeWriteToFile); + TEST(testCopyFileContents); } //////////////////////////////////////////////////////////////////////////////// @@ -59,7 +61,7 @@ void TestFileSys::runTests(IGameDef *gamedef) // adjusts a POSIX path to system-specific conventions // -> changes '/' to DIR_DELIM // -> absolute paths start with "C:\\" on windows -std::string p(std::string path) +static std::string p(std::string path) { for (size_t i = 0; i < path.size(); ++i) { if (path[i] == '/') { @@ -275,5 +277,37 @@ void TestFileSys::testSafeWriteToFile() UASSERT(fs::PathExists(dest_path)); std::string contents_actual; UASSERT(fs::ReadFile(dest_path, contents_actual)); - UASSERT(contents_actual == test_data); + UASSERTEQ(auto, contents_actual, test_data); +} + +void TestFileSys::testCopyFileContents() +{ + const auto dir_path = getTestTempDirectory(); + const auto file1 = dir_path + DIR_DELIM "src", file2 = dir_path + DIR_DELIM "dst"; + const std::string test_data("hello\0world", 11); + + // error case + UASSERT(!fs::CopyFileContents(file1, "somewhere")); + + { + std::ofstream ofs(file1); + ofs << test_data; + } + + // normal case + UASSERT(fs::CopyFileContents(file1, file2)); + std::string contents_actual; + UASSERT(fs::ReadFile(file2, contents_actual)); + UASSERTEQ(auto, contents_actual, test_data); + + // should overwrite and truncate + { + std::ofstream ofs(file2); + for (int i = 0; i < 10; i++) + ofs << "OH MY GAH"; + } + UASSERT(fs::CopyFileContents(file1, file2)); + contents_actual.clear(); + UASSERT(fs::ReadFile(file2, contents_actual)); + UASSERTEQ(auto, contents_actual, test_data); } diff --git a/util/wireshark/minetest.lua b/util/wireshark/minetest.lua index 93e4f03872..837ea29639 100644 --- a/util/wireshark/minetest.lua +++ b/util/wireshark/minetest.lua @@ -1280,13 +1280,14 @@ do function p_minetest.dissector(buffer, pinfo, tree) - -- Add Minetest tree item and verify the ID + -- Defer if payload doesn't have Minetest's magic number + if buffer(0,4):uint() ~= minetest_id then + return false + end + + -- Add Minetest tree item local t = tree:add(p_minetest, buffer(0,8)) t:add(f_id, buffer(0,4)) - if buffer(0,4):uint() ~= minetest_id then - t:add_expert_info(PI_UNDECODED, PI_WARN, "Invalid ID, this is not a Minetest packet") - return - end -- ID is valid, so replace packet's shown protocol pinfo.cols.protocol = "Minetest" @@ -1339,12 +1340,11 @@ do end pinfo.cols.info:append(" (" .. reliability_info .. ")") + return true end - -- FIXME Is there a way to let the dissector table check if the first payload bytes are 0x4f457403? - DissectorTable.get("udp.port"):add(30000, p_minetest) - DissectorTable.get("udp.port"):add(30001, p_minetest) + p_minetest:register_heuristic("udp", p_minetest.dissector) end