Skip to content

Implement support for Lua 5.1 #1814

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 1 commit into from
Closed

Implement support for Lua 5.1 #1814

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 4 additions & 14 deletions build/lua.m4
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ AC_DEFUN([CHECK_LUA],
[dnl

# Possible names for the lua library/package (pkg-config)
LUA_POSSIBLE_LIB_NAMES="lua lua53 lua5.3 lua52 lua5.2"
LUA_POSSIBLE_LIB_NAMES="lua lua53 lua5.3 lua52 lua5.2 lua51 lua5.1"

# Possible extensions for the library
LUA_POSSIBLE_EXTENSIONS="so so0 la sl dll dylib so.0.0.0"
Expand Down Expand Up @@ -88,10 +88,6 @@ if test -z "${LUA_CFLAGS}"; then
LUA_FOUND=-1
fi
else
if test "${lua_5_1}" = 1 && test "x${LUA_MANDATORY}" == "xyes" ; then
AC_MSG_ERROR([LUA was explicitly referenced but LUA v5.1 was found and it is not currently supported on libModSecurity. LUA_VERSION: ${LUA_VERSION}])
LUA_FOUND=-1
fi
if test -z "${LUA_MANDATORY}" || test "x${LUA_MANDATORY}" == "xno"; then
LUA_FOUND=1
AC_MSG_NOTICE([using LUA ${LUA_LDADD}])
Expand All @@ -113,15 +109,6 @@ else
fi
fi

if test "${lua_5_1}" = 1 ; then
AC_MSG_NOTICE([LUA 5.1 was found and it is not currently supported on libModSecurity. LUA_VERSION: ${LUA_VERSION}. LUA build disabled.])
LUA_FOUND=2
LUA_CFLAGS=
LUA_DISPLAY=
LUA_LDADD=
LUA_LDFLAGS=
fi

AC_SUBST(LUA_FOUND)

]) # AC_DEFUN [CHECK_LUA]
Expand Down Expand Up @@ -179,6 +166,9 @@ AC_DEFUN([CHECK_FOR_LUA_AT], [
elif test -e "${path}/include/lua5.2/lua.h"; then
lua_inc_path="${path}/include/lua5.2"
LUA_VERSION=502
elif test -e "${path}/include/lua5.1/lua.h"; then
lua_inc_path="${path}/include/lua5.1"
LUA_VERSION=501
fi

if test -n "${lua_lib_path}"; then
Expand Down
12 changes: 11 additions & 1 deletion src/engine/lua.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@ bool Lua::load(std::string script, std::string *err) {
return false;
}

#ifdef WITH_LUA_5_2
#if defined (WITH_LUA_5_2) || defined (WITH_LUA_5_1)
if (lua_dump(L, Lua::blob_keeper, reinterpret_cast<void *>(&m_blob))) {
#else
if (lua_dump(L, Lua::blob_keeper, reinterpret_cast<void *>(&m_blob), 0)) {
Expand Down Expand Up @@ -138,8 +138,12 @@ int Lua::run(Transaction *t) {
luaL_setfuncs(L, mscLuaLib, 0);
lua_setglobal(L, "m");

#ifdef WITH_LUA_5_1
int rc = lua_load(L, Lua::blob_reader, &m_blob, m_scriptName.c_str());
#else
int rc = lua_load(L, Lua::blob_reader, &m_blob, m_scriptName.c_str(),
NULL);
#endif
if (rc != LUA_OK) {
std::string e;
e.assign("Failed to execute lua script: " + m_scriptName + ". ");
Expand All @@ -150,9 +154,11 @@ int Lua::run(Transaction *t) {
case LUA_ERRMEM:
e.assign("Memory error. ");
break;
#ifndef WITH_LUA_5_1
case LUA_ERRGCMM:
e.assign("Garbage Collector error. ");
break;
#endif
}
e.append(lua_tostring(L, -1));
#ifndef NO_LOGS
Expand Down Expand Up @@ -402,7 +408,11 @@ std::string Lua::applyTransformations(lua_State *L, Transaction *t,

if (lua_istable(L, idx)) {
const char *name = NULL;
#ifdef WITH_LUA_5_1
int i, n = lua_objlen(L, idx);
#else
int i, n = lua_rawlen(L, idx);
#endif

for (i = 1; i <= n; i++) {
lua_rawgeti(L, idx, i);
Expand Down
25 changes: 25 additions & 0 deletions src/engine/lua.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -101,4 +101,29 @@ static const struct luaL_Reg mscLuaLib[] = {
} // namespace engine
} // namespace modsecurity

#ifdef WITH_LUA
#if defined LUA_VERSION_NUM && LUA_VERSION_NUM < 502
/*
** Adapted from Lua 5.2.0
*/
#define LUA_OK 0

static void luaL_setfuncs(lua_State *L, const luaL_Reg *l, int nup) {
luaL_checkstack(L, nup + 1, "too many upvalues");

for (; l->name != NULL; l++) { /* fill the table with given functions */
int i;
lua_pushstring(L, l->name);
for (i = 0; i < nup; i++) /* copy upvalues to the top */
lua_pushvalue(L, -(nup + 1));
lua_pushcclosure(L, l->func, nup); /* closure with those upvalues */
lua_settable(L, -(nup + 3));
}

lua_pop(L, nup); /* remove upvalues */
}
#endif
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe the intent is to eventually promote support for LuaJIT with this PR (totally speculating here), but if so, and if I recall correctly, some versions of LuaJIT define LUA_VERSION_NUM as 501 and implement luaL_setfuncs. Should we follow Mike's advice here? See:

https://www.freelists.org/post/luajit/ANN-LuaJIT210beta3,3

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If this isn't a goal nor a long-term goal of this PR, then feel free to ignore this!

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey @thibaultcha, the goal here is just to get support for 5.1 introduced. Trying to integrate with OpenResty was a bit of an exercise (see #1809), and there's no clean way to do it at this point (it was also the initial impetus for this PR). Eventually LuaJIT support might be nice, but since a number of distros only provide Lua lib packages for 5.1 out of the box, getting 5.1 supported is the sole goal here. As such I'd rather not introduce new symbols unnecessarily and increase the scope of this change.

If there's a large push for LuaJIT support here, perhaps we can revisit in the future, but at this point I do not see much value for that.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All good 👍

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's an attempt to support LuaJIT on some distros at #1854. You might want to give it a try to see if it works for you @thibaultcha.

#endif


#endif // SRC_ENGINE_LUA_H_