Tighten up the check for allowed key and group names. (#343191, Tommi

2006-12-18  Matthias Clasen  <mclasen@redhat.com>

        * glib/gkeyfile.c: Tighten up the check for allowed
        key and group names.  (#343191, Tommi Komulainen)

        * tests/keyfile-test.c: Test handling of key and group names.

ChangeLog

@@ -1,5 +1,10 @@
 2006-12-18  Matthias Clasen  <mclasen@redhat.com>
 
+	* glib/gkeyfile.c: Tighten up the check for allowed
+	key and group names.  (#343191, Tommi Komulainen)
+
+	* tests/keyfile-test.c: Test handling of key and group names.
+
 	* tests/Makefile.am: Don't use $RANDOM if the shell doesn't
 	have it.  (#346373, Thomas Klausner)
 

docs/reference/glib/tmpl/keyfile.sgml

@@ -98,6 +98,8 @@ Note that in contrast to the
 Entry Specification</ulink>, groups in key files may contain the same
 key multiple times; the last entry wins. Key files may also contain
 multiple groups with the same name; they are merged together.
+Another difference is that keys and group names in key files are not
+restricted to ASCII characters.
 </para>
 
 <!-- ##### SECTION See_Also ##### -->

glib/gkeyfile.c

@@ -136,6 +136,8 @@ static void                  g_key_file_add_key                (GKeyFile
 								const gchar            *value);
 static void                  g_key_file_add_group              (GKeyFile               *key_file,
 								const gchar            *group_name);
+static gboolean              g_key_file_is_group_name          (const gchar *name);
+static gboolean              g_key_file_is_key_name            (const gchar *name);
 static void                  g_key_file_key_value_pair_free    (GKeyFileKeyValuePair   *pair);
 static gboolean              g_key_file_line_is_comment        (const gchar            *line);
 static gboolean              g_key_file_line_is_group          (const gchar            *line);
@@ -260,6 +262,8 @@ void
 g_key_file_set_list_separator (GKeyFile *key_file,
 			       gchar     separator)
 {
+  g_return_if_fail (key_file != NULL);
+
   key_file->list_separator = separator;
 }
 
@@ -747,6 +751,15 @@ g_key_file_parse_group (GKeyFile     *key_file,
   group_name = g_strndup (group_name_start, 
                           group_name_end - group_name_start);
   
+  if (!g_key_file_is_group_name (group_name))
+    {
+      g_set_error (error, G_KEY_FILE_ERROR,
+		   G_KEY_FILE_ERROR_PARSE,
+		   _("Invalid group name: %s"), group_name);
+      g_free (group_name);
+      return;
+    }
+
   g_key_file_add_group (key_file, group_name);
   g_free (group_name);
 }
@@ -786,6 +799,15 @@ g_key_file_parse_key_value_pair (GKeyFile     *key_file,
 
   key = g_strndup (line, key_len - 1);
 
+  if (!g_key_file_is_key_name (key))
+    {
+      g_set_error (error, G_KEY_FILE_ERROR,
+                   G_KEY_FILE_ERROR_PARSE,
+                   _("Invalid key name: %s"), key);
+      g_free (key);
+      return; 
+    }
+
   /* Pull the value from the line (chugging leading whitespace)
    */
   while (g_ascii_isspace (*value_start))
@@ -1216,8 +1238,8 @@ g_key_file_set_value (GKeyFile    *key_file,
   GKeyFileKeyValuePair *pair;
 
   g_return_if_fail (key_file != NULL);
-  g_return_if_fail (group_name != NULL);
+  g_return_if_fail (g_key_file_is_group_name (group_name));
-  g_return_if_fail (key != NULL);
+  g_return_if_fail (g_key_file_is_key_name (key));
   g_return_if_fail (value != NULL);
 
   group = g_key_file_lookup_group (key_file, group_name);
@@ -1344,8 +1366,6 @@ g_key_file_set_string (GKeyFile    *key_file,
   gchar *value;
 
   g_return_if_fail (key_file != NULL);
-  g_return_if_fail (group_name != NULL);
-  g_return_if_fail (key != NULL);
   g_return_if_fail (string != NULL);
 
   value = g_key_file_parse_string_as_value (key_file, string, FALSE);
@@ -1470,8 +1490,6 @@ g_key_file_set_string_list (GKeyFile            *key_file,
   gsize i;
 
   g_return_if_fail (key_file != NULL);
-  g_return_if_fail (group_name != NULL);
-  g_return_if_fail (key != NULL);
   g_return_if_fail (list != NULL);
 
   value_list = g_string_sized_new (length * 128);
@@ -1514,7 +1532,6 @@ g_key_file_set_locale_string (GKeyFile     *key_file,
   gchar *full_key, *value;
 
   g_return_if_fail (key_file != NULL);
-  g_return_if_fail (group_name != NULL);
   g_return_if_fail (key != NULL);
   g_return_if_fail (locale != NULL);
   g_return_if_fail (string != NULL);
@@ -1717,7 +1734,6 @@ g_key_file_set_locale_string_list (GKeyFile            *key_file,
   gsize i;
 
   g_return_if_fail (key_file != NULL);
-  g_return_if_fail (group_name != NULL);
   g_return_if_fail (key != NULL);
   g_return_if_fail (locale != NULL);
   g_return_if_fail (length != 0);
@@ -1827,8 +1843,6 @@ g_key_file_set_boolean (GKeyFile    *key_file,
   gchar *result;
 
   g_return_if_fail (key_file != NULL);
-  g_return_if_fail (group_name != NULL);
-  g_return_if_fail (key != NULL);
 
   result = g_key_file_parse_boolean_as_value (key_file, value);
   g_key_file_set_value (key_file, group_name, key, result);
@@ -1933,8 +1947,6 @@ g_key_file_set_boolean_list (GKeyFile    *key_file,
   gsize i;
 
   g_return_if_fail (key_file != NULL);
-  g_return_if_fail (group_name != NULL);
-  g_return_if_fail (key != NULL);
   g_return_if_fail (list != NULL);
 
   value_list = g_string_sized_new (length * 8);
@@ -2043,8 +2055,6 @@ g_key_file_set_integer (GKeyFile    *key_file,
   gchar *result;
 
   g_return_if_fail (key_file != NULL);
-  g_return_if_fail (group_name != NULL);
-  g_return_if_fail (key != NULL);
 
   result = g_key_file_parse_integer_as_value (key_file, value);
   g_key_file_set_value (key_file, group_name, key, result);
@@ -2146,8 +2156,6 @@ g_key_file_set_integer_list (GKeyFile     *key_file,
   gsize i;
 
   g_return_if_fail (key_file != NULL);
-  g_return_if_fail (group_name != NULL);
-  g_return_if_fail (key != NULL);
   g_return_if_fail (list != NULL);
 
   values = g_string_sized_new (length * 16);
@@ -2257,10 +2265,8 @@ g_key_file_set_double  (GKeyFile    *key_file,
   gchar result[G_ASCII_DTOSTR_BUF_SIZE];
 
   g_return_if_fail (key_file != NULL);
-  g_return_if_fail (group_name != NULL);
-  g_return_if_fail (key != NULL);
 
-  g_ascii_dtostr ( result, sizeof (result), value );
+  g_ascii_dtostr (result, sizeof (result), value);
   g_key_file_set_value (key_file, group_name, key, result);
 }
 
@@ -2360,8 +2366,6 @@ g_key_file_set_double_list (GKeyFile     *key_file,
   gsize i;
 
   g_return_if_fail (key_file != NULL);
-  g_return_if_fail (group_name != NULL);
-  g_return_if_fail (key != NULL);
   g_return_if_fail (list != NULL);
 
   values = g_string_sized_new (length * 16);
@@ -2455,6 +2459,8 @@ g_key_file_set_group_comment (GKeyFile             *key_file,
 {
   GKeyFileGroup *group;
   
+  g_return_if_fail (g_key_file_is_group_name (group_name));
+
   group = g_key_file_lookup_group (key_file, group_name);
   if (!group)
     {
@@ -2574,6 +2580,8 @@ g_key_file_get_key_comment (GKeyFile             *key_file,
   GString *string;
   gchar *comment;
 
+  g_return_val_if_fail (g_key_file_is_group_name (group_name), NULL);
+
   group = g_key_file_lookup_group (key_file, group_name);
   if (!group)
     {
@@ -2885,7 +2893,7 @@ g_key_file_add_group (GKeyFile    *key_file,
   GKeyFileGroup *group;
 
   g_return_if_fail (key_file != NULL);
-  g_return_if_fail (group_name != NULL);
+  g_return_if_fail (g_key_file_is_group_name (group_name));
 
   group = g_key_file_lookup_group (key_file, group_name);
   if (group != NULL)
@@ -3199,6 +3207,54 @@ g_key_file_line_is_comment (const gchar *line)
   return (*line == '#' || *line == '\0' || *line == '\n');
 }
 
+static gboolean 
+g_key_file_is_group_name (const gchar *name)
+{
+  gchar *p, *q;
+
+  if (name == NULL)
+    return FALSE;
+
+  p = q = (gchar *) name;
+  while (*q && *q != ']' && *q != '[' && !g_ascii_iscntrl (*q))
+    q = g_utf8_next_char (q);
+  
+  if (*q != '\0' || q == p)
+    return FALSE;
+
+  return TRUE;
+}
+
+static gboolean
+g_key_file_is_key_name (const gchar *name)
+{
+  gchar *p, *q;
+
+  if (name == NULL)
+    return FALSE;
+
+  p = q = (gchar *) name;
+  while (*q && (g_unichar_isalnum (g_utf8_get_char (q)) || *q == '-'))
+    q = g_utf8_next_char (q);
+  
+  if (*q == '[')
+    {
+      q++;
+      while (*q && (g_unichar_isalnum (g_utf8_get_char (q)) || *q == '-' || *q == '_' || *q == '.'))
+        q = g_utf8_next_char (q);
+
+      if (*q != ']')
+        return FALSE;     
+
+      q++;
+    }
+
+  if (*q != '\0' || q == p)
+    return FALSE;
+
+  return TRUE;
+}
+
 /* A group in a key file is made up of a starting '[' followed by one
  * or more letters making up the group name followed by ']'.
  */
@@ -3211,12 +3267,7 @@ g_key_file_line_is_group (const gchar *line)
   if (*p != '[')
     return FALSE;
 
-  p = g_utf8_next_char (p);
+  p++;
-
-  /* Group name must be non-empty
-   */
-  if (!*p || *p == ']')
-    return FALSE;
 
   while (*p && *p != ']')
     p = g_utf8_next_char (p);

tests/keyfile-test.c

@@ -747,8 +747,6 @@ static void
 test_locale_string (void)
 {
   GKeyFile *keyfile;
-  GError *error = NULL;
-  gchar *value;
 
   const gchar *data = 
     "[valid]\n"
@@ -757,11 +755,7 @@ test_locale_string (void)
     "key1[de_DE]=v1-de_DE\n"
     "key1[de_DE.UTF8]=v1-de_DE.UTF8\n"
     "key1[fr]=v1-fr\n"
-    "key1[en] =v1-en\n"
+    "key1[en] =v1-en\n";
-    "[invalid]\n"
-    "key1[de=v1\n"
-    "key1[fr]]=v2\n"
-    "key1 [en]=v3\n";  
   
   keyfile = load_data (data, G_KEY_FILE_KEEP_TRANSLATIONS);
 
@@ -773,18 +767,6 @@ test_locale_string (void)
   check_locale_string_value (keyfile, "valid", "key1", "fr_FR", "v1-fr");
   check_locale_string_value (keyfile, "valid", "key1", "en", "v1-en");
   
-  value = g_key_file_get_locale_string (keyfile, "invalid", "key1", "de", &error);
-  check_error (&error, G_KEY_FILE_ERROR, G_KEY_FILE_ERROR_KEY_NOT_FOUND);
-  g_free (value);
-
-  value = g_key_file_get_locale_string (keyfile, "invalid", "key1", "fr", &error);
-  check_error (&error, G_KEY_FILE_ERROR, G_KEY_FILE_ERROR_KEY_NOT_FOUND);
-  g_free (value);
-
-  value = g_key_file_get_locale_string (keyfile, "invalid", "key1", "en", &error);
-  check_error (&error, G_KEY_FILE_ERROR, G_KEY_FILE_ERROR_KEY_NOT_FOUND);
-  g_free (value);
-  
   g_key_file_free (keyfile);
 
   /* now test that translations are thrown away */
@@ -974,6 +956,180 @@ test_groups (void)
   g_key_file_free (keyfile);  
 }
 
+static void
+test_group_names (void)
+{
+  GKeyFile *keyfile;
+  GError *error = NULL;
+  const gchar *data;
+  gchar *value;
+
+  /* [ in group name */
+  data = "[a[b]\n"
+         "key1=123\n";
+  keyfile = g_key_file_new ();
+  g_key_file_load_from_data (keyfile, data, -1, 0, &error);
+  g_key_file_free (keyfile);  
+  check_error (&error, 
+	       G_KEY_FILE_ERROR,
+	       G_KEY_FILE_ERROR_PARSE);
+
+  /* ] in group name */
+  data = "[a]b]\n"
+         "key1=123\n";
+  keyfile = g_key_file_new ();
+  g_key_file_load_from_data (keyfile, data, -1, 0, &error);
+  g_key_file_free (keyfile);  
+  check_error (&error, 
+	       G_KEY_FILE_ERROR,
+	       G_KEY_FILE_ERROR_PARSE);
+
+  /* control char in group name */
+  data = "[a\tb]\n"
+         "key1=123\n";
+  keyfile = g_key_file_new ();
+  g_key_file_load_from_data (keyfile, data, -1, 0, &error);
+  g_key_file_free (keyfile);  
+  check_error (&error, 
+	       G_KEY_FILE_ERROR,
+	       G_KEY_FILE_ERROR_PARSE);
+
+  /* Unicode in group name */
+  data = "[\xc2\xbd]\n"
+         "key1=123\n";
+  keyfile = g_key_file_new ();
+  g_key_file_load_from_data (keyfile, data, -1, 0, &error);
+  g_key_file_free (keyfile);  
+  check_no_error (&error);
+
+  keyfile = g_key_file_new ();
+  g_key_file_set_string (keyfile, "a[b", "key1", "123");
+  value = g_key_file_get_string (keyfile, "a[b", "key1", &error);
+  check_error (&error, 
+               G_KEY_FILE_ERROR,
+               G_KEY_FILE_ERROR_GROUP_NOT_FOUND);  
+  g_key_file_free (keyfile);  
+
+  keyfile = g_key_file_new ();
+  g_key_file_set_string (keyfile, "a]b", "key1", "123");
+  value = g_key_file_get_string (keyfile, "a]b", "key1", &error);
+  check_error (&error, 
+               G_KEY_FILE_ERROR,
+               G_KEY_FILE_ERROR_GROUP_NOT_FOUND);  
+  g_key_file_free (keyfile);  
+
+  keyfile = g_key_file_new ();
+  g_key_file_set_string (keyfile, "a\tb", "key1", "123");
+  value = g_key_file_get_string (keyfile, "a\tb", "key1", &error);
+  check_error (&error, 
+               G_KEY_FILE_ERROR,
+               G_KEY_FILE_ERROR_GROUP_NOT_FOUND);  
+  g_key_file_free (keyfile);  
+
+  keyfile = g_key_file_new ();
+  g_key_file_set_string (keyfile, "\xc2\xbd", "key1", "123");
+  check_string_value (keyfile, "\xc2\xbd", "key1", "123");
+  g_key_file_free (keyfile);  
+}
+
+static void
+test_key_names (void)
+{
+  GKeyFile *keyfile;
+  GError *error = NULL;
+  const gchar *data;
+  gchar *value;
+
+  /* [ in key name */
+  data = "[a]\n"
+         "key[=123\n";
+  keyfile = g_key_file_new ();
+  g_key_file_load_from_data (keyfile, data, -1, 0, &error);
+  g_key_file_free (keyfile);  
+  check_error (&error, 
+	       G_KEY_FILE_ERROR,
+	       G_KEY_FILE_ERROR_PARSE);
+
+  /* + in key name */
+  data = "[a]\n"
+         "key+foo=123\n";
+  keyfile = g_key_file_new ();
+  g_key_file_load_from_data (keyfile, data, -1, 0, &error);
+  g_key_file_free (keyfile);  
+  check_error (&error, 
+	       G_KEY_FILE_ERROR,
+	       G_KEY_FILE_ERROR_PARSE);
+
+  /* control char in key name */
+  data = "[a]\n"
+         "key\tfoo=123\n";
+  keyfile = g_key_file_new ();
+  g_key_file_load_from_data (keyfile, data, -1, 0, &error);
+  g_key_file_free (keyfile);  
+  check_error (&error, 
+	       G_KEY_FILE_ERROR,
+	       G_KEY_FILE_ERROR_PARSE);
+
+  /* Unicode in key name */
+  data = "[a]\n"
+         "\xc2\xbd=123\n";
+  keyfile = g_key_file_new ();
+  g_key_file_load_from_data (keyfile, data, -1, 0, &error);
+  g_key_file_free (keyfile);  
+  check_no_error (&error); 
+
+  keyfile = g_key_file_new ();
+  g_key_file_set_string (keyfile, "a", "x", "123");
+  g_key_file_set_string (keyfile, "a", "key=", "123");
+  value = g_key_file_get_string (keyfile, "a", "key=", &error);
+  check_error (&error, 
+               G_KEY_FILE_ERROR,
+               G_KEY_FILE_ERROR_KEY_NOT_FOUND);  
+  g_key_file_free (keyfile);  
+
+  keyfile = g_key_file_new ();
+  g_key_file_set_string (keyfile, "a", "x", "123");
+  g_key_file_set_string (keyfile, "a", "key[", "123");
+  value = g_key_file_get_string (keyfile, "a", "key[", &error);
+  check_error (&error, 
+               G_KEY_FILE_ERROR,
+               G_KEY_FILE_ERROR_KEY_NOT_FOUND);  
+  g_key_file_free (keyfile);  
+
+  keyfile = g_key_file_new ();
+  g_key_file_set_string (keyfile, "a", "x", "123");
+  g_key_file_set_string (keyfile, "a", "key+foo", "123");
+  value = g_key_file_get_string (keyfile, "a", "key+foo", &error);
+  check_error (&error, 
+               G_KEY_FILE_ERROR,
+               G_KEY_FILE_ERROR_KEY_NOT_FOUND);  
+  g_key_file_free (keyfile);  
+
+  keyfile = g_key_file_new ();
+  g_key_file_set_string (keyfile, "a", "x", "123");
+  g_key_file_set_string (keyfile, "a", "key\tfoo", "123");
+  value = g_key_file_get_string (keyfile, "a", "key\tfoo", &error);
+  check_error (&error, 
+               G_KEY_FILE_ERROR,
+               G_KEY_FILE_ERROR_KEY_NOT_FOUND);  
+  g_key_file_free (keyfile);  
+
+  keyfile = g_key_file_new ();
+  g_key_file_set_string (keyfile, "a", "x", "123");
+  g_key_file_set_string (keyfile, "a", " key", "123");
+  value = g_key_file_get_string (keyfile, "a", " key", &error);
+  check_error (&error, 
+               G_KEY_FILE_ERROR,
+               G_KEY_FILE_ERROR_KEY_NOT_FOUND);  
+  g_key_file_free (keyfile);  
+
+  keyfile = g_key_file_new ();
+  g_key_file_set_string (keyfile, "a", "x", "123");
+  g_key_file_set_string (keyfile, "a", "\xc2\xbd", "123");
+  check_string_value (keyfile, "a", "\xc2\xbd", "123");
+  g_key_file_free (keyfile);  
+}
+
 static void
 test_duplicate_keys (void)
 {
@@ -1027,9 +1183,19 @@ test_duplicate_groups2 (void)
   g_key_file_free (keyfile);  
 }
 
+static void 
+log_func (const gchar   *log_domain,
+          GLogLevelFlags log_level,
+          const gchar   *message,
+          gpointer       user_data)
+{
+}
+
 int
 main (int argc, char *argv[])
 {
+  g_log_set_default_handler (log_func, NULL);
+
   test_line_ends ();
   test_whitespace ();
   test_comments ();
@@ -1045,6 +1211,8 @@ main (int argc, char *argv[])
   test_duplicate_keys ();
   test_duplicate_groups ();
   test_duplicate_groups2 ();
+  test_group_names ();
+  test_key_names ();
   
   return 0;
 }