Merge branch 'fix_findpath_use_after_free' into 'master'

Fix findPath returning a raw vector Closes #8238 See merge request OpenMW/openmw!4470
OpenMW · Dec 3, 2024 · b285e2f · b285e2f
2 parents c454675 + 30e6821
commit b285e2f
Show file tree

Hide file tree

Showing 5 changed files with 19 additions and 10 deletions.
diff --git a/apps/openmw/mwlua/nearbybindings.cpp b/apps/openmw/mwlua/nearbybindings.cpp
@@ -227,7 +227,7 @@ namespace MWLua
             | DetourNavigator::Flag_swim | DetourNavigator::Flag_openDoor | DetourNavigator::Flag_usePathgrid;
 
         api["findPath"]
-            = [](const osg::Vec3f& source, const osg::Vec3f& destination, const sol::optional<sol::table>& options) {
+            = [lua](const osg::Vec3f& source, const osg::Vec3f& destination, const sol::optional<sol::table>& options) {
                   DetourNavigator::AgentBounds agentBounds = defaultAgentBounds;
                   DetourNavigator::Flags includeFlags = defaultIncludeFlags;
                   DetourNavigator::AreaCosts areaCosts{};
@@ -259,13 +259,15 @@ namespace MWLua
                           destinationTolerance = *v;
                   }
 
-                  std::vector<osg::Vec3f> result;
+                  std::vector<osg::Vec3f> path;
 
-                  const DetourNavigator::Status status = DetourNavigator::findPath(
-                      *MWBase::Environment::get().getWorld()->getNavigator(), agentBounds, source, destination,
-                      includeFlags, areaCosts, destinationTolerance, std::back_inserter(result));
+                  const DetourNavigator::Status status
+                      = DetourNavigator::findPath(*MWBase::Environment::get().getWorld()->getNavigator(), agentBounds,
+                          source, destination, includeFlags, areaCosts, destinationTolerance, std::back_inserter(path));
 
-                  return std::make_tuple(status, std::move(result));
+                  sol::table result(lua, sol::create);
+                  LuaUtil::copyVectorToTable(path, result);
+                  return std::make_tuple(status, result);
               };
 
         api["findRandomPointAroundCircle"] = [](const osg::Vec3f& position, float maxRadius,

diff --git a/components/lua/luastate.hpp b/components/lua/luastate.hpp
@@ -372,6 +372,12 @@ namespace LuaUtil
     }
     sol::table getMutableFromReadOnly(const sol::userdata&);
 
+    template <class T>
+    void copyVectorToTable(const std::vector<T>& v, sol::table& out)
+    {
+        for (const T& t : v)
+            out.add(t);
+    }
 }
 
 #endif // COMPONENTS_LUA_LUASTATE_H
diff --git a/scripts/data/integration_tests/test_lua_api/player.lua b/scripts/data/integration_tests/test_lua_api/player.lua
@@ -176,8 +176,8 @@ testing.registerLocalTest('findPath',
         }
         local status, path = nearby.findPath(src, dst, options)
         testing.expectEqual(status, nearby.FIND_PATH_STATUS.Success, 'Status')
-        testing.expectLessOrEqual((path[path:size()] - dst):length(), 1,
-            'Last path point '  .. testing.formatActualExpected(path[path:size()], dst))
+        testing.expectLessOrEqual((path[#path] - dst):length(), 1,
+            'Last path point '  .. testing.formatActualExpected(path[#path], dst))
     end)
 
 testing.registerLocalTest('findRandomPointAroundCircle',

diff --git a/scripts/data/morrowind_tests/.gitignore b/scripts/data/morrowind_tests/.gitignore
@@ -0,0 +1 @@
+test_workdir/*
diff --git a/scripts/data/morrowind_tests/player.lua b/scripts/data/morrowind_tests/player.lua
@@ -42,8 +42,8 @@ testing.registerLocalTest('Guard in Imperial Prison Ship should find path (#7241
         }
         local status, path = nearby.findPath(src, dst, options)
         testing.expectEqual(status, nearby.FIND_PATH_STATUS.Success, 'Status')
-        testing.expectLessOrEqual((util.vector2(path[path:size()].x, path[path:size()].y) - util.vector2(dst.x, dst.y)):length(), 1, 'Last path point x, y')
-        testing.expectLessOrEqual(path[path:size()].z - dst.z, 20, 'Last path point z')
+        testing.expectLessOrEqual((util.vector2(path[#path].x, path[#path].y) - util.vector2(dst.x, dst.y)):length(), 1, 'Last path point x, y')
+        testing.expectLessOrEqual(path[#path].z - dst.z, 20, 'Last path point z')
         if agentBounds.shapeType == nearby.COLLISION_SHAPE_TYPE.Aabb then
             testing.expectThat(path, testing.elementsAreArray({
                 testing.closeToVector(util.vector3(34.29737091064453125, 806.3817138671875, 112.76610565185546875), 1e-1),