From 9f99cf80f076a4a922f8d433336719a0498b3f40 Mon Sep 17 00:00:00 2001
From: Marcus Tillmanns <marcus.tillmanns@qt.io>
Date: Tue, 23 Apr 2024 07:47:42 +0200
Subject: [PATCH] Lua: Fix soft loading

Previously the LuaEngine::loadPlugin() function tried to fully setup the
lua interpreter. This was called before the dependencies of a lua plugin
were fully soft loaded. Therefore the dependent lua packages had not
been registered yet.

This patch changes that to where the packages and lua libraries are only
loaded right before the actual setup call is done. This also allows us
to safely load the plugin spec without giving it access to all the
functionality directly.

In the future we could show and ask the user before we load the plugin
whether he agrees to give it the requested access.

Change-Id: Ibf3e81db54e2ba94473e8ecf2650dcf2e97f1360
Reviewed-by: <github-actions-qt-creator@cristianadam.eu>
Reviewed-by: Christian Stenger <christian.stenger@qt.io>
---
 src/plugins/lua/luaengine.cpp     | 61 +++++++++++++++++++------------
 src/plugins/lua/luaengine.h       |  2 +
 src/plugins/lua/luapluginspec.cpp | 15 ++++----
 3 files changed, 47 insertions(+), 31 deletions(-)

diff --git a/src/plugins/lua/luaengine.cpp b/src/plugins/lua/luaengine.cpp
index 253a488bf11..d75d47ac7df 100644
--- a/src/plugins/lua/luaengine.cpp
+++ b/src/plugins/lua/luaengine.cpp
@@ -88,34 +88,10 @@ expected_str<LuaPluginSpec *> LuaEngine::loadPlugin(const Utils::FilePath &path)
 
     sol::state lua;
 
-    // TODO: Only open libraries requested by the plugin
-    lua.open_libraries(sol::lib::base,
-                       sol::lib::package,
-                       sol::lib::coroutine,
-                       sol::lib::string,
-                       sol::lib::os,
-                       sol::lib::math,
-                       sol::lib::table,
-                       sol::lib::debug,
-                       sol::lib::bit32,
-                       sol::lib::io);
-
     lua["print"] = [prefix = path.fileName()](sol::variadic_args va) {
         qDebug().noquote() << "[" << prefix << "]" << variadicToStringList(va).join("\t");
     };
 
-    for (const auto &[name, func] : d->m_providers.asKeyValueRange()) {
-        lua["package"]["preload"][name.toStdString()] = [func = func](const sol::this_state &s) {
-            return func(s);
-        };
-    }
-
-    for (const auto &func : d->m_autoProviders)
-        func(lua);
-
-    const QString searchPath = (path.parentDir() / "?.lua").toUserOutput();
-    lua["package"]["path"] = searchPath.toStdString();
-
     auto result = lua.safe_script(
         std::string_view(contents->data(), contents->size()),
         sol::script_pass_on_error,
@@ -135,6 +111,43 @@ expected_str<LuaPluginSpec *> LuaEngine::loadPlugin(const Utils::FilePath &path)
     return LuaPluginSpec::create(path, std::move(lua), pluginInfo);
 }
 
+expected_str<void> LuaEngine::prepareSetup(
+    sol::state_view &lua, const LuaPluginSpec &pluginSpec, sol::optional<sol::table> hookTable)
+{
+    // TODO: Only open libraries requested by the plugin
+    lua.open_libraries(
+        sol::lib::base,
+        sol::lib::bit32,
+        sol::lib::coroutine,
+        sol::lib::debug,
+        sol::lib::io,
+        sol::lib::math,
+        sol::lib::os,
+        sol::lib::package,
+        sol::lib::string,
+        sol::lib::table,
+        sol::lib::utf8);
+
+    const QString searchPath
+        = (FilePath::fromUserInput(pluginSpec.filePath()).parentDir() / "?.lua").toUserOutput();
+    lua["package"]["path"] = searchPath.toStdString();
+
+    // TODO: only register what the plugin requested
+    for (const auto &[name, func] : d->m_providers.asKeyValueRange()) {
+        lua["package"]["preload"][name.toStdString()] = [func = func](const sol::this_state &s) {
+            return func(s);
+        };
+    }
+
+    for (const auto &func : d->m_autoProviders)
+        func(lua);
+
+    if (hookTable)
+        return LuaEngine::connectHooks(lua, *hookTable);
+
+    return {};
+}
+
 bool LuaEngine::isCoroutine(lua_State *state)
 {
     bool ismain = lua_pushthread(state) == 1;
diff --git a/src/plugins/lua/luaengine.h b/src/plugins/lua/luaengine.h
index 0ea78e0211a..b0e34eb4169 100644
--- a/src/plugins/lua/luaengine.h
+++ b/src/plugins/lua/luaengine.h
@@ -41,6 +41,8 @@ public:
     static LuaEngine &instance();
 
     Utils::expected_str<LuaPluginSpec *> loadPlugin(const Utils::FilePath &path);
+    Utils::expected_str<void> prepareSetup(
+        sol::state_view &lua, const LuaPluginSpec &pluginSpec, sol::optional<sol::table> hookTable);
 
     static void registerProvider(const QString &packageName, const PackageProvider &provider);
     static void autoRegister(const std::function<void(sol::state_view)> &registerFunction);
diff --git a/src/plugins/lua/luapluginspec.cpp b/src/plugins/lua/luapluginspec.cpp
index 44f260bea04..dfbb54a41d2 100644
--- a/src/plugins/lua/luapluginspec.cpp
+++ b/src/plugins/lua/luapluginspec.cpp
@@ -94,14 +94,15 @@ bool LuaPluginSpec::loadLibrary()
 }
 bool LuaPluginSpec::initializePlugin()
 {
-    std::optional<sol::table> hookTable = d->pluginTable.get<std::optional<sol::table>>("hooks");
-    if (hookTable) {
-        auto res = LuaEngine::connectHooks(d->lua, *hookTable);
-        if (!res) {
-            setError(Lua::Tr::tr("Failed to connect hooks: %1").arg(res.error()));
-            return false;
-        }
+    expected_str<void> setupResult
+        = LuaEngine::instance()
+              .prepareSetup(d->lua, *this, d->pluginTable.get<sol::optional<sol::table>>("hooks"));
+
+    if (!setupResult) {
+        setError(Lua::Tr::tr("Failed to prepare plugin setup: %1").arg(setupResult.error()));
+        return false;
     }
+
     auto result = d->setupFunction.call();
 
     if (result.get_type() == sol::type::boolean && result.get<bool>() == false) {