Merge pull request #431 from bazsi/filterx-check-if-value-is-a-macro-…

…during-compilation

Filterx: check if value is a macro during compilation

lib/filterx/expr-variable.c

@@ -36,7 +36,7 @@ typedef struct _FilterXVariableExpr
   FilterXExpr super;
   FilterXObject *variable_name;
   NVHandle handle;
-  gboolean declared;
+  guint32 declared:1, handle_is_macro:1;
 } FilterXVariableExpr;
 
 static FilterXObject *
@@ -51,7 +51,7 @@ _pull_variable_from_message(FilterXVariableExpr *self, FilterXEvalContext *conte
       return NULL;
     }
 
-  if (log_msg_is_value_from_macro(value))
+  if (self->handle_is_macro)
     return filterx_message_value_new(value, value_len, t);
   else
     return filterx_message_value_new_borrowed(value, value_len, t);
@@ -220,6 +220,8 @@ filterx_variable_expr_new(FilterXString *name, FilterXVariableType type)
 
   self->variable_name = (FilterXObject *) name;
   self->handle = filterx_map_varname_to_handle(filterx_string_get_value_ref(self->variable_name, NULL), type);
+  if (type == FX_VAR_MESSAGE)
+    self->handle_is_macro = log_msg_is_handle_macro(filterx_variable_handle_to_nv_handle(self->handle));
 
   return &self->super;
 }

lib/filterx/filterx-variable.h

@@ -68,6 +68,12 @@ filterx_variable_handle_is_floating(FilterXVariableHandle handle)
   return !!(handle & FILTERX_HANDLE_FLOATING_BIT);
 }
 
+static inline NVHandle
+filterx_variable_handle_to_nv_handle(FilterXVariableHandle handle)
+{
+  return handle & ~FILTERX_HANDLE_FLOATING_BIT;
+}
+
 static inline gboolean
 filterx_variable_is_floating(FilterXVariable *v)
 {
@@ -77,7 +83,7 @@ filterx_variable_is_floating(FilterXVariable *v)
 static inline NVHandle
 filterx_variable_get_nv_handle(FilterXVariable *v)
 {
-  return v->handle & ~FILTERX_HANDLE_FLOATING_BIT;
+  return filterx_variable_handle_to_nv_handle(v->handle);
 }
 
 static inline const gchar *

lib/filterx/object-message-value.c

@@ -169,13 +169,16 @@ _is_value_type_pair_truthy(const gchar  *repr, gssize repr_len, LogMessageValueT
         return TRUE;
       break;
     case LM_VT_STRING:
+    case LM_VT_PROTOBUF:
+    case LM_VT_BYTES:
       if (repr_len > 0)
         return TRUE;
       break;
     case LM_VT_JSON:
     case LM_VT_LIST:
     case LM_VT_DATETIME:
       return TRUE;
+    case LM_VT_NULL:
     default:
       break;
     }

lib/logmsg/logmsg.c

@@ -502,14 +502,6 @@ log_msg_get_macro_value(const LogMessage *self, gint id, gssize *value_len, LogM
   return value->str;
 }
 
-gboolean
-log_msg_is_value_from_macro(const gchar *value)
-{
-  GString *buffer = g_private_get(&priv_macro_value);
-  return buffer && buffer->str == value;
-}
-
-
 static void
 log_msg_init_queue_node(LogMessage *msg, LogMessageQueueNode *node, const LogPathOptions *path_options)
 {

lib/logmsg/logmsg.h

@@ -395,8 +395,6 @@ log_msg_get_value_if_set_with_type(const LogMessage *self, NVHandle handle,
     return nv_table_get_value(self->payload, handle, value_len, type);
 }
 
-gboolean log_msg_is_value_from_macro(const gchar *value);
-
 static inline gboolean
 log_msg_is_value_set(const LogMessage *self, NVHandle handle)
 {

lib/template/compiler.c

@@ -88,7 +88,7 @@ parse_msg_ref(LogTemplateCompiler *self)
           if ((*self->cursor) != '@')
             {
               msg_warning("Non-numeric correlation state ID found, assuming a literal '@' character. To avoid confusion when using a literal '@' after a macro or template function, write '@@' in the template.",
-                          evt_tag_str("Template", self->template->template_str));
+                          evt_tag_str("template", self->template->template_str));
               self->cursor--;
             }
           self->msg_ref = 0;
@@ -399,7 +399,7 @@ log_template_compiler_process_token(LogTemplateCompiler *self, GError **error)
                       "Use '$$' to specify a literal dollar sign instead of '\\$' and "
                       "remove the escaping of the backslash character when you upgrade "
                       "your configuration",
-                      evt_tag_str("Template", self->template->template_str));
+                      evt_tag_str("template", self->template->template_str));
           self->cursor++;
         }
 

