[PATCH (3.5) 0/3]: Catching template syntax errors at config time
Following up on the previous patch that added a check to the file() and pipe() destinations, allowing them to catch and report template syntax errors at config time, following this mail are three others, that do similar things to the set() and subst() rewrite functions, to the pair() statement of value-pairs() and to templatable settings in afamqp. These catch these issues: rewrite s_broken { set("$(+ 0", value(MESSAGE)); subst("original", "$(+ 0", value(MESSAGE)); }; destination d_broken { afamqp(body("$(+ 0") routing-key("$(+ 0") value-pairs(pair("INVALID", "$(+ 0"))); }; There are a couple of more cases in the codebase where the same issue is present, namely afsmtp() (the subject, body and header templates), afsql (fields and table name), and perhaps in patterndb too, I have not throughly inspected that one yet. -- |8]
If there is a syntax error in the template part of set() or subst(), catch it at config time, and report it appropriately. We do this by adding an error output variable to the constructors, and checking the constructors' return value in the grammar. Signed-off-by: Gergely Nagy <algernon@balabit.hu> --- lib/rewrite/rewrite-expr-grammar.ym | 12 +++++++++--- lib/rewrite/rewrite-set.c | 16 ++++++++++++---- lib/rewrite/rewrite-set.h | 2 +- lib/rewrite/rewrite-subst.c | 15 +++++++++++---- lib/rewrite/rewrite-subst.h | 2 +- 5 files changed, 34 insertions(+), 13 deletions(-) diff --git a/lib/rewrite/rewrite-expr-grammar.ym b/lib/rewrite/rewrite-expr-grammar.ym index c28e4c8..99adddc 100644 --- a/lib/rewrite/rewrite-expr-grammar.ym +++ b/lib/rewrite/rewrite-expr-grammar.ym @@ -1,5 +1,5 @@ /* - * Copyright (c) 2002-2012 BalaBit IT Ltd, Budapest, Hungary + * Copyright (c) 2002-2013 BalaBit IT Ltd, Budapest, Hungary * Copyright (c) 1998-2012 Balázs Scheidler * * This library is free software; you can redistribute it and/or @@ -76,7 +76,10 @@ rewrite_expr_list rewrite_expr : KW_SUBST '(' string string { - last_rewrite = log_rewrite_subst_new($4); + GError *error; + + last_rewrite = log_rewrite_subst_new($4, &error); + CHECK_ERROR(last_rewrite, @4, "Error compiling template; %s", error->message); free($4); } rewrite_subst_opts ')' @@ -87,7 +90,10 @@ rewrite_expr } | KW_SET '(' string { - last_rewrite = log_rewrite_set_new($3); + GError *error; + + last_rewrite = log_rewrite_set_new($3, &error); + CHECK_ERROR(last_rewrite, @3, "Error compiling template; %s", error->message); free($3); } rewrite_expr_opts ')' { $$ = last_rewrite; } diff --git a/lib/rewrite/rewrite-set.c b/lib/rewrite/rewrite-set.c index 978e0fa..69b93f8 100644 --- a/lib/rewrite/rewrite-set.c +++ b/lib/rewrite/rewrite-set.c @@ -56,7 +56,7 @@ log_rewrite_set_clone(LogPipe *s) LogRewriteSet *self = (LogRewriteSet *) s; LogRewriteSet *cloned; - cloned = (LogRewriteSet *) log_rewrite_set_new(self->value_template->template); + cloned = (LogRewriteSet *) log_rewrite_set_new(self->value_template->template, NULL); cloned->super.value_handle = self->super.value_handle; cloned->super.condition = self->super.condition; return &cloned->super.super; @@ -72,16 +72,24 @@ log_rewrite_set_free(LogPipe *s) } LogRewrite * -log_rewrite_set_new(const gchar *new_value) +log_rewrite_set_new(const gchar *new_value, GError **error) { LogRewriteSet *self = g_new0(LogRewriteSet, 1); + LogTemplate *value_template; + + value_template = log_template_new(configuration, NULL); + if (!log_template_compile(value_template, new_value, error)) + { + log_template_unref(value_template); + return NULL; + } log_rewrite_init(&self->super); self->super.super.free_fn = log_rewrite_set_free; self->super.super.clone = log_rewrite_set_clone; self->super.process = log_rewrite_set_process; - self->value_template = log_template_new(configuration, NULL); - log_template_compile(self->value_template, new_value, NULL); + self->value_template = value_template; + return &self->super; } diff --git a/lib/rewrite/rewrite-set.h b/lib/rewrite/rewrite-set.h index 72de581..e3427d8 100644 --- a/lib/rewrite/rewrite-set.h +++ b/lib/rewrite/rewrite-set.h @@ -28,6 +28,6 @@ #include "rewrite/rewrite-expr.h" /* LogRewriteSet */ -LogRewrite *log_rewrite_set_new(const gchar *new_value); +LogRewrite *log_rewrite_set_new(const gchar *new_value, GError **error); #endif diff --git a/lib/rewrite/rewrite-subst.c b/lib/rewrite/rewrite-subst.c index 86a9447..937a4a2 100644 --- a/lib/rewrite/rewrite-subst.c +++ b/lib/rewrite/rewrite-subst.c @@ -101,7 +101,7 @@ log_rewrite_subst_clone(LogPipe *s) LogRewriteSubst *self = (LogRewriteSubst *) s; LogRewriteSubst *cloned; - cloned = (LogRewriteSubst *) log_rewrite_subst_new(self->replacement->template); + cloned = (LogRewriteSubst *) log_rewrite_subst_new(self->replacement->template, NULL); cloned->matcher = log_matcher_ref(self->matcher); cloned->super.value_handle = self->super.value_handle; cloned->super.condition = self->super.condition; @@ -120,9 +120,17 @@ log_rewrite_subst_free(LogPipe *s) } LogRewrite * -log_rewrite_subst_new(const gchar *replacement) +log_rewrite_subst_new(const gchar *replacement, GError **error) { LogRewriteSubst *self = g_new0(LogRewriteSubst, 1); + LogTemplate *replacement_template; + + replacement_template = log_template_new(configuration, NULL); + if (!log_template_compile(replacement_template, replacement, error)) + { + log_template_unref(replacement_template); + return NULL; + } log_rewrite_init(&self->super); @@ -130,7 +138,6 @@ log_rewrite_subst_new(const gchar *replacement) self->super.super.clone = log_rewrite_subst_clone; self->super.process = log_rewrite_subst_process; - self->replacement = log_template_new(configuration, NULL); - log_template_compile(self->replacement, replacement, NULL); + self->replacement = replacement_template; return &self->super; } diff --git a/lib/rewrite/rewrite-subst.h b/lib/rewrite/rewrite-subst.h index 4d4ddc3..a2aecbd 100644 --- a/lib/rewrite/rewrite-subst.h +++ b/lib/rewrite/rewrite-subst.h @@ -33,6 +33,6 @@ gboolean log_rewrite_subst_set_regexp(LogRewrite *s, const gchar *regexp); void log_rewrite_subst_set_matcher(LogRewrite *s, LogMatcher *matcher); void log_rewrite_subst_set_flags(LogRewrite *s, gint flags); -LogRewrite *log_rewrite_subst_new(const gchar *replacement); +LogRewrite *log_rewrite_subst_new(const gchar *replacement, GError **error); #endif -- 1.7.10.4
Instead of allowing syntax errors in templates to trigger at runtime, catch them at config time by verifying the return value of log_template_compile() and propagating the error message upwards. Signed-off-by: Gergely Nagy <algernon@balabit.hu> --- lib/cfg-grammar.y | 20 ++++++++++++++++++-- lib/value-pairs.c | 27 ++++++++++++++++++++------- lib/value-pairs.h | 7 ++++--- 3 files changed, 42 insertions(+), 12 deletions(-) diff --git a/lib/cfg-grammar.y b/lib/cfg-grammar.y index 9505c29..0d6ab2e 100644 --- a/lib/cfg-grammar.y +++ b/lib/cfg-grammar.y @@ -1066,8 +1066,24 @@ vp_options ; vp_option - : KW_PAIR '(' string ':' string ')' { value_pairs_add_pair(last_value_pairs, configuration, $3, $5); free($3); free($5); } - | KW_PAIR '(' string string ')' { value_pairs_add_pair(last_value_pairs, configuration, $3, $4); free($3); free($4); } + : KW_PAIR '(' string ':' string ')' + { + GError *error; + + CHECK_ERROR(value_pairs_add_pair(last_value_pairs, configuration, $3, $5, &error), + @5, "Error compiling template; %s", error->message); + free($3); + free($5); + } + | KW_PAIR '(' string string ')' + { + GError *error; + + CHECK_ERROR(value_pairs_add_pair(last_value_pairs, configuration, $3, $4, &error), + @4, "Error compiling template; %s", error->message); + free($3); + free($4); + } | KW_KEY '(' string KW_REKEY '(' { last_vp_transset = value_pairs_transform_set_new($3); diff --git a/lib/value-pairs.c b/lib/value-pairs.c index 689293c..0a53dc7 100644 --- a/lib/value-pairs.c +++ b/lib/value-pairs.c @@ -151,16 +151,28 @@ value_pairs_add_glob_pattern(ValuePairs *vp, const gchar *pattern, vp->patterns[i] = p; } -void -value_pairs_add_pair(ValuePairs *vp, GlobalConfig *cfg, const gchar *key, const gchar *value) +gboolean +value_pairs_add_pair(ValuePairs *vp, GlobalConfig *cfg, const gchar *key, + const gchar *value, GError **error) { - VPPairConf *p = g_new(VPPairConf, 1); + VPPairConf *p; + LogTemplate *template; + + template = log_template_new(cfg, NULL); + if (!log_template_compile(template, value, error)) + { + log_template_unref(template); + return FALSE; + } + + p = g_new(VPPairConf, 1); p->name = g_strdup(key); - p->template = log_template_new(cfg, NULL); - log_template_compile(p->template, value, NULL); + p->template = template; g_ptr_array_add(vp->vpairs, p); + + return TRUE; } static gchar * @@ -730,6 +742,7 @@ vp_cmdline_parse_pair (const gchar *option_name, const gchar *value, ValuePairs *vp = (ValuePairs *) args[1]; GlobalConfig *cfg = (GlobalConfig *) args[0]; gchar **kv; + gboolean success; vp_cmdline_parse_rekey_finish (data); if (!g_strstr_len (value, strlen (value), "=")) @@ -740,13 +753,13 @@ vp_cmdline_parse_pair (const gchar *option_name, const gchar *value, } kv = g_strsplit(value, "=", 2); - value_pairs_add_pair (vp, cfg, kv[0], kv[1]); + success = value_pairs_add_pair (vp, cfg, kv[0], kv[1], error); g_free (kv[0]); g_free (kv[1]); g_free (kv); - return TRUE; + return success; } static ValuePairsTransformSet * diff --git a/lib/value-pairs.h b/lib/value-pairs.h index a6c9f34..bddddb5 100644 --- a/lib/value-pairs.h +++ b/lib/value-pairs.h @@ -1,6 +1,6 @@ /* - * Copyright (c) 2011-2012 BalaBit IT Ltd, Budapest, Hungary - * Copyright (c) 2011-2012 Gergely Nagy <algernon@balabit.hu> + * Copyright (c) 2011-2013 BalaBit IT Ltd, Budapest, Hungary + * Copyright (c) 2011-2013 Gergely Nagy <algernon@balabit.hu> * * This library is free software; you can redistribute it and/or * modify it under the terms of the GNU Lesser General Public @@ -41,7 +41,8 @@ typedef gboolean (*VPWalkCallbackFunc)(const gchar *name, gboolean value_pairs_add_scope(ValuePairs *vp, const gchar *scope); void value_pairs_add_glob_pattern(ValuePairs *vp, const gchar *pattern, gboolean include); -void value_pairs_add_pair(ValuePairs *vp, GlobalConfig *cfg, const gchar *key, const gchar *value); +gboolean value_pairs_add_pair(ValuePairs *vp, GlobalConfig *cfg, const gchar *key, + const gchar *value, GError **error); void value_pairs_add_transforms(ValuePairs *vp, gpointer vpts); -- 1.7.10.4
Teach the AMQP destination to catch template syntax errors at config time, by adding an output error variable and a return value to afamqp_dd_set_routing_key() and afamqp_dd_set_body(), and checking the return value of log_template_compile(), and propagating error messages upwards. Signed-off-by: Gergely Nagy <algernon@balabit.hu> --- modules/afamqp/afamqp-grammar.ym | 22 ++++++++++++++++++---- modules/afamqp/afamqp.c | 14 +++++++------- modules/afamqp/afamqp.h | 8 ++++---- 3 files changed, 29 insertions(+), 15 deletions(-) diff --git a/modules/afamqp/afamqp-grammar.ym b/modules/afamqp/afamqp-grammar.ym index a336d5d..0adf6bb 100644 --- a/modules/afamqp/afamqp-grammar.ym +++ b/modules/afamqp/afamqp-grammar.ym @@ -1,7 +1,7 @@ /* * Copyright (c) 2012 Nagy, Attila <bra@fsn.hu> - * Copyright (c) 2012 BalaBit IT Ltd, Budapest, Hungary - * Copyright (c) 2012 Gergely Nagy <algernon@balabit.hu> + * Copyright (c) 2012-2013 BalaBit IT Ltd, Budapest, Hungary + * Copyright (c) 2012-2013 Gergely Nagy <algernon@balabit.hu> * * This program is free software; you can redistribute it and/or modify it * under the terms of the GNU General Public License version 2 as published @@ -80,8 +80,22 @@ afamqp_option | KW_EXCHANGE '(' string ')' { afamqp_dd_set_exchange(last_driver, $3); free($3); } | KW_EXCHANGE_DECLARE '(' yesno ')' { afamqp_dd_set_exchange_declare(last_driver, $3); } | KW_EXCHANGE_TYPE '(' string ')' { afamqp_dd_set_exchange_type(last_driver, $3); free($3); } - | KW_ROUTING_KEY '(' string ')' { afamqp_dd_set_routing_key(last_driver, $3); free($3); } - | KW_BODY '(' string ')' { afamqp_dd_set_body(last_driver, $3); free($3); } + | KW_ROUTING_KEY '(' string ')' + { + GError *error; + + CHECK_ERROR(afamqp_dd_set_routing_key(last_driver, $3, &error), @3, + "Error compiling template; %s", error->message); + free($3); + } + | KW_BODY '(' string ')' + { + GError *error; + + CHECK_ERROR(afamqp_dd_set_body(last_driver, $3, &error), @3, + "Error compiling template; %s", error->message); + free($3); + } | KW_PERSISTENT '(' yesno ')' { afamqp_dd_set_persistent(last_driver, $3); } | KW_USERNAME '(' string ')' { afamqp_dd_set_user(last_driver, $3); free($3); } | KW_PASSWORD '(' string ')' { afamqp_dd_set_password(last_driver, $3); free($3); } diff --git a/modules/afamqp/afamqp.c b/modules/afamqp/afamqp.c index 9a6c07d..315335c 100644 --- a/modules/afamqp/afamqp.c +++ b/modules/afamqp/afamqp.c @@ -156,22 +156,22 @@ afamqp_dd_set_exchange_type(LogDriver *d, const gchar *exchange_type) self->exchange_type = g_strdup(exchange_type); } -void -afamqp_dd_set_routing_key(LogDriver *d, const gchar *routing_key) +gboolean +afamqp_dd_set_routing_key(LogDriver *d, const gchar *routing_key, GError **error) { AMQPDestDriver *self = (AMQPDestDriver *) d; - log_template_compile(self->routing_key_template, routing_key, NULL); + return log_template_compile(self->routing_key_template, routing_key, error); } -void -afamqp_dd_set_body(LogDriver *d, const gchar *body) +gboolean +afamqp_dd_set_body(LogDriver *d, const gchar *body, GError **error) { AMQPDestDriver *self = (AMQPDestDriver *) d; if (!self->body_template) self->body_template = log_template_new(configuration, NULL); - log_template_compile(self->body_template, body, NULL); + return log_template_compile(self->body_template, body, error); } void @@ -698,7 +698,7 @@ afamqp_dd_new(void) afamqp_dd_set_port((LogDriver *) self, 5672); afamqp_dd_set_exchange((LogDriver *) self, "syslog"); afamqp_dd_set_exchange_type((LogDriver *) self, "fanout"); - afamqp_dd_set_routing_key((LogDriver *) self, ""); + afamqp_dd_set_routing_key((LogDriver *) self, "", NULL); afamqp_dd_set_persistent((LogDriver *) self, TRUE); afamqp_dd_set_exchange_declare((LogDriver *) self, FALSE); diff --git a/modules/afamqp/afamqp.h b/modules/afamqp/afamqp.h index 1247a25..5876231 100644 --- a/modules/afamqp/afamqp.h +++ b/modules/afamqp/afamqp.h @@ -1,7 +1,7 @@ /* * Copyright (c) 2012 Nagy, Attila <bra@fsn.hu> - * Copyright (c) 2012 BalaBit IT Ltd, Budapest, Hungary - * Copyright (c) 2012 Gergely Nagy <algernon@balabit.hu> + * Copyright (c) 2012-2013 BalaBit IT Ltd, Budapest, Hungary + * Copyright (c) 2012-2013 Gergely Nagy <algernon@balabit.hu> * * This program is free software; you can redistribute it and/or modify it * under the terms of the GNU General Public License version 2 as published @@ -36,8 +36,8 @@ void afamqp_dd_set_exchange(LogDriver *d, const gchar *database); void afamqp_dd_set_exchange_declare(LogDriver *d, gboolean declare); void afamqp_dd_set_exchange_type(LogDriver *d, const gchar *exchange_type); void afamqp_dd_set_vhost(LogDriver *d, const gchar *vhost); -void afamqp_dd_set_routing_key(LogDriver *d, const gchar *routing_key); -void afamqp_dd_set_body(LogDriver *d, const gchar *body); +gboolean afamqp_dd_set_routing_key(LogDriver *d, const gchar *routing_key, GError **error); +gboolean afamqp_dd_set_body(LogDriver *d, const gchar *body, GError **error); void afamqp_dd_set_persistent(LogDriver *d, gboolean persistent); void afamqp_dd_set_user(LogDriver *d, const gchar *user); void afamqp_dd_set_password(LogDriver *d, const gchar *password); -- 1.7.10.4
participants (1)
-
Gergely Nagy