Name every Lua filter failure in the log

A script raising anything but a string handed die a null to print, and
a Lua state that could not be created was used anyway once the filter
closed.

2 files changed, 30 insertions, 9 deletions

diff --git a/source/filter.c b/source/filter.cindex b36f03e..43ba70a 100644--- a/source/filter.c+++ b/source/filter.c@@ -191,12 +191,20 @@ static inline void unhook_write(void)  static NORETURN void die_lua_error(struct lua_filter *filter) {+	lua_State *lua_state = filter->lua_state;+	const char *message = lua_tostring(lua_state, -1);++	// A script may raise any value, and only a string or a number has text+	// of its own, so anything else is named by its type, as lua.c does.+	if (!message)+		message = lua_pushfstring(lua_state, "(error object is a %s value)",+			luaL_typename(lua_state, -1)); 	// The error page goes out through stdout, which may still be hooked 	// to the very filter that failed, so the hook comes off first or the 	// page would be fed back into the broken filter. 	if (filter_write) 		unhook_write();-	die("Lua error in %s: %s", filter->script_file, lua_tostring(filter->lua_state, -1));+	die("Lua error in %s: %s", filter->script_file, message); }  static ssize_t write_lua_filter(struct cgit_filter *base, const void *buf, size_t count)@@ -294,15 +302,18 @@ static const struct { 	{ "html_include", script_html_include }, }; -static int init_lua_filter(struct lua_filter *filter)+static void init_lua_filter(struct lua_filter *filter) { 	size_t i;  	if (filter->lua_state)-		return 0;+		return; -	if (!(filter->lua_state = luaL_newstate()))-		return 1;+	// A new state fails only when memory has run out, which cgit answers by+	// dying wherever else it allocates.+	filter->lua_state = luaL_newstate();+	if (!filter->lua_state)+		die("Unable to create a Lua state for %s", filter->script_file);  	luaL_openlibs(filter->lua_state); @@ -313,7 +324,6 @@ static int init_lua_filter(struct lua_filter *filter)  	if (luaL_dofile(filter->lua_state, filter->script_file)) 		die_lua_error(filter);-	return 0; }  static int open_lua_filter(struct cgit_filter *base, va_list ap)@@ -321,9 +331,7 @@ static int open_lua_filter(struct cgit_filter *base, va_list ap) 	struct lua_filter *filter = (struct lua_filter *)base; 	int i; -	if (init_lua_filter(filter))-		return 1;-+	init_lua_filter(filter); 	hook_write(base, write_lua_filter);  	lua_getglobal(filter->lua_state, "filter_open");
diff --git a/source/filter.c b/source/filter.c
index b36f03e..43ba70a 100644
--- a/source/filter.c
+++ b/source/filter.c
@@ -191,12 +191,20 @@ static inline void unhook_write(void)
 
 static NORETURN void die_lua_error(struct lua_filter *filter)
 {
+	lua_State *lua_state = filter->lua_state;
+	const char *message = lua_tostring(lua_state, -1);
+
+	// A script may raise any value, and only a string or a number has text
+	// of its own, so anything else is named by its type, as lua.c does.
+	if (!message)
+		message = lua_pushfstring(lua_state, "(error object is a %s value)",
+			luaL_typename(lua_state, -1));
 	// The error page goes out through stdout, which may still be hooked
 	// to the very filter that failed, so the hook comes off first or the
 	// page would be fed back into the broken filter.
 	if (filter_write)
 		unhook_write();
-	die("Lua error in %s: %s", filter->script_file, lua_tostring(filter->lua_state, -1));
+	die("Lua error in %s: %s", filter->script_file, message);
 }
 
 static ssize_t write_lua_filter(struct cgit_filter *base, const void *buf, size_t count)
@@ -294,15 +302,18 @@ static const struct {
 	{ "html_include", script_html_include },
 };
 
-static int init_lua_filter(struct lua_filter *filter)
+static void init_lua_filter(struct lua_filter *filter)
 {
 	size_t i;
 
 	if (filter->lua_state)
-		return 0;
+		return;
 
-	if (!(filter->lua_state = luaL_newstate()))
-		return 1;
+	// A new state fails only when memory has run out, which cgit answers by
+	// dying wherever else it allocates.
+	filter->lua_state = luaL_newstate();
+	if (!filter->lua_state)
+		die("Unable to create a Lua state for %s", filter->script_file);
 
 	luaL_openlibs(filter->lua_state);
 
@@ -313,7 +324,6 @@ static int init_lua_filter(struct lua_filter *filter)
 
 	if (luaL_dofile(filter->lua_state, filter->script_file))
 		die_lua_error(filter);
-	return 0;
 }
 
 static int open_lua_filter(struct cgit_filter *base, va_list ap)
@@ -321,9 +331,7 @@ static int open_lua_filter(struct cgit_filter *base, va_list ap)
 	struct lua_filter *filter = (struct lua_filter *)base;
 	int i;
 
-	if (init_lua_filter(filter))
-		return 1;
-
+	init_lua_filter(filter);
 	hook_write(base, write_lua_filter);
 
 	lua_getglobal(filter->lua_state, "filter_open");
diff --git a/tests/t0201-filters.sh b/tests/t0201-filters.shindex c2d0ec4..1ffb973 100755--- a/tests/t0201-filters.sh+++ b/tests/t0201-filters.sh@@ -130,6 +130,19 @@ test_expect_success CGIT_LUA 'a failing lua filter still renders an error page' 	grep "Lua error in" err ' +# A script may raise a value that is no string, which the log still names.+test_expect_success CGIT_LUA 'a lua filter raising a table is named by type' '+	cat >tabled.lua <<-\EOF &&+	function filter_open(...) error({}) end+	function filter_write(str) end+	function filter_close() return 0 end+	EOF+	sed "s|broken.lua|tabled.lua|" brokenrc >tabledrc &&+	CGIT_CONFIG="$PWD/tabledrc" QUERY_STRING="url=foo/commit" cgit >tmp 2>err &&+	grep "Status: 500" tmp &&+	grep "Lua error in .*tabled.lua: (error object is a table value)" err+'+ # A script runs its top level before cgit hooks stdout for it, so what it # writes there goes straight onto the page instead of tripping the hook. test_expect_success CGIT_LUA 'a lua filter that writes while loading still renders' '
diff --git a/tests/t0201-filters.sh b/tests/t0201-filters.sh
index c2d0ec4..1ffb973 100755
--- a/tests/t0201-filters.sh
+++ b/tests/t0201-filters.sh
@@ -130,6 +130,19 @@ test_expect_success CGIT_LUA 'a failing lua filter still renders an error page'
 	grep "Lua error in" err
 '
 
+# A script may raise a value that is no string, which the log still names.
+test_expect_success CGIT_LUA 'a lua filter raising a table is named by type' '
+	cat >tabled.lua <<-\EOF &&
+	function filter_open(...) error({}) end
+	function filter_write(str) end
+	function filter_close() return 0 end
+	EOF
+	sed "s|broken.lua|tabled.lua|" brokenrc >tabledrc &&
+	CGIT_CONFIG="$PWD/tabledrc" QUERY_STRING="url=foo/commit" cgit >tmp 2>err &&
+	grep "Status: 500" tmp &&
+	grep "Lua error in .*tabled.lua: (error object is a table value)" err
+'
+
 # A script runs its top level before cgit hooks stdout for it, so what it
 # writes there goes straight onto the page instead of tripping the hook.
 test_expect_success CGIT_LUA 'a lua filter that writes while loading still renders' '