Rework the handling of invalid keys/groups again. We are back to being

2007-01-12  Matthias Clasen  <mclasen@redhat.com>

        * glib/gkeyfile.c: Rework the handling of invalid
        keys/groups again. We are back to being liberal about
        what we accept, and only reject things that would lead
        to non-rereadable keyfiles.

        * tests/keyfile-test.c: Adapt tests.



svn path=/trunk/; revision=5254

ChangeLog

@@ -1,3 +1,12 @@
+2007-01-12  Matthias Clasen  <mclasen@redhat.com>
+
+	* glib/gkeyfile.c: Rework the handling of invalid
+	keys/groups again. We are back to being liberal about
+	what we accept, and only reject things that would lead
+	to non-rereadable keyfiles.
+
+	* tests/keyfile-test.c: Adapt tests.
+
 2007-01-12  Matthias Clasen  <mclasen@redhat.com>
 
 	* glib/gutils.c (g_get_home_dir): Clarify docs.  (#394687,

docs/reference/ChangeLog

@@ -1,3 +1,7 @@
+2007-01-12  Matthias Clasen  <mclasen@redhat.com>
+
+	* glib/tmpl/keyfile.sgml: Small clarifications.
+
 2007-01-03  Behdad Esfahbod  <behdad@gnome.org>
 
 	* glib/glib-sections.txt: Add g_unichar_iszerowidth.

docs/reference/glib/tmpl/keyfile.sgml

@@ -60,8 +60,8 @@ Key-value pairs generally have the form <literal>key=value</literal>,
 with the exception of localized strings, which have the form 
 <literal>key[locale]=value</literal>. Space before and after the 
 '=' character are ignored. Newline, tab, carriage return and backslash 
-characters are escaped as \n, \t, \r, and \\, respectively. To preserve
+characters in value are escaped as \n, \t, \r, and \\, respectively. 
-leading spaces in values, these can also be escaped as \s.
+To preserve leading spaces in values, these can also be escaped as \s.
 </para>
 
 <para>

glib/gkeyfile.c

@@ -3237,10 +3237,21 @@ g_key_file_is_key_name (const gchar *name)
   /* We accept a little more than the desktop entry spec says,
    * since gnome-vfs uses mime-types as keys in its cache.
    */
-  while (*q && (g_unichar_isalnum (g_utf8_get_char (q)) || 
+  while (*q && *q != '=' && *q != '[' && *q != ']')
-                *q == '-' || *q == '_' || *q == '/' || *q == '+' || *q == '.' || *q == '*')) 
     q = g_utf8_next_char (q);
   
+  /* No empty keys, please */
+  if (q == p)
+    return FALSE;
+
+  /* We accept spaces in the middle of keys to not break
+   * existing apps, but we don't tolerate initial of final
+   * spaces, which would lead to silent corruption when
+   * rereading the file.
+   */
+  if (*p == ' ' || q[-1] == ' ')
+    return FALSE;
+
   if (*q == '[')
     {
       q++;
@@ -3253,7 +3264,7 @@ g_key_file_is_key_name (const gchar *name)
       q++;
     }
 
-  if (*q != '\0' || q == p)
+  if (*q != '\0')
     return FALSE;
 
   return TRUE;
@@ -3276,7 +3287,15 @@ g_key_file_line_is_group (const gchar *line)
   while (*p && *p != ']')
     p = g_utf8_next_char (p);
 
-  if (!*p)
+  if (*p != ']')
+    return FALSE;
+ 
+  /* silently accept whitespace after the ] */
+  p = g_utf8_next_char (p);
+  while (*p == ' ' || *p == '\t')
+    p = g_utf8_next_char (p);
+     
+  if (*p)
     return FALSE;
 
   return TRUE;

tests/keyfile-test.c

@@ -996,6 +996,16 @@ test_group_names (void)
 	       G_KEY_FILE_ERROR,
 	       G_KEY_FILE_ERROR_PARSE);
 
+  /* empty group name */
+  data = "[]\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";
@@ -1052,9 +1062,9 @@ test_key_names (void)
 	       G_KEY_FILE_ERROR,
 	       G_KEY_FILE_ERROR_PARSE);
 
-  /* control char in key name */
+  /* empty key name */
   data = "[a]\n"
-         "key\tfoo=123\n";
+         " =123\n";
   keyfile = g_key_file_new ();
   g_key_file_load_from_data (keyfile, data, -1, 0, &error);
   g_key_file_free (keyfile);  
@@ -1062,6 +1072,71 @@ test_key_names (void)
 	       G_KEY_FILE_ERROR,
 	       G_KEY_FILE_ERROR_PARSE);
 
+  /* empty key name */
+  data = "[a]\n"
+         " [de] =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);
+
+  /* bad locale suffix */
+  data = "[a]\n"
+         "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);
+
+  /* initial space */
+  data = "[a]\n"
+         " foo=123\n";
+  keyfile = g_key_file_new ();
+  g_key_file_load_from_data (keyfile, data, -1, 0, &error);
+  check_no_error (&error);
+  check_string_value (keyfile, "a", "foo", "123");
+  g_key_file_free (keyfile);  
+
+  /* final space */
+  data = "[a]\n"
+         "foo =123\n";
+  keyfile = g_key_file_new ();
+  g_key_file_load_from_data (keyfile, data, -1, 0, &error);
+  check_no_error (&error);
+  check_string_value (keyfile, "a", "foo", "123");
+  g_key_file_free (keyfile);  
+
+  /* inner space */
+  data = "[a]\n"
+         "foo bar=123\n";
+  keyfile = g_key_file_new ();
+  g_key_file_load_from_data (keyfile, data, -1, 0, &error);
+  check_no_error (&error);
+  check_string_value (keyfile, "a", "foo bar", "123");
+  g_key_file_free (keyfile);  
+
+  /* inner space */
+  data = "[a]\n"
+         "foo [de] =123\n";
+  keyfile = g_key_file_new ();
+  g_key_file_load_from_data (keyfile, data, -1, 0, &error);
+  check_error (&error, 
+	       G_KEY_FILE_ERROR,
+	       G_KEY_FILE_ERROR_PARSE);
+  g_key_file_free (keyfile);  
+
+  /* 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_no_error (&error);
+
   /* Unicode in key name */
   data = "[a]\n"
          "\xc2\xbd=123\n";
@@ -1092,9 +1167,7 @@ test_key_names (void)
   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, 
+  check_no_error (&error);
-               G_KEY_FILE_ERROR,
-               G_KEY_FILE_ERROR_KEY_NOT_FOUND);  
   g_key_file_free (keyfile);  
 
   keyfile = g_key_file_new ();