From c218b1da72019185fdc59df88679690e907acc12 Mon Sep 17 00:00:00 2001
From: Uli Schlachter <psychon@znc.in>
Date: Sat, 29 Oct 2016 09:34:45 +0200
Subject: [PATCH] Test and fix swapping clients

The code in luaA_client_swap() is incorrect, because
luaA_object_emit_signal() already pops the arguments to the signal.
Still, the code here tried to remove the arguments from the Lua stack
again, thereby corrupting the stack (removing more items than there are
in the stack).

Normally, popping more things from the stack than it has entries
silently corrupts the Lua stack. Apparently this doesn't necessarily
cause any immediate issues, because this code has been broken since nine
months and no one noticed. This mistakes was introduced in commit
55190646.

This issue was only noticed by accident. Thus, this commit also adds a
small integration test that exercises this bug. This test catches the
issue, but only on Travis, because there we are building our own version
of Lua 5.3 and that one has assertions enabled.

Signed-off-by: Uli Schlachter <psychon@znc.in>
---
 objects/client.c           |  2 --
 tests/test-client-swap.lua | 33 +++++++++++++++++++++++++++++++++
 2 files changed, 33 insertions(+), 2 deletions(-)
 create mode 100644 tests/test-client-swap.lua

diff --git a/objects/client.c b/objects/client.c
index 94897aad0..e21c2ab93 100644
--- a/objects/client.c
+++ b/objects/client.c
@@ -2343,13 +2343,11 @@ luaA_client_swap(lua_State *L)
         luaA_object_push(L, swap);
         lua_pushboolean(L, true);
         luaA_object_emit_signal(L, -4, "swapped", 2);
-        lua_pop(L, 2);
 
         luaA_object_push(L, swap);
         luaA_object_push(L, c);
         lua_pushboolean(L, false);
         luaA_object_emit_signal(L, -3, "swapped", 2);
-        lua_pop(L, 3);
     }
 
     return 0;
diff --git a/tests/test-client-swap.lua b/tests/test-client-swap.lua
new file mode 100644
index 000000000..6ed96828c
--- /dev/null
+++ b/tests/test-client-swap.lua
@@ -0,0 +1,33 @@
+-- Test if client's c:swap() corrupts the Lua stack
+
+local runner = require("_runner")
+local test_client = require("_client")
+
+runner.run_steps({
+    -- Spawn two clients
+    function(count)
+        if count == 1 then
+            test_client()
+            test_client()
+        end
+        if #client.get() >= 2 then
+            return true
+        end
+    end,
+
+    -- Swap them
+    function()
+        assert(#client.get() == 2, #client.get())
+        local c1 = client.get()[1]
+        local c2 = client.get()[2]
+
+        c2:swap(c1)
+        c1:swap(c2)
+        c1:swap(c2)
+        c1:swap(c2)
+        c2:swap(c1)
+        return true
+    end,
+})
+
+-- vim: filetype=lua:expandtab:shiftwidth=4:tabstop=8:softtabstop=4:textwidth=80