mod_lua: fix LuaScope server with inline blocks and .htaccess paths

changes-entries/lua_dump_lua55.txt

@@ -0,0 +1,2 @@
+  *) mod_lua: Inline <LuaHook...> blocks now work when built against
+     Lua 5.5. [Eric Covener]

changes-entries/lua_scope_server_copy.txt

@@ -0,0 +1,3 @@
+  *) mod_lua: Inline <LuaHook...> blocks, and LuaPackagePath/LuaPackageCPath
+     set in .htaccess, now work together with "LuaScope server".
+     [Eric Covener]

modules/lua/lua_vmprep.c

@@ -395,16 +395,44 @@ static apr_status_t vm_construct(lua_State **vm, void *params, apr_pool_t *lifec
     return APR_SUCCESS;
 }
 
+/* Deep-copy an array of strings; apr_array_copy() copies only the
+ * pointers, and the strings may live in a shorter-lived pool.
+ */
+static apr_array_header_t *copy_string_array(apr_pool_t *pool,
+                                             const apr_array_header_t *arr)
+{
+    apr_array_header_t *copy;
+    int i;
+
+    if (arr == NULL) {
+        return NULL;
+    }
+    copy = apr_array_copy(pool, arr);
+    for (i = 0; i < copy->nelts; i++) {
+        APR_ARRAY_IDX(copy, i, const char *) =
+            apr_pstrdup(pool, APR_ARRAY_IDX(copy, i, const char *));
+    }
+    return copy;
+}
+
+/* Copy a VM spec into a longer-lived pool for the server-scoped VM pool.
+ * Everything the spec points to has to be copied too, since the original
+ * may belong to a request (or .htaccess) pool.
+ */
 static ap_lua_vm_spec* copy_vm_spec(apr_pool_t* pool, ap_lua_vm_spec* spec) 
 {
     ap_lua_vm_spec* copied_spec = apr_pcalloc(pool, sizeof(ap_lua_vm_spec));
     copied_spec->bytecode_len = spec->bytecode_len;
-    copied_spec->bytecode = apr_pstrdup(pool, spec->bytecode);
+    if (spec->bytecode && spec->bytecode_len > 0) {
+        /* bytecode is binary and contains NUL bytes; not a C string */
+        copied_spec->bytecode = apr_pmemdup(pool, spec->bytecode,
+                                            spec->bytecode_len);
+    }
     copied_spec->cb = spec->cb;
     copied_spec->cb_arg = NULL;
     copied_spec->file = apr_pstrdup(pool, spec->file);
-    copied_spec->package_cpaths = apr_array_copy(pool, spec->package_cpaths);
-    copied_spec->package_paths = apr_array_copy(pool, spec->package_paths);
+    copied_spec->package_cpaths = copy_string_array(pool, spec->package_cpaths);
+    copied_spec->package_paths = copy_string_array(pool, spec->package_paths);
     copied_spec->pool = pool;
     copied_spec->scope = AP_LUA_SCOPE_SERVER;
     copied_spec->codecache = spec->codecache;

modules/lua/mod_lua.c

@@ -39,6 +39,7 @@
 #include "lua_config.h"
 #include "mod_auth.h"
 #include "util_mutex.h"
+#include "util_varbuf.h"
 
 #ifdef APR_HAS_THREADS
 #include "apr_thread_proc.h"
@@ -951,10 +952,14 @@ static const char *direct_chunkreader(lua_State *lvm, void *udata,
     return ctx->buf;
 }
 
-static int ldump_writer(lua_State *L, const void *b, size_t size, void *B)
+/* lua_dump() writer: collect the bytecode outside the Lua stack, which
+ * lua_dump() itself may be using (Lua 5.5 pushes a table while dumping),
+ * so a luaL_Buffer must not be used here.
+ */
+static int ldump_writer(lua_State *L, const void *b, size_t size, void *vb)
 {
     (void) L;
-    luaL_addlstring((luaL_Buffer *) B, (const char *) b, size);
+    ap_varbuf_strmemcat((struct ap_varbuf *) vb, (const char *) b, (int) size);
     return 0;
 }
 
@@ -1078,14 +1083,17 @@ static const char *register_named_block_function_hook(const char *name,
             return errstr;
         }
         else {
-            luaL_Buffer b;
-            luaL_buffinit(lvm, &b);
-            lua_dump(lvm, ldump_writer, &b);
-            luaL_pushresult(&b);
-            spec->bytecode_len = lua_rawlen(lvm, -1);
-            spec->bytecode = apr_pstrmemdup(cmd->pool, lua_tostring(lvm, -1),
-                                            spec->bytecode_len);
+            struct ap_varbuf vb;
+            ap_varbuf_init(cmd->temp_pool, &vb, 0);
+            vb.strlen = 0;
+            rv = lua_dump(lvm, ldump_writer, &vb);
             lua_close(lvm);
+            if (rv != 0) {
+                return apr_pstrcat(cmd->pool, cmd->cmd->name,
+                                   "> could not dump compiled Lua code", NULL);
+            }
+            spec->bytecode_len = vb.strlen;
+            spec->bytecode = apr_pmemdup(cmd->pool, vb.buf, vb.strlen);
         }
 
         current = mconfig;

test/modules/lua/conftest.py

@@ -0,0 +1,35 @@
+import logging
+import os
+
+import pytest
+import sys
+
+from .env import LuaTestEnv
+
+sys.path.append(os.path.join(os.path.dirname(__file__), '../..'))
+
+
+def pytest_report_header(config, start_path):
+    env = LuaTestEnv()
+    return f"lua [apache: {env.get_httpd_version()}, mpm: {env.mpm_module}, {env.prefix}]"
+
+
+@pytest.fixture(scope="package")
+def env(pytestconfig) -> LuaTestEnv:
+    level = logging.INFO
+    console = logging.StreamHandler()
+    console.setLevel(level)
+    console.setFormatter(logging.Formatter('%(levelname)s: %(message)s'))
+    logging.getLogger('').addHandler(console)
+    logging.getLogger('').setLevel(level=level)
+    env = LuaTestEnv(pytestconfig=pytestconfig)
+    env.setup_httpd()
+    env.apache_access_log_clear()
+    env.httpd_error_log.clear_log()
+    return env
+
+
+@pytest.fixture(autouse=True, scope="package")
+def _stop_package_scope(env):
+    yield
+    assert env.apache_stop() == 0

test/modules/lua/env.py

@@ -0,0 +1,27 @@
+import inspect
+import logging
+import os
+
+from pyhttpd.env import HttpdTestEnv, HttpdTestSetup
+
+log = logging.getLogger(__name__)
+
+
+class LuaTestSetup(HttpdTestSetup):
+
+    def __init__(self, env: 'HttpdTestEnv'):
+        super().__init__(env=env)
+        self.add_source_dir(os.path.dirname(inspect.getfile(LuaTestSetup)))
+        # mod_lua needs liblua, so it may not have been built; the tests
+        # skip when it is absent.
+        self.add_optional_modules(["lua"])
+
+
+class LuaTestEnv(HttpdTestEnv):
+
+    def __init__(self, pytestconfig=None):
+        super().__init__(pytestconfig=pytestconfig)
+        self.add_httpd_log_modules(["lua"])
+
+    def setup_httpd(self, setup: HttpdTestSetup = None):
+        super().setup_httpd(setup=LuaTestSetup(env=self))

test/modules/lua/test_005_scope_server_block.py

@@ -0,0 +1,40 @@
+import pytest
+
+from pyhttpd.conf import HttpdConf
+
+
+# A <LuaHook...> block is compiled to bytecode when the config is read.
+# With "LuaScope server" the VM spec is copied for the per-server VM pool,
+# and the copy has to carry the whole bytecode, which contains NUL bytes.
+class TestScopeServerBlock:
+
+    @pytest.fixture(autouse=True, scope='class')
+    def _class_scope(self, env):
+        env.require(env.has_shared_module("lua"), "mod_lua not built",
+                    dep="mod_lua")
+
+    def configure(self, env, scope):
+        conf = HttpdConf(env, extras={
+            f"test1.{env.http_tld}": f"""
+            <Location "/">
+                LuaScope {scope}
+                <LuaHookFixups fixup>
+                function fixup(r)
+                    r.headers_out["X-Lua-Fixup"] = "hit"
+                    return apache2.DECLINED
+                end
+                </LuaHookFixups>
+            </Location>
+            """,
+        })
+        conf.add_vhost_test1()
+        conf.install()
+        assert env.apache_restart() == 0
+
+    @pytest.mark.parametrize("scope", ["once", "request", "server 1 1"])
+    def test_lua_005_01_block_hook(self, env, scope):
+        self.configure(env, scope)
+        for _ in range(3):
+            r = env.curl_get(env.mkurl("http", "test1", "/index.html"))
+            assert r.response["status"] == 200, r.response
+            assert r.response["header"].get("x-lua-fixup") == "hit"