[Tarantool-patches] [PATCH] lua: fix panic in case when log.cfg.log incorrecly specified
olegrok at tarantool.org
olegrok at tarantool.org
Wed Sep 9 11:19:55 MSK 2020
From: Oleg Babin <babinoleg at mail.ru>
This patch makes log.cfg{log = ...} behaviour the same as in
box.cfg{log = ...} and fixes panic if "log" is incorrectly
specified. For such purpose we export "say_parse_logger_type"
function and use for logger type validation and logger type
parsing.
Closes #5130
---
Issue: https://github.com/tarantool/tarantool/issues/5130
Branch: https://github.com/tarantool/tarantool/tree/olegrok/5130-panic-on-invalid-log
src/exports.h | 1 +
src/lua/log.lua | 28 ++++++++++++++++---
.../gh-5130-panic-on-invalid-log.test.lua | 23 +++++++++++++++
3 files changed, 48 insertions(+), 4 deletions(-)
create mode 100755 test/app-tap/gh-5130-panic-on-invalid-log.test.lua
diff --git a/src/exports.h b/src/exports.h
index 7f0601f4f..6d8303180 100644
--- a/src/exports.h
+++ b/src/exports.h
@@ -434,6 +434,7 @@ EXPORT(_say)
EXPORT(say_logger_init)
EXPORT(say_logger_initialized)
EXPORT(say_logrotate)
+EXPORT(say_parse_logger_type)
EXPORT(say_set_log_format)
EXPORT(say_set_log_level)
EXPORT(SHA1internal)
diff --git a/src/lua/log.lua b/src/lua/log.lua
index cfd83f592..62ea61f2d 100644
--- a/src/lua/log.lua
+++ b/src/lua/log.lua
@@ -54,6 +54,9 @@ ffi.cdef[[
pid_t log_pid;
extern int log_level;
extern int log_format;
+
+ int
+ say_parse_logger_type(const char **str, enum say_logger_type *type);
]]
local S_WARN = ffi.C.S_WARN
@@ -193,6 +196,20 @@ local function verify_static(k, v)
return true
end
+local function parse_format(log)
+ -- There is no easy way to get pointer to ponter via FFI
+ local str_p = ffi.new('const char*[1]')
+ str_p[0] = ffi.cast('char*', log)
+ local logger_type = ffi.new('enum say_logger_type[1]')
+ local rc = ffi.C.say_parse_logger_type(str_p, logger_type)
+
+ if rc ~= 0 then
+ box.error()
+ end
+
+ return logger_type[0]
+end
+
-- Test if format is valid.
local function verify_format(key, name, cfg)
assert(log_cfg[key] ~= nil)
@@ -213,9 +230,7 @@ local function verify_format(key, name, cfg)
-- The good thing that we're only needed log
-- entry which is the same key for both.
if cfg ~= nil and cfg['log'] ~= nil then
- if string.startswith(cfg['log'], "syslog:") then
- log_type = ffi.C.SAY_LOGGER_SYSLOG
- end
+ log_type = parse_format(cfg['log'])
end
if fmt_str2num[name] == ffi.C.SF_JSON then
@@ -465,7 +480,7 @@ local function load_cfg(self, cfg)
-- log option might be zero length string, which
-- is fine, we should treat it as nil.
if cfg.log ~= nil then
- if type(cfg.log) == 'string' and cfg.log:len() == 0 then
+ if type(cfg.log) ~= 'string' or cfg.log:len() == 0 then
cfg.log = nil
end
end
@@ -511,6 +526,11 @@ local function load_cfg(self, cfg)
nonblock = 1
end
+ -- Parsing for validation purposes
+ if cfg.log ~= nil then
+ parse_format(cfg.log)
+ end
+
-- We never allow confgure the logger in background
-- mode since we don't know how the box will be configured
-- later.
diff --git a/test/app-tap/gh-5130-panic-on-invalid-log.test.lua b/test/app-tap/gh-5130-panic-on-invalid-log.test.lua
new file mode 100755
index 000000000..24af8139c
--- /dev/null
+++ b/test/app-tap/gh-5130-panic-on-invalid-log.test.lua
@@ -0,0 +1,23 @@
+#!/usr/bin/env tarantool
+
+local test = require('tap').test('gh-5130')
+local log = require('log')
+test:plan(3)
+
+--
+-- gh-5130: panic on invalid "log"
+--
+
+-- Invalid log. No panic, just error
+_, err = pcall(log.cfg, {log=' :invalid'})
+test:like(err, "expecting a file name or a prefix")
+
+-- Empty string - default log (to be compatible with box.cfg)
+ok = pcall(log.cfg, {log=''})
+test:ok(ok)
+
+-- Dynamic reconfiguration - error, no info about invalid logger type
+_, err = pcall(log.cfg, {log=' :invalid'})
+test:like(err, "can't be set dynamically")
+
+os.exit(test:check() and 0 or 1)
--
2.23.0
More information about the Tarantool-patches
mailing list