Skip to content
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.

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

Fix error reporting of templates #3660

Merged
merged 17 commits into from
May 4, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
4 changes: 1 addition & 3 deletions lib/filter/tests/test_filters_common.c
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,7 @@ create_template(const gchar *template)
LogTemplate *t;

t = log_template_new(configuration, NULL);
log_template_compile(t, template, NULL);
cr_assert(log_template_compile(t, template, NULL));
return t;
}

Expand Down Expand Up @@ -240,5 +240,3 @@ teardown(void)
scratch_buffers_explicit_gc();
app_shutdown();
}


2 changes: 1 addition & 1 deletion lib/rewrite/tests/test_set_facility.c
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ _create_template(const gchar *str)
{
GError *error = NULL;
LogTemplate *template = log_template_new(cfg, NULL);
log_template_compile(template, str, &error);
cr_assert(log_template_compile(template, str, &error));

cr_expect_null(error);

Expand Down
2 changes: 1 addition & 1 deletion lib/rewrite/tests/test_set_pri.c
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ _create_template(const gchar *str)
{
GError *error = NULL;
LogTemplate *template = log_template_new(cfg, NULL);
log_template_compile(template, str, &error);
cr_assert(log_template_compile(template, str, &error));

cr_expect_null(error);

Expand Down
2 changes: 1 addition & 1 deletion lib/rewrite/tests/test_set_severity.c
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ _create_template(const gchar *str)
{
GError *error = NULL;
LogTemplate *template = log_template_new(cfg, NULL);
log_template_compile(template, str, &error);
cr_assert(log_template_compile(template, str, &error));

cr_expect_null(error);

Expand Down
7 changes: 7 additions & 0 deletions lib/template/repr.h
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
#define TEMPLATE_REPR_H_INCLUDED

#include "template/function.h"
#include "template/macros.h"
#include "logmsg/logmsg.h"

enum
Expand Down Expand Up @@ -61,6 +62,12 @@ LogTemplateElem *log_template_elem_new_func(LogTemplate *template,
const gchar *text, gint argc, gchar *argv[], gint msg_ref,
GError **error);

static inline gboolean
log_template_elem_is_literal_string(const LogTemplateElem *self)
{
return self->type == LTE_MACRO && self->macro == M_NONE;
}

void log_template_elem_free_list(GList *el);


Expand Down
46 changes: 39 additions & 7 deletions lib/template/templates.c
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,34 @@
#include "template/repr.h"
#include "cfg.h"

gboolean
log_template_is_literal_string(const LogTemplate *self)
{
if (!self->compiled_template)
return TRUE;

if (self->escape || self->compiled_template->next)
return FALSE;

return log_template_elem_is_literal_string((LogTemplateElem *) self->compiled_template->data);
}

const gchar *
log_template_get_literal_value(const LogTemplate *self, gssize *value_len)
{
g_assert(log_template_is_literal_string(self));

if (!self->compiled_template)
return "";

LogTemplateElem *e = (LogTemplateElem *) self->compiled_template->data;

if (value_len)
*value_len = e->text_len;

return e->text;
}

gboolean
log_template_is_trivial(LogTemplate *self)
{
Expand All @@ -40,6 +68,9 @@ log_template_get_trivial_value(LogTemplate *self, LogMessage *msg, gssize *value
{
g_assert(self->trivial);

if (!self->compiled_template)
return "";

LogTemplateElem *e = (LogTemplateElem *) self->compiled_template->data;

switch (e->type)
Expand Down Expand Up @@ -70,30 +101,29 @@ _calculate_triviality(LogTemplate *self)
if (self->escape)
return FALSE;

/* no compiled template */
/* empty templates are trivial */
if (self->compiled_template == NULL)
return FALSE;
return TRUE;

/* more than one element */
if (self->compiled_template->next != NULL)
return FALSE;

LogTemplateElem *e = (LogTemplateElem *) self->compiled_template->data;
const LogTemplateElem *e = (LogTemplateElem *) self->compiled_template->data;

/* reference to non-last element of the context, that's not trivial */
if (e->msg_ref > 0)
return FALSE;

if (log_template_elem_is_literal_string(e))
return TRUE;

switch (e->type)
{
case LTE_FUNC:
/* functions are never trivial */
return FALSE;
case LTE_MACRO:
/* Macros are trivial if they only contain a text but not a real
* macro. Empty strings are represented this way. */
if (e->macro == M_NONE)
return TRUE;
if (e->text_len > 0)
return FALSE;

Expand Down Expand Up @@ -148,6 +178,8 @@ log_template_compile_literal_string(LogTemplate *self, const gchar *literal)
self->template = g_strdup(literal);
self->compiled_template = g_list_append(self->compiled_template,
log_template_elem_new_macro(literal, M_NONE, NULL, 0));

self->trivial = _calculate_triviality(self);
}

void
Expand Down
2 changes: 2 additions & 0 deletions lib/template/templates.h
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,8 @@ void log_template_set_escape(LogTemplate *self, gboolean enable);
gboolean log_template_set_type_hint(LogTemplate *self, const gchar *hint, GError **error);
gboolean log_template_compile(LogTemplate *self, const gchar *template, GError **error);
void log_template_compile_literal_string(LogTemplate *self, const gchar *literal);
gboolean log_template_is_literal_string(const LogTemplate *self);
const gchar *log_template_get_literal_value(const LogTemplate *self, gssize *value_len);
gboolean log_template_is_trivial(LogTemplate *self);
const gchar *log_template_get_trivial_value(LogTemplate *self, LogMessage *msg, gssize *value_len);
void log_template_set_name(LogTemplate *self, const gchar *name);
Expand Down
119 changes: 103 additions & 16 deletions lib/template/tests/test_template.c
Original file line number Diff line number Diff line change
Expand Up @@ -349,35 +349,122 @@ Test(template, test_template_function_args)
"49 50 51 52 53 54 55 56 57 58 59 60 61 62 63 64");
}

static void
assert_template_trivial_value(const gchar *template_code, LogMessage *msg, const gchar *expected_value)
{
LogTemplate *template = compile_template(template_code, FALSE);

cr_assert(log_template_is_trivial(template));

const gchar *trivial_value = log_template_get_trivial_value(template, msg, NULL);
cr_assert_str_eq(trivial_value, expected_value);

GString *formatted_value = g_string_sized_new(64);
log_template_format(template, msg, &DEFAULT_TEMPLATE_EVAL_OPTIONS, formatted_value);
cr_assert_str_eq(trivial_value, formatted_value->str,
"Formatted and trivial value does not match: '%s' - '%s'", trivial_value, formatted_value->str);

g_string_free(formatted_value, TRUE);
log_template_unref(template);
}


Test(template, test_single_values_and_literal_strings_are_considered_trivial)
{
LogTemplate *template;
LogMessage *msg = create_sample_message();

template = compile_template("literal", FALSE);
cr_assert(log_template_is_trivial(template));
cr_assert_str_eq(log_template_get_trivial_value(template, msg, NULL), "literal");
assert_template_trivial_value("", msg, "");
assert_template_trivial_value(" ", msg, " ");
assert_template_trivial_value("literal", msg, "literal");
assert_template_trivial_value("$1", msg, "first-match");
assert_template_trivial_value("$MSG", msg, "árvíztűrőtükörfúrógép");
assert_template_trivial_value("$HOST", msg, "bzorp");
assert_template_trivial_value("${APP.VALUE}", msg, "value");

LogTemplate *template = log_template_new(configuration, NULL);
cr_assert_not(log_template_compile(template, "$1 $2 ${MSG invalid", NULL));
cr_assert(log_template_is_trivial(template), "Invalid templates are trivial");
cr_assert(g_str_has_prefix(log_template_get_trivial_value(template, NULL, NULL), "error in template"));
log_template_unref(template);

template = compile_template("$1", FALSE);
cr_assert(log_template_is_trivial(template));
cr_assert_str_eq(log_template_get_trivial_value(template, msg, NULL), "first-match");
log_msg_unref(msg);
}

Test(template, test_non_trivial_templates)
{
LogTemplate *template;

template = compile_template("$1", TRUE);
cr_assert_not(log_template_is_trivial(template), "Escaped template is not trivial");
log_template_unref(template);

template = compile_template("$MSG", FALSE);
cr_assert(log_template_is_trivial(template));
cr_assert_str_eq(log_template_get_trivial_value(template, msg, NULL), "árvíztűrőtükörfúrógép");
template = compile_template("$1 $2", FALSE);
cr_assert_not(log_template_is_trivial(template), "Multi-element template is not trivial");
log_template_unref(template);

template = compile_template("$HOST", FALSE);
cr_assert(log_template_is_trivial(template));
cr_assert_str_eq(log_template_get_trivial_value(template, msg, NULL), "bzorp");
template = compile_template("$1 literal", FALSE);
cr_assert_not(log_template_is_trivial(template), "Multi-element template is not trivial");
log_template_unref(template);

template = compile_template("${APP.VALUE}", FALSE);
cr_assert(log_template_is_trivial(template));
cr_assert_str_eq(log_template_get_trivial_value(template, msg, NULL), "value");
template = compile_template("pre${1}", FALSE);
cr_assert_not(log_template_is_trivial(template), "Single-value template with preliminary text is not trivial");
log_template_unref(template);

template = compile_template("${MSG}@3", FALSE);
cr_assert_not(log_template_is_trivial(template), "Template referencing non-last context element is not trivial");
log_template_unref(template);

template = compile_template("$(echo test)", FALSE);
cr_assert_not(log_template_is_trivial(template), "Template functions are not trivial");
log_template_unref(template);

template = compile_template("$DATE", FALSE);
cr_assert_not(log_template_is_trivial(template), "Hard macros are not trivial");
log_template_unref(template);
}

static void
assert_template_literal_value(const gchar *template_code, const gchar *expected_value)
{
LogTemplate *template = compile_template(template_code, FALSE);

cr_assert(log_template_is_literal_string(template));

const gchar *literal_val = log_template_get_literal_value(template, NULL);
cr_assert_str_eq(literal_val, expected_value);

GString *formatted_value = g_string_sized_new(64);
LogMessage *msg = create_sample_message();
log_template_format(template, msg, &DEFAULT_TEMPLATE_EVAL_OPTIONS, formatted_value);
cr_assert_str_eq(literal_val, formatted_value->str,
"Formatted and literal value does not match: '%s' - '%s'", literal_val, formatted_value->str);

log_msg_unref(msg);
g_string_free(formatted_value, TRUE);
log_template_unref(template);
}

Test(template, test_literal_string_templates)
{
assert_template_literal_value("", "");
assert_template_literal_value(" ", " ");
assert_template_literal_value("literal string", "literal string");
assert_template_literal_value("$$not a macro", "$not a macro");

LogTemplate *template = compile_template("a b c d $MSG", FALSE);
cr_assert_not(log_template_is_literal_string(template));
log_template_unref(template);
}

Test(template, test_compile_literal_string)
{
LogTemplate *template = log_template_new(configuration, NULL);
log_template_compile_literal_string(template, "test literal");

cr_assert(log_template_is_literal_string(template));
cr_assert(log_template_is_trivial(template));

cr_assert_str_eq(log_template_get_literal_value(template, NULL), "test literal");

log_template_unref(template);
}
2 changes: 1 addition & 1 deletion lib/value-pairs/tests/test_value_pairs.c
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@ create_template(const gchar *type_hint_string, const gchar *template_string)
LogTemplate *template;

template = log_template_new(configuration, NULL);
log_template_compile(template, template_string, NULL);
cr_assert(log_template_compile(template, template_string, NULL));
log_template_set_type_hint(template, type_hint_string, NULL);
return template;
}
Expand Down
4 changes: 2 additions & 2 deletions modules/afamqp/afamqp-grammar.ym
Original file line number Diff line number Diff line change
Expand Up @@ -85,8 +85,8 @@ 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 '(' template_content ')' { afamqp_dd_set_routing_key(last_driver, $3); }
| KW_BODY '(' template_content ')' { afamqp_dd_set_body(last_driver, $3); }
| KW_PERSISTENT '(' yesno ')' { afamqp_dd_set_persistent(last_driver, $3); }
| KW_AUTH_METHOD '(' string ')' { CHECK_ERROR(afamqp_dd_set_auth_method(last_driver, $3), @3, "unknown auth-method() argument"); free($3); }
| KW_USERNAME '(' string ')' { afamqp_dd_set_user(last_driver, $3); free($3); }
Expand Down
14 changes: 7 additions & 7 deletions modules/afamqp/afamqp.c
Original file line number Diff line number Diff line change
Expand Up @@ -170,21 +170,21 @@ afamqp_dd_set_exchange_type(LogDriver *d, const gchar *exchange_type)
}

void
afamqp_dd_set_routing_key(LogDriver *d, const gchar *routing_key)
afamqp_dd_set_routing_key(LogDriver *d, LogTemplate *routing_key_template)
{
AMQPDestDriver *self = (AMQPDestDriver *) d;

log_template_compile(self->routing_key_template, routing_key, NULL);
log_template_unref(self->routing_key_template);
self->routing_key_template = routing_key_template;
}

void
afamqp_dd_set_body(LogDriver *d, const gchar *body)
afamqp_dd_set_body(LogDriver *d, LogTemplate *body_template)
{
AMQPDestDriver *self = (AMQPDestDriver *) d;

if (!self->body_template)
self->body_template = log_template_new(configuration, NULL);
log_template_compile(self->body_template, body, NULL);
log_template_unref(self->body_template);
self->body_template = body_template;
}

void
Expand Down Expand Up @@ -794,6 +794,7 @@ afamqp_dd_new(GlobalConfig *cfg)
self->super.stats_source = stats_register_type("amqp");

self->routing_key_template = log_template_new(cfg, NULL);
log_template_compile_literal_string(self->routing_key_template, "");

LogDriver *driver = &self->super.super.super;
afamqp_dd_set_auth_method(driver, "plain");
Expand All @@ -802,7 +803,6 @@ afamqp_dd_new(GlobalConfig *cfg)
afamqp_dd_set_port(driver, 5672);
afamqp_dd_set_exchange(driver, "syslog");
afamqp_dd_set_exchange_type(driver, "fanout");
afamqp_dd_set_routing_key(driver, "");
afamqp_dd_set_persistent(driver, TRUE);
afamqp_dd_set_exchange_declare(driver, FALSE);
afamqp_dd_set_max_channel(driver, AMQP_DEFAULT_MAX_CHANNELS);
Expand Down
4 changes: 2 additions & 2 deletions modules/afamqp/afamqp.h
Original file line number Diff line number Diff line change
Expand Up @@ -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);
void afamqp_dd_set_routing_key(LogDriver *d, LogTemplate *routing_key_template);
void afamqp_dd_set_body(LogDriver *d, LogTemplate *body_template);
void afamqp_dd_set_persistent(LogDriver *d, gboolean persistent);
gboolean afamqp_dd_set_auth_method(LogDriver *d, const gchar *auth_method);
void afamqp_dd_set_user(LogDriver *d, const gchar *user);
Expand Down
Loading