From 3d6b7d3b75f91cd6ddbc2e1e8323651f6aa1d87b Mon Sep 17 00:00:00 2001 From: Ancurio Date: Mon, 27 Sep 2021 18:23:16 +0200 Subject: [PATCH] Revert "FileSystem: Allow ::openReadRaw() to break out of game directory" This reverts commit d45a40022763eb3cc52ab419df7140902bf3c0e3. Causes memory corruption in its current state. --- binding-mri/filesystem-binding.cpp | 7 +++-- binding-mruby/binding-mruby.cpp | 8 +++--- src/filesystem.cpp | 45 +++++++++++++++--------------- src/filesystem.h | 4 ++- src/font.cpp | 7 +++-- 5 files changed, 39 insertions(+), 32 deletions(-) diff --git a/binding-mri/filesystem-binding.cpp b/binding-mri/filesystem-binding.cpp index b71e042..a9449af 100644 --- a/binding-mri/filesystem-binding.cpp +++ b/binding-mri/filesystem-binding.cpp @@ -33,6 +33,7 @@ fileIntFreeInstance(void *inst) { SDL_RWops *ops = static_cast(inst); + SDL_RWclose(ops); SDL_FreeRW(ops); } @@ -41,14 +42,16 @@ DEF_TYPE_CUSTOMFREE(FileInt, fileIntFreeInstance); static VALUE fileIntForPath(const char *path, bool rubyExc) { - SDL_RWops *ops; + SDL_RWops *ops = SDL_AllocRW(); try { - ops = shState->fileSystem().openReadRaw(path); + shState->fileSystem().openReadRaw(*ops, path); } catch (const Exception &e) { + SDL_FreeRW(ops); + if (rubyExc) raiseRbExc(e); else diff --git a/binding-mruby/binding-mruby.cpp b/binding-mruby/binding-mruby.cpp index b788d80..b2d7fcf 100644 --- a/binding-mruby/binding-mruby.cpp +++ b/binding-mruby/binding-mruby.cpp @@ -273,23 +273,23 @@ runRMXPScripts(mrb_state *mrb, mrbc_context *ctx) /* We use a secondary util state to unmarshal the scripts */ mrb_state *scriptMrb = mrb_open(); + SDL_RWops ops; - SDL_RWops *ops = shState->fileSystem().openReadRaw(scriptPack.c_str()); + shState->fileSystem().openReadRaw(ops, scriptPack.c_str()); mrb_value scriptArray = mrb_nil_value(); std::string readError; try { - scriptArray = marshalLoadInt(scriptMrb, ops); + scriptArray = marshalLoadInt(scriptMrb, &ops); } catch (const Exception &e) { readError = std::string(": ") + e.msg; } - SDL_RWclose(ops); - SDL_FreeRW(ops); + SDL_RWclose(&ops); if (!mrb_array_p(scriptArray)) { diff --git a/src/filesystem.cpp b/src/filesystem.cpp index a5c5dbf..c03dec6 100644 --- a/src/filesystem.cpp +++ b/src/filesystem.cpp @@ -222,6 +222,15 @@ static int SDL_RWopsClose(SDL_RWops *ops) return (result != 0) ? 0 : -1; } +static int SDL_RWopsCloseFree(SDL_RWops *ops) +{ + int result = SDL_RWopsClose(ops); + + SDL_FreeRW(ops); + + return result; +} + /* Copies the first srcN characters from src into dst, * or the full string if srcN == -1. Never writes more * than dstMax, and guarantees dst to be null terminated. @@ -263,13 +272,18 @@ findExt(const char *filename) static void initReadOps(PHYSFS_File *handle, - SDL_RWops &ops) + SDL_RWops &ops, + bool freeOnClose) { ops.size = SDL_RWopsSize; ops.seek = SDL_RWopsSeek; ops.read = SDL_RWopsRead; ops.write = SDL_RWopsWrite; - ops.close = SDL_RWopsClose; + + if (freeOnClose) + ops.close = SDL_RWopsCloseFree; + else + ops.close = SDL_RWopsClose; ops.type = SDL_RWOPS_PHYSFS; ops.hidden.unknown.data1 = handle; @@ -490,7 +504,7 @@ fontSetEnumCB (void *data, const char *dir, const char *fname) return PHYSFS_ENUM_ERROR; SDL_RWops ops; - initReadOps(handle, ops); + initReadOps(handle, ops, false); d->sfs->initFontSetCB(ops, filename); @@ -606,7 +620,7 @@ openReadEnumCB(void *d, const char *dirpath, const char *filename) return PHYSFS_ENUM_ERROR; } - initReadOps(phys, data.ops); + initReadOps(phys, data.ops, false); const char *ext = findExt(filename); @@ -670,27 +684,14 @@ void FileSystem::openRead(OpenHandler &handler, const char *filename) throw Exception(Exception::NoFileError, "%s", filename); } -SDL_RWops *FileSystem::openReadRaw(const char *filename) +void FileSystem::openReadRaw(SDL_RWops &ops, + const char *filename, + bool freeOnClose) { - SDL_RWops *ops; - PHYSFS_File *handle = PHYSFS_openRead(filename); - if (!handle) { - Debug() << "Couldn't open " << filename << " via PhysFS; trying normal FILE*"; - ops = SDL_RWFromFile(filename, "rb"); + assert(handle); - if (!ops) { - Debug() << "FILE* path failed too.."; - throw Exception(Exception::NoFileError, "%s", filename); - } - } - else - { - ops = SDL_AllocRW(); - initReadOps(handle, *ops); - } - - return ops; + initReadOps(handle, ops, freeOnClose); } bool FileSystem::exists(const char *filename) diff --git a/src/filesystem.h b/src/filesystem.h index 5e5a805..71cdb1b 100644 --- a/src/filesystem.h +++ b/src/filesystem.h @@ -60,7 +60,9 @@ public: const char *filename); /* Circumvents extension supplementing */ - SDL_RWops *openReadRaw(const char *filename); + void openReadRaw(SDL_RWops &ops, + const char *filename, + bool freeOnClose = false); /* Does not perform extension supplementing */ bool exists(const char *filename); diff --git a/src/font.cpp b/src/font.cpp index 33e48ec..97ebc76 100644 --- a/src/font.cpp +++ b/src/font.cpp @@ -167,13 +167,14 @@ _TTF_Font *SharedFontState::getFont(std::string family, const char *path = !req.regular.empty() ? req.regular.c_str() : req.other.c_str(); - ops = shState->fileSystem().openReadRaw(path); + ops = SDL_AllocRW(); + shState->fileSystem().openReadRaw(*ops, path, true); } // FIXME 0.9 is guesswork at this point // float gamma = (96.0/45.0)*(5.0/14.0)*(size-5); -// font = TTF_OpenFontRW(ops, 0, gamma /** .90*/); - font = TTF_OpenFontRW(ops, 0, size* 0.90f); +// font = TTF_OpenFontRW(ops, 1, gamma /** .90*/); + font = TTF_OpenFontRW(ops, 1, size* 0.90f); if (!font) throw Exception(Exception::SDLError, "%s", SDL_GetError());