lib/template/eval.c

@@ -75,6 +75,10 @@ log_template_append_elem_value(LogTemplate *self, LogTemplateElem *e, LogTemplat
     }
   else if (value_type == LM_VT_BYTES || value_type == LM_VT_PROTOBUF)
     {
+      msg_warning_once("template: not rendering binary name-value pair, use an explicit type hint",
+                       evt_tag_str("template", self->template_str),
+                       evt_tag_str("name", log_msg_get_handle_name(e->value_handle, NULL)),
+                       evt_tag_str("type", log_msg_value_type_to_str(value_type)));
       value_type = LM_VT_NULL;
     }
   *type = _propagate_type(*type, value_type);

lib/template/globals.c

@@ -25,13 +25,7 @@
 #include "templates.h"
 #include "macros.h"
 
-static LogTemplateOptions global_template_options;
-
-LogTemplateOptions *
-log_template_get_global_template_options(void)
-{
-  return &global_template_options;
-}
+LogTemplateOptions global_template_options;
 
 void
 log_template_global_init(void)

lib/template/globals.h

@@ -27,7 +27,13 @@
 
 #include "common-template-typedefs.h"
 
-LogTemplateOptions *log_template_get_global_template_options(void);
+extern LogTemplateOptions global_template_options;
+
+static inline LogTemplateOptions *
+log_template_get_global_template_options(void)
+{
+  return &global_template_options;
+}
 
 void log_template_global_init(void);
 void log_template_global_deinit(void);

lib/template/macros.c

@@ -221,8 +221,6 @@ LogMacroDef macros[] =
   { "UNIQID", M_UNIQID },
 
   /* values that have specific behaviour with older syslog-ng config versions */
-  { "MSG", M_MESSAGE },
-  { "MESSAGE", M_MESSAGE },
   { "HOST", M_HOST },
 
   /* message independent macros */
@@ -585,11 +583,6 @@ log_macro_expand(gint id, LogTemplateEvalOptions *options, const LogMessage *msg
         }
       break;
     }
