BREAKING: Replace custom import() with normal Lua require(). (#5014)

* Use require() instead of a custom import()

* Also search relative to the current file

* Update documentation

CHANGELOG.md

@@ -89,6 +89,7 @@
 - Dev: Refactored the Image Uploader feature. (#4971)
 - Dev: Fixed deadlock and use-after-free in tests. (#4981)
 - Dev: Load less message history upon reconnects. (#5001)
+- Dev: BREAKING: Replace custom `import()` with normal Lua `require()`. (#5014)
 - Dev: Fixed most compiler warnings. (#5028)
 
 ## 2.4.6

docs/wip-plugins.md

@@ -158,18 +158,25 @@ It achieves this by forcing all inputs to be encoded with `UTF-8`.
 
 See [official documentation](https://www.lua.org/manual/5.4/manual.html#pdf-load)
 
-#### `import(filename)`
+#### `require(modname)`
 
-This function mimics Lua's `dofile` however relative paths are relative to your plugin's directory.
+This is Lua's [`require()`](https://www.lua.org/manual/5.3/manual.html#pdf-require) function.
-You are restricted to loading files in your plugin's directory. You cannot load files with bytecode inside.
+However the searcher and load configuration is notably different than default:
+
+- Lua's built-in dynamic library searcher is removed,
+- `package.path` is not used, in its place are two searchers,
+- when `require()` is used, first a file relative to the currently executing
+  file will be checked, then a file relative to the plugin directory,
+- binary chunks are never loaded
+
+As in normal Lua, dots are converted to the path separators (`'/'` on Linux and Mac, `'\'` on Windows).
 
 Example:
 
 ```lua
-import("stuff.lua") -- executes Plugins/name/stuff.lua
+require("stuff") -- executes Plugins/name/stuff.lua or $(dirname $CURR_FILE)/stuff.lua
-import("./stuff.lua") -- executes Plugins/name/stuff.lua
+require("dir.name") -- executes Plugins/name/dir/name.lua or $(dirname $CURR_FILE)/dir/name.lua
-import("../stuff.lua") -- tries to load Plugins/stuff.lua and errors
+require("binary") -- tried to load Plugins/name/binary.lua and errors because binary is not a text file
-import("luac.out") -- tried to load Plugins/name/luac.out and errors because it contains non-utf8 data
 ```
 
 #### `print(Args...)`

src/controllers/plugins/LuaAPI.cpp

@@ -15,6 +15,7 @@
 #    include <QFileInfo>
 #    include <QLoggingCategory>
 #    include <QTextCodec>
+#    include <QUrl>
 
 namespace {
 using namespace chatterino;
@@ -282,69 +283,87 @@ int g_load(lua_State *L)
 #    endif
 }
 
-int g_import(lua_State *L)
+int loadfile(lua_State *L, const QString &str)
 {
-    auto countArgs = lua_gettop(L);
-    // Lua allows dofile() which loads from stdin, but this is very useless in our case
-    if (countArgs == 0)
-    {
-        lua_pushnil(L);
-        luaL_error(L, "it is not allowed to call import() without arguments");
-        return 1;
-    }
-
     auto *pl = getApp()->plugins->getPluginByStatePtr(L);
-    QString fname;
+    if (pl == nullptr)
-    if (!lua::pop(L, &fname))
     {
-        lua_pushnil(L);
+        return luaL_error(L, "loadfile: internal error: no plugin?");
-        luaL_error(L, "chatterino g_import: expected a string for a filename");
-        return 1;
     }
     auto dir = QUrl(pl->loadDirectory().canonicalPath() + "/");
-    auto file = dir.resolved(fname);
 
-    qCDebug(chatterinoLua) << "plugin" << pl->id << "is trying to load" << file
+    if (!dir.isParentOf(str))
-                           << "(its dir is" << dir << ")";
-    if (!dir.isParentOf(file))
     {
-        lua_pushnil(L);
+        // XXX: This intentionally hides the resolved path to not leak it
-        luaL_error(L, "chatterino g_import: filename must be inside of the "
+        lua::push(
-                      "plugin directory");
+            L, QString("requested module is outside of the plugin directory"));
+        return 1;
+    }
+    QFileInfo info(str);
+    if (!info.exists())
+    {
+        lua::push(L, QString("no file '%1'").arg(str));
         return 1;
     }
 
-    auto path = file.path(QUrl::FullyDecoded);
+    auto temp = str.toStdString();
-    QFile qf(path);
+    const auto *filename = temp.c_str();
-    qf.open(QIODevice::ReadOnly);
+
-    if (qf.size() > 10'000'000)
+    auto res = luaL_loadfilex(L, filename, "t");
+    // Yoinked from checkload lib/lua/src/loadlib.c
+    if (res == LUA_OK)
     {
-        lua_pushnil(L);
+        lua_pushstring(L, filename);
-        luaL_error(L, "chatterino g_import: size limit of 10MB exceeded, what "
+        return 2;
-                      "the hell are you doing");
+    }
+
+    return luaL_error(L, "error loading module '%s' from file '%s':\n\t%s",
+                      lua_tostring(L, 1), filename, lua_tostring(L, -1));
+}
+
+int searcherAbsolute(lua_State *L)
+{
+    auto name = QString::fromUtf8(luaL_checkstring(L, 1));
+    name = name.replace('.', QDir::separator());
+
+    QString filename;
+    auto *pl = getApp()->plugins->getPluginByStatePtr(L);
+    if (pl == nullptr)
+    {
+        return luaL_error(L, "searcherAbsolute: internal error: no plugin?");
+    }
+
+    QFileInfo file(pl->loadDirectory().filePath(name + ".lua"));
+    return loadfile(L, file.canonicalFilePath());
+}
+
+int searcherRelative(lua_State *L)
+{
+    lua_Debug dbg;
+    lua_getstack(L, 1, &dbg);
+    lua_getinfo(L, "S", &dbg);
+    auto currentFile = QString::fromUtf8(dbg.source, dbg.srclen);
+    if (currentFile.startsWith("@"))
+    {
+        currentFile = currentFile.mid(1);
+    }
+    if (currentFile == "=[C]" || currentFile == "")
+    {
+        lua::push(
+            L,
+            QString(
+                "Unable to load relative to file:caller has no source file"));
         return 1;
     }
 
-    // validate utf-8 to block bytecode exploits
+    auto parent = QFileInfo(currentFile).dir();
-    auto data = qf.readAll();
-    auto *utf8 = QTextCodec::codecForName("UTF-8");
-    QTextCodec::ConverterState state;
-    utf8->toUnicode(data.constData(), data.size(), &state);
-    if (state.invalidChars != 0)
-    {
-        lua_pushnil(L);
-        luaL_error(L, "invalid utf-8 in import() target (%s) is not allowed",
-                   fname.toStdString().c_str());
-        return 1;
-    }
 
-    // fetch dofile and call it
+    auto name = QString::fromUtf8(luaL_checkstring(L, 1));
-    lua_getfield(L, LUA_REGISTRYINDEX, "real_dofile");
+    name = name.replace('.', QDir::separator());
-    // maybe data race here if symlink was swapped?
+    QString filename =
-    lua::push(L, path);
+        parent.canonicalPath() + QDir::separator() + name + ".lua";
-    lua_call(L, 1, LUA_MULTRET);
 
-    return lua_gettop(L);
+    return loadfile(L, filename);
 }
 
 int g_print(lua_State *L)

src/controllers/plugins/LuaAPI.hpp

@@ -20,9 +20,12 @@ int c2_log(lua_State *L);
 // These ones are global
 int g_load(lua_State *L);
 int g_print(lua_State *L);
-int g_import(lua_State *L);
 // NOLINTEND(readability-identifier-naming)
 
+// This is for require() exposed as an element of package.searchers
+int searcherAbsolute(lua_State *L);
+int searcherRelative(lua_State *L);
+
 // Exposed as c2.LogLevel
 // Represents "calls" to qCDebug, qCInfo ...
 enum class LogLevel { Debug, Info, Warning, Critical };

src/controllers/plugins/PluginController.cpp

@@ -103,9 +103,10 @@ bool PluginController::tryLoadFromDir(const QDir &pluginDir)
     return true;
 }
 
-void PluginController::openLibrariesFor(lua_State *L,
+void PluginController::openLibrariesFor(lua_State *L, const PluginMeta &meta,
-                                        const PluginMeta & /*meta*/)
+                                        const QDir &pluginDir)
 {
+    lua::StackGuard guard(L);
     // Stuff to change, remove or hide behind a permission system:
     static const std::vector<luaL_Reg> loadedlibs = {
         luaL_Reg{LUA_GNAME, luaopen_base},
@@ -123,6 +124,7 @@ void PluginController::openLibrariesFor(lua_State *L,
         luaL_Reg{LUA_STRLIBNAME, luaopen_string},
         luaL_Reg{LUA_MATHLIBNAME, luaopen_math},
         luaL_Reg{LUA_UTF8LIBNAME, luaopen_utf8},
+        luaL_Reg{LUA_LOADLIBNAME, luaopen_package},
     };
     // Warning: Do not add debug library to this, it would make the security of
     // this a living nightmare due to stuff like registry access
@@ -144,7 +146,7 @@ void PluginController::openLibrariesFor(lua_State *L,
         {nullptr, nullptr},
     };
     lua_pushglobaltable(L);
-    auto global = lua_gettop(L);
+    auto gtable = lua_gettop(L);
 
     // count of elements in C2LIB + LogLevel + EventType
     auto c2libIdx = lua::pushEmptyTable(L, 8);
@@ -157,14 +159,11 @@ void PluginController::openLibrariesFor(lua_State *L,
     lua::pushEnumTable<lua::api::EventType>(L);
     lua_setfield(L, c2libIdx, "EventType");
 
-    lua_setfield(L, global, "c2");
+    lua_setfield(L, gtable, "c2");
 
     // ban functions
     // Note: this might not be fully secure? some kind of metatable fuckery might come up?
 
-    lua_pushglobaltable(L);
-    auto gtable = lua_gettop(L);
-
     // possibly randomize this name at runtime to prevent some attacks?
 
 #    ifndef NDEBUG
@@ -172,16 +171,10 @@ void PluginController::openLibrariesFor(lua_State *L,
     lua_setfield(L, LUA_REGISTRYINDEX, "real_load");
 #    endif
 
-    lua_getfield(L, gtable, "dofile");
-    lua_setfield(L, LUA_REGISTRYINDEX, "real_dofile");
-
     // NOLINTNEXTLINE(*-avoid-c-arrays)
     static const luaL_Reg replacementFuncs[] = {
         {"load", lua::api::g_load},
         {"print", lua::api::g_print},
-
-        // This function replaces both `dofile` and `require`, see docs/wip-plugins.md for more info
-        {"import", lua::api::g_import},
         {nullptr, nullptr},
     };
     luaL_setfuncs(L, replacementFuncs, 0);
@@ -192,7 +185,43 @@ void PluginController::openLibrariesFor(lua_State *L,
     lua_pushnil(L);
     lua_setfield(L, gtable, "dofile");
 
-    lua_pop(L, 1);
+    // set up package lib
+    lua_getfield(L, gtable, "package");
+
+    auto package = lua_gettop(L);
+    lua_pushstring(L, "");
+    lua_setfield(L, package, "cpath");
+
+    // we don't use path
+    lua_pushstring(L, "");
+    lua_setfield(L, package, "path");
+
+    {
+        lua_getfield(L, gtable, "table");
+        auto table = lua_gettop(L);
+        lua_getfield(L, -1, "remove");
+        lua_remove(L, table);
+    }
+    auto remove = lua_gettop(L);
+
+    // remove searcher_Croot, searcher_C and searcher_Lua leaving only searcher_preload
+    for (int i = 0; i < 3; i++)
+    {
+        lua_pushvalue(L, remove);
+        lua_getfield(L, package, "searchers");
+        lua_pcall(L, 1, 0, 0);
+    }
+    lua_pop(L, 1);  // get rid of remove
+
+    lua_getfield(L, package, "searchers");
+    lua_pushcclosure(L, lua::api::searcherRelative, 0);
+    lua_seti(L, -2, 2);
+
+    lua::push(L, QString(pluginDir.absolutePath()));
+    lua_pushcclosure(L, lua::api::searcherAbsolute, 1);
+    lua_seti(L, -2, 3);
+
+    lua_pop(L, 3);  // remove gtable, package, package.searchers
 }
 
 void PluginController::load(const QFileInfo &index, const QDir &pluginDir,
@@ -211,7 +240,7 @@ void PluginController::load(const QFileInfo &index, const QDir &pluginDir,
                                  << " because safe mode is enabled.";
         return;
     }
-    PluginController::openLibrariesFor(l, meta);
+    PluginController::openLibrariesFor(l, meta, pluginDir);
 
     if (!PluginController::isPluginEnabled(pluginName) ||
         !getSettings()->pluginsEnabled)

src/controllers/plugins/PluginController.hpp

@@ -63,7 +63,8 @@ private:
               const PluginMeta &meta);
 
     // This function adds lua standard libraries into the state
-    static void openLibrariesFor(lua_State *L, const PluginMeta & /*meta*/);
+    static void openLibrariesFor(lua_State *L, const PluginMeta & /*meta*/,
+                                 const QDir &pluginDir);
     static void loadChatterinoLib(lua_State *l);
     bool tryLoadFromDir(const QDir &pluginDir);
     std::map<QString, std::unique_ptr<Plugin>> plugins_;