Merge pull request #1650 from presidento/sanitize-sdata-keys

logmsg: sanitize sdata keys

lib/logmsg/logmsg.c

@@ -43,6 +43,7 @@
 #include <unistd.h>
 #include <string.h>
 #include <stdlib.h>
+#include <ctype.h>
 
 /*
  * Reference/ACK counting for LogMessage structures
@@ -861,6 +862,27 @@ log_msg_is_tag_by_name(LogMessage *self, const gchar *name)
 
 /* structured data elements */
 
+static void
+log_msg_sdata_append_key_escaped(GString *result, const gchar *sstr, gssize len)
+{
+  /* The specification does not have any way to escape keys.
+   * The goal is to create syntactically valid structured data fields. */
+  const guchar *ustr = (const guchar *) sstr;
+
+  for (gssize i = 0; i < len; i++)
+    {
+      if (!isascii(ustr[i]) || ustr[i] == '=' || ustr[i] == ' '
+          || ustr[i] == '[' || ustr[i] == ']' || ustr[i] == '"')
+        {
+          gchar hex_code[4];
+          g_sprintf(hex_code, "%%%02X", ustr[i]);
+          g_string_append(result, hex_code);
+        }
+      else
+        g_string_append_c(result, ustr[i]);
+    }
+}
+
 static void
 log_msg_sdata_append_escaped(GString *result, const gchar *sstr, gssize len)
 {
@@ -993,7 +1015,7 @@ log_msg_append_format_sdata(const LogMessage *self, GString *result,  guint32 se
           if (value)
             {
               g_string_append_c(result, ' ');
-              g_string_append_len(result, sdata_param, sdata_param_len);
+              log_msg_sdata_append_key_escaped(result, sdata_param, sdata_param_len);
               g_string_append(result, "=\"");
               log_msg_sdata_append_escaped(result, value, len);
               g_string_append_c(result, '"');

lib/logmsg/tests/test_log_message.c

@@ -97,7 +97,7 @@ assert_sdata_value_with_seqnum_equals(LogMessage *msg, guint32 seq_num, const gc
   GString *result = g_string_sized_new(0);
 
   log_msg_append_format_sdata(msg, result, seq_num);
-  cr_assert_str_eq(result->str, expected, "SDATA value does not match");
+  cr_assert_str_eq(result->str, expected, "SDATA value does not match, '%s' vs '%s'", expected, result->str);
   g_string_free(result, TRUE);
 }
 
@@ -332,6 +332,40 @@ Test(log_message, test_local_logmsg_created_with_the_right_flags_and_timestamps)
   cr_assert(are_equals, "The timestamps in a LogMessage created by log_msg_new_local() should be equals");
 }
 
+Test(log_message, test_sdata_sanitize_keys)
+{
+  LogMessage *msg;
+  /* These keys looks strange, but JSON object can be parsed to SDATA,
+   * so the key could contain any character, while the specification
+   * does not declare any way to encode the the keys, just the values.
+   * The goal is to have a syntactically valid syslog message. */
+
+  msg = log_msg_new_empty();
+  log_msg_set_value_by_name(msg, ".SDATA.foo.bar[0]", "value[0]", -1);
+  assert_sdata_value_equals(msg, "[foo bar%5B0%5D=\"value[0\\]\"]");
+  log_msg_unref(msg);
+
+  msg = log_msg_new_empty();
+  log_msg_set_value_by_name(msg, ".SDATA.foo.bácsi", "bácsi", -1);
+  assert_sdata_value_equals(msg, "[foo b%C3%A1csi=\"bácsi\"]");
+  log_msg_unref(msg);
+
+  msg = log_msg_new_empty();
+  log_msg_set_value_by_name(msg, ".SDATA.foo.sp ace", "sp ace", -1);
+  assert_sdata_value_equals(msg, "[foo sp%20ace=\"sp ace\"]");
+  log_msg_unref(msg);
+
+  msg = log_msg_new_empty();
+  log_msg_set_value_by_name(msg, ".SDATA.foo.eq=al", "eq=al", -1);
+  assert_sdata_value_equals(msg, "[foo eq%3Dal=\"eq=al\"]");
+  log_msg_unref(msg);
+
+  msg = log_msg_new_empty();
+  log_msg_set_value_by_name(msg, ".SDATA.foo.quo\"te", "quo\"te", -1);
+  assert_sdata_value_equals(msg, "[foo quo%22te=\"quo\\\"te\"]");
+  log_msg_unref(msg);
+}
+
 Test(log_message, test_sdata_value_is_updated_by_sdata_name_value_pairs)
 {
   LogMessage *msg;