-    case M_MESSAGE:
-    {
-      _result_append_value(result, msg, LM_V_MESSAGE, &t);
-      break;
-    }
     case M_SOURCE_IP:
     {
       gchar *ip;

lib/template/macros.h

@@ -47,7 +47,6 @@ enum
   M_SDATA,
 
   M_MSGHDR,
-  M_MESSAGE,
   M_SOURCE_IP,
   M_DEST_IP,
   M_DEST_PORT,

lib/template/templates.c

@@ -71,9 +71,7 @@ log_template_get_trivial_value_handle(LogTemplate *self)
   switch (e->type)
     {
     case LTE_MACRO:
-      if (e->macro == M_MESSAGE)
-        return LM_V_MESSAGE;
-      else if (e->macro == M_HOST)
+      if (e->macro == M_HOST)
         return LM_V_HOST;
       else
         g_assert_not_reached();
@@ -170,7 +168,7 @@ _calculate_if_trivial(LogTemplate *self)
       /* we have macros for MESSAGE and HOST for compatibility reasons, but
        * they should be considered trivial */
 
-      if (e->macro == M_MESSAGE || e->macro == M_HOST)
+      if (e->macro == M_HOST)
         return TRUE;
       return FALSE;
     case LTE_VALUE:

lib/template/tests/test_template_compile.c

@@ -156,56 +156,56 @@ Test(template_compile, test_simple_string_literal)
 
 Test(template_compile, test_simple_macro)
 {
-  assert_template_compile("${MESSAGE}");
-  assert_compiled_template(text = "", default_value = NULL, macro = M_MESSAGE, type = LTE_MACRO, msg_ref = 0);
+  assert_template_compile("${MSGHDR}");
+  assert_compiled_template(text = "", default_value = NULL, macro = M_MSGHDR, type = LTE_MACRO, msg_ref = 0);
 }
 
 Test(template_compile, test_macro_and_text)
 {
 
-  assert_template_compile("${MESSAGE}test value");
-  assert_compiled_template(text = "", default_value = NULL, macro = M_MESSAGE, type = LTE_MACRO, msg_ref = 0);
+  assert_template_compile("${MSGHDR}test value");
+  assert_compiled_template(text = "", default_value = NULL, macro = M_MSGHDR, type = LTE_MACRO, msg_ref = 0);
 
   select_next_element();
   assert_compiled_template(text = "test value", default_value = NULL, macro = M_NONE, type = LTE_MACRO, msg_ref = 0);
 }
 
 Test(template_compile, test_macro_without_braces)
 {
-  assert_template_compile("$MESSAGE");
-  assert_compiled_template(text = "", default_value = NULL, macro = M_MESSAGE, type = LTE_MACRO, msg_ref = 0);
+  assert_template_compile("$MSGHDR");
+  assert_compiled_template(text = "", default_value = NULL, macro = M_MSGHDR, type = LTE_MACRO, msg_ref = 0);
 }
 
 Test(template_compile, test_macro_name_without_braces_are_terminated_with_non_identifier_characters)
 {
   /* macro names consist of [A-Z0-9_] */
-  assert_template_compile("$MESSAGE test value");
-  assert_compiled_template(text = "", default_value = NULL, macro = M_MESSAGE, type = LTE_MACRO, msg_ref = 0);
+  assert_template_compile("$MSGHDR test value");
+  assert_compiled_template(text = "", default_value = NULL, macro = M_MSGHDR, type = LTE_MACRO, msg_ref = 0);
 
   select_next_element();
   assert_compiled_template(text = " test value", default_value = NULL, macro = M_NONE, type = LTE_MACRO, msg_ref = 0);
 }
 
 Test(template_compile, test_macro_without_at_records_that_no_msgref_was_present_by_msgref_zero)
 {
-  assert_template_compile("${MESSAGE}");
-  assert_compiled_template(text = "", default_value = NULL, macro = M_MESSAGE, type = LTE_MACRO, msg_ref = 0);
+  assert_template_compile("${MSGHDR}");
+  assert_compiled_template(text = "", default_value = NULL, macro = M_MSGHDR, type = LTE_MACRO, msg_ref = 0);
 }
 
 Test(template_compile, test_macro_with_at_references_a_single_msg_in_the_context_stack_by_setting_msgref)
 {
-  assert_template_compile("${MESSAGE}@0");
-  assert_compiled_template(text = "", default_value = NULL, macro = M_MESSAGE, type = LTE_MACRO, msg_ref = 1);
+  assert_template_compile("${MSGHDR}@0");
+  assert_compiled_template(text = "", default_value = NULL, macro = M_MSGHDR, type = LTE_MACRO, msg_ref = 1);
 
-  assert_template_compile("${MESSAGE}@1");
-  assert_compiled_template(text = "", default_value = NULL, macro = M_MESSAGE, type = LTE_MACRO, msg_ref = 2);
+  assert_template_compile("${MSGHDR}@1");
+  assert_compiled_template(text = "", default_value = NULL, macro = M_MSGHDR, type = LTE_MACRO, msg_ref = 2);
 
 }
 
 Test(template_compile, test_macro_with_invalid_msgref_are_recognized_as_the_top_element_in_the_stack)
 {
-  assert_template_compile("${MESSAGE}@gmail.com");
-  assert_compiled_template(text = "", default_value = NULL, macro = M_MESSAGE, type = LTE_MACRO, msg_ref = 0);
+  assert_template_compile("${MSGHDR}@gmail.com");
+  assert_compiled_template(text = "", default_value = NULL, macro = M_MSGHDR, type = LTE_MACRO, msg_ref = 0);
 
   select_next_element();
   assert_compiled_template(text = "@gmail.com", default_value = NULL, macro = M_NONE, type = LTE_MACRO, msg_ref = 0);
@@ -225,11 +225,11 @@ Test(template_compile, test_dollar_prefixed_with_backslash_is_a_literal_dollar)
 
 Test(template_compile, test_colon_dash_in_braces_is_parsed_as_default_value)
 {
-  assert_template_compile("${MESSAGE:-default value}");
-  assert_compiled_template(text = "", default_value = "default value", macro = M_MESSAGE, type = LTE_MACRO, msg_ref = 0);
+  assert_template_compile("${MSGHDR:-default value}");
+  assert_compiled_template(text = "", default_value = "default value", macro = M_MSGHDR, type = LTE_MACRO, msg_ref = 0);
 
-  assert_template_compile("${MESSAGE:-}");
-  assert_compiled_template(text = "", default_value = "", macro = M_MESSAGE, type = LTE_MACRO, msg_ref = 0);
+  assert_template_compile("${MSGHDR:-}");
+  assert_compiled_template(text = "", default_value = "", macro = M_MSGHDR, type = LTE_MACRO, msg_ref = 0);
 }
 
 Test(template_compile, test_double_dollars_is_a_literal_dollar)
@@ -264,8 +264,8 @@ Test(template_compile, test_backslash_without_finishing_the_escape_sequence_is_i
 
 Test(template_compile, test_double_at_is_a_literal_at)
 {
-  assert_template_compile("${MESSAGE}@@12");
-  assert_compiled_template(text = "", default_value = NULL, macro = M_MESSAGE, type = LTE_MACRO, msg_ref = 0);
+  assert_template_compile("${MSGHDR}@@12");
+  assert_compiled_template(text = "", default_value = NULL, macro = M_MSGHDR, type = LTE_MACRO, msg_ref = 0);
 
   select_next_element();
   assert_compiled_template(text = "@12", default_value = NULL, macro = M_NONE, type = LTE_MACRO, msg_ref = 0);
@@ -332,15 +332,15 @@ Test(template_compile, test_qouted_string_in_name_template_function)
 
 Test(template_compile, test_invalid_macro)
 {
-  assert_failed_template_compile("${MESSAGE", "Invalid macro, '}' is missing, error_pos='9'");
-  assert_compiled_template(text = "error in template: ${MESSAGE", default_value = NULL, macro = M_NONE, type = LTE_MACRO,
+  assert_failed_template_compile("${MSGHDR", "Invalid macro, '}' is missing, error_pos='8'");
+  assert_compiled_template(text = "error in template: ${MSGHDR", default_value = NULL, macro = M_NONE, type = LTE_MACRO,
                            msg_ref = 0);
 }
 
 Test(template_compile, test_invalid_subst)
 {
-  assert_failed_template_compile("${MESSAGE:1}", "Unknown substitution function, error_pos='10'");
-  assert_compiled_template(text = "error in template: ${MESSAGE:1}", default_value = NULL, macro = M_NONE,
+  assert_failed_template_compile("${MSGHDR:1}", "Unknown substitution function, error_pos='9'");
+  assert_compiled_template(text = "error in template: ${MSGHDR:1}", default_value = NULL, macro = M_NONE,
                            type = LTE_MACRO, msg_ref = 0);
 }
 

lib/value-pairs/value-pairs.c

@@ -1107,16 +1107,43 @@ value_pairs_global_init(void)
   value_pairs_init_set(rfc5424);
   value_pairs_init_set(selected_macros);
 
+  ValuePairSpec pair;
+
   a = g_array_new(TRUE, TRUE, sizeof(ValuePairSpec));
   for (i = 0; macros[i].name; i++)
     {
-      ValuePairSpec pair;
-
       pair.name = macros[i].name;
       pair.type = VPT_MACRO;
       pair.id = macros[i].id;
       g_array_append_val(a, pair);
     }
+  /* NOTE: this is a hack
+   *
+   * We used to have $MSG and $MESSAGE both as macros, doing some weird
+   * escaping back in the day but have been doing basically nothing forever.
+   * When removing these as macros, we have MESSAGE as a name-value pair and
+   * MSG being an alias to the same.
+   *
+   * However, when a value-pairs expression has `--key MSG`, that does not
+   * match the $MESSAGE name-value pair and it does not consult aliases.
+   * Adding alias support to --key would be difficult (and probably risky as
+   * well), so we add a $MSG entry to all_macros here, which would mimic the
+   * alias support as it exists in NVRegistry.
+   *
+   * For everything outside of value-pairs, we only have a name-value pair
+   * called $MESSAGE and $MSG as its alias.  Within value-pairs, we also
+   * have $MSG listed as a part of all macros, causing any --key using MSG
+   * to remain operational.
+   *
+   * Without this --key MSG would not match any name-value pairs (as that's
+   * called MESSAGE) and would not encounter a macro that is called MSG),
+   * causing that key to be never become part of the value-pairs output.
+   */
+
+  pair.name = "MSG";
+  pair.type = VPT_NVPAIR;
+  pair.id = LM_V_MESSAGE;
+  g_array_append_val(a, pair);
   all_macros = (ValuePairSpec *) g_array_free(a, FALSE);
 }