From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtp10.mail.ru (smtp10.mail.ru [94.100.181.92]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by dev.tarantool.org (Postfix) with ESMTPS id E2D94469719 for ; Wed, 9 Sep 2020 11:19:56 +0300 (MSK) From: olegrok@tarantool.org Date: Wed, 9 Sep 2020 11:19:55 +0300 Message-Id: <20200909081955.45791-1-olegrok@tarantool.org> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit Subject: [Tarantool-patches] [PATCH] lua: fix panic in case when log.cfg.log incorrecly specified List-Id: Tarantool development patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: gorcunov@gmail.com Cc: tarantool-patches@dev.tarantool.org From: Oleg Babin 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