From 1a9c50f2285362a35fe0b0d5cb572bde808b37dd Mon Sep 17 00:00:00 2001
From: kyren <kerriganw@gmail.com>
Date: Sun, 5 Aug 2018 20:03:47 -0400
Subject: [PATCH] Solve (maybe) *another* soundness issue with `Lua::scope`

Callbacks should not be able to capture their arguments and hold onto them,
because the `&Lua` used in previous calls will not remain valid across calls.
One could imagine an API where the specific `&Lua` is simply stored inside the
`Scope` itself, but this is harder to do, and would (badly) encourage storing
references inside Lua userdata.

Ideally, the only way it should be possible to store Lua handles inside Lua
itself is through usafety or the `rental` crate or other self-borrowing
techniques to make references into 'static types.  If at all possible this
roadblock should stay, because reference types inside userdata are almost always
going to lead to a a memory leak, and if you accept the risks you should just
use `RegistryKey` with its manual removal.
---
 src/scope.rs                                 | 14 +++++------
 tests/compile-fail/scope_callback_capture.rs | 25 ++++++++++++++++++++
 tests/compile-fail/scope_callback_escape.rs  |  2 +-
 tests/compile-fail/scope_callback_inner.rs   |  2 +-
 4 files changed, 34 insertions(+), 9 deletions(-)
 create mode 100644 tests/compile-fail/scope_callback_capture.rs

diff --git a/src/scope.rs b/src/scope.rs
index b8c2ad8..efda276 100644
--- a/src/scope.rs
+++ b/src/scope.rs
@@ -43,15 +43,15 @@ impl<'scope> Scope<'scope> {
     /// [`Lua::scope`]: struct.Lua.html#method.scope
     pub fn create_function<'lua, A, R, F>(&'lua self, func: F) -> Result<Function<'lua>>
     where
-        A: FromLuaMulti<'scope>,
-        R: ToLuaMulti<'scope>,
-        F: 'scope + Fn(&'scope Lua, A) -> Result<R>,
+        A: FromLuaMulti<'lua>,
+        R: ToLuaMulti<'lua>,
+        F: 'scope + Fn(&'lua Lua, A) -> Result<R>,
     {
         unsafe {
             let f = Box::new(move |lua, args| {
                 func(lua, A::from_lua_multi(args, lua)?)?.to_lua_multi(lua)
             });
-            let f = mem::transmute::<Callback<'scope, 'scope>, Callback<'scope, 'static>>(f);
+            let f = mem::transmute::<Callback<'lua, 'scope>, Callback<'lua, 'static>>(f);
             let f = self.lua.create_callback(f)?;
 
             let mut destructors = self.destructors.borrow_mut();
@@ -85,9 +85,9 @@ impl<'scope> Scope<'scope> {
     /// [`Scope::create_function`]: #method.create_function
     pub fn create_function_mut<'lua, A, R, F>(&'lua self, func: F) -> Result<Function<'lua>>
     where
-        A: FromLuaMulti<'scope>,
-        R: ToLuaMulti<'scope>,
-        F: 'scope + FnMut(&'scope Lua, A) -> Result<R>,
+        A: FromLuaMulti<'lua>,
+        R: ToLuaMulti<'lua>,
+        F: 'scope + FnMut(&'lua Lua, A) -> Result<R>,
     {
         let func = RefCell::new(func);
         self.create_function(move |lua, args| {
diff --git a/tests/compile-fail/scope_callback_capture.rs b/tests/compile-fail/scope_callback_capture.rs
new file mode 100644
index 0000000..4dbc881
--- /dev/null
+++ b/tests/compile-fail/scope_callback_capture.rs
@@ -0,0 +1,25 @@
+extern crate rlua;
+
+use rlua::*;
+
+fn main() {
+    struct Test {
+        field: i32,
+    }
+
+    let lua = Lua::new();
+    lua.scope(|scope| {
+        let mut inner: Option<Table> = None;
+        let f = scope
+            .create_function_mut(move |lua, t: Table| {
+                //~^ error: cannot infer an appropriate lifetime for autoref due to conflicting requirements
+                if let Some(old) = inner.take() {
+                    // Access old callback `Lua`.
+                }
+                inner = Some(t);
+                Ok(())
+            })
+            .unwrap();
+        f.call::<_, ()>(lua.create_table()).unwrap();
+    });
+}
diff --git a/tests/compile-fail/scope_callback_escape.rs b/tests/compile-fail/scope_callback_escape.rs
index 2da9d61..43c5226 100644
--- a/tests/compile-fail/scope_callback_escape.rs
+++ b/tests/compile-fail/scope_callback_escape.rs
@@ -12,8 +12,8 @@ fn main() {
     lua.scope(|scope| {
         let f = scope
             .create_function_mut(|_, t: Table| {
+                //~^^ error: borrowed data cannot be stored outside of its closure
                 outer = Some(t);
-                //~^ error: `*outer` does not live long enough
                 Ok(())
             })
             .unwrap();
diff --git a/tests/compile-fail/scope_callback_inner.rs b/tests/compile-fail/scope_callback_inner.rs
index 6e95a9e..6d086c6 100644
--- a/tests/compile-fail/scope_callback_inner.rs
+++ b/tests/compile-fail/scope_callback_inner.rs
@@ -12,8 +12,8 @@ fn main() {
         let mut inner: Option<Table> = None;
         let f = scope
             .create_function_mut(|_, t: Table| {
+                //~^ error: cannot infer an appropriate lifetime for autoref due to conflicting requirements
                 inner = Some(t);
-                //~^ error: `inner` does not live long enough
                 Ok(())
             })
             .unwrap();