Status of KeyFile regressions in glib2.0
Hi,
First, I'm the one who uploaded the new upstream release of glib2.0
with the regressions you noticed in Gnucash. I apologize for not
testing this new release for a longer time: it only spent a single day
in experimental and on my system; Sébastien Bacher suggested that I
should test core libs such as glib2.0 for a longer time before upload.
(Please note that the new upstream release did not bump shlibs, so I
didn't expect it to hijack the etch release cycle; it still caused an
inconvenient situation obviously.)
Given that the problem not only hit gnome-vfs2 but also Gnucash, I can
only agree that the strictness in KeyFile checks is too intrusive;
hence I've added a patch reverting to the older checks but also
outputting warnings when the new checks fail. (Attached.)
Finally, since glib's upstream added support for key names with MIME
types chars such as "/" and "+" for gnome-vfs2, I also added a patch to
silently support key names with spaces in the middle of the name (not
at the beginning or at the end) as I think this might be relatively
common. (Attached as well.)
Please let me know if the patches still cause regressions or introduce
new regressions via bug reports against glib2.0.
It would be best if Gnucash switched as soon as possible to stricter
key names.
The first patch will probably be reverted in the lenny cycle, and the
second patch might be kept if upstream merges it.
Happy new year!
Bye,
--
Loïc Minier <lool@dooz.org>
"Forget your stupid theme park! I'm gonna make my own! With hookers!
And blackjack! In fact, forget the theme park!" -- Bender
--- /home/lool/tarballs/gnome/glib/2.12/glib-2.12.6/glib/gkeyfile.c 2006-12-19 22:03:00.000000000 +0100
+++ glib-2.12.6/glib/gkeyfile.c 2006-12-31 19:22:33.000000000 +0100
@@ -753,11 +753,15 @@ g_key_file_parse_group (GKeyFile *ke
if (!g_key_file_is_group_name (group_name))
{
+ /* Debian: print a warning instead of failing */
+ g_critical(_("Invalid group name: %s"), group_name);
+#if 0
g_set_error (error, G_KEY_FILE_ERROR,
G_KEY_FILE_ERROR_PARSE,
_("Invalid group name: %s"), group_name);
g_free (group_name);
return;
+#endif
}
g_key_file_add_group (key_file, group_name);
@@ -801,11 +805,15 @@ g_key_file_parse_key_value_pair (GKeyFil
if (!g_key_file_is_key_name (key))
{
+ /* Debian: print a warning instead of failing */
+ g_critical(_("Invalid key name: %s"), key);
+#if 0
g_set_error (error, G_KEY_FILE_ERROR,
G_KEY_FILE_ERROR_PARSE,
_("Invalid key name: %s"), key);
g_free (key);
return;
+#endif
}
/* Pull the value from the line (chugging leading whitespace)
@@ -1238,8 +1246,22 @@ g_key_file_set_value (GKeyFile *key_f
GKeyFileKeyValuePair *pair;
g_return_if_fail (key_file != NULL);
+ /* Debian: restore older and simpler checks for NULLity; convert the new
+ * checks to warnings */
+ g_return_if_fail (group_name != NULL);
+ g_return_if_fail (key != NULL);
+ if (G_LIKELY(g_key_file_is_group_name (group_name))) { } else
+ {
+ g_critical(_("Invalid group name: %s"), group_name);
+ }
+ if (G_LIKELY(g_key_file_is_key_name (key))) { } else
+ {
+ g_critical(_("Invalid key name: %s"), key);
+ }
+#if 0
g_return_if_fail (g_key_file_is_group_name (group_name));
g_return_if_fail (g_key_file_is_key_name (key));
+#endif
g_return_if_fail (value != NULL);
group = g_key_file_lookup_group (key_file, group_name);
@@ -1366,6 +1388,10 @@ g_key_file_set_string (GKeyFile *key_
gchar *value;
g_return_if_fail (key_file != NULL);
+ /* Debian: restore checks for NULLity */
+ g_return_if_fail (group_name != NULL);
+ g_return_if_fail (key != NULL);
+ /* end of Debian specific block */
g_return_if_fail (string != NULL);
value = g_key_file_parse_string_as_value (key_file, string, FALSE);
@@ -1490,6 +1516,10 @@ g_key_file_set_string_list (GKeyFile
gsize i;
g_return_if_fail (key_file != NULL);
+ /* Debian: restore checks for NULLity */
+ g_return_if_fail (group_name != NULL);
+ g_return_if_fail (key != NULL);
+ /* end of Debian specific block */
g_return_if_fail (list != NULL);
value_list = g_string_sized_new (length * 128);
@@ -1532,6 +1562,9 @@ g_key_file_set_locale_string (GKeyFile
gchar *full_key, *value;
g_return_if_fail (key_file != NULL);
+ /* Debian: restore checks for NULLity */
+ g_return_if_fail (group_name != NULL);
+ /* end of Debian specific block */
g_return_if_fail (key != NULL);
g_return_if_fail (locale != NULL);
g_return_if_fail (string != NULL);
@@ -1734,6 +1767,9 @@ g_key_file_set_locale_string_list (GKeyF
gsize i;
g_return_if_fail (key_file != NULL);
+ /* Debian: restore checks for NULLity */
+ g_return_if_fail (group_name != NULL);
+ /* end of Debian specific block */
g_return_if_fail (key != NULL);
g_return_if_fail (locale != NULL);
g_return_if_fail (length != 0);
@@ -1843,6 +1879,10 @@ g_key_file_set_boolean (GKeyFile *key
gchar *result;
g_return_if_fail (key_file != NULL);
+ /* Debian: restore checks for NULLity */
+ g_return_if_fail (group_name != NULL);
+ g_return_if_fail (key != NULL);
+ /* end of Debian specific block */
result = g_key_file_parse_boolean_as_value (key_file, value);
g_key_file_set_value (key_file, group_name, key, result);
@@ -1947,6 +1987,10 @@ g_key_file_set_boolean_list (GKeyFile
gsize i;
g_return_if_fail (key_file != NULL);
+ /* Debian: restore checks for NULLity */
+ g_return_if_fail (group_name != NULL);
+ g_return_if_fail (key != NULL);
+ /* end of Debian specific block */
g_return_if_fail (list != NULL);
value_list = g_string_sized_new (length * 8);
@@ -2055,6 +2099,10 @@ g_key_file_set_integer (GKeyFile *key
gchar *result;
g_return_if_fail (key_file != NULL);
+ /* Debian: restore checks for NULLity */
+ g_return_if_fail (group_name != NULL);
+ g_return_if_fail (key != NULL);
+ /* end of Debian specific block */
result = g_key_file_parse_integer_as_value (key_file, value);
g_key_file_set_value (key_file, group_name, key, result);
@@ -2156,6 +2204,10 @@ g_key_file_set_integer_list (GKeyFile
gsize i;
g_return_if_fail (key_file != NULL);
+ /* Debian: restore checks for NULLity */
+ g_return_if_fail (group_name != NULL);
+ g_return_if_fail (key != NULL);
+ /* end of Debian specific block */
g_return_if_fail (list != NULL);
values = g_string_sized_new (length * 16);
@@ -2265,6 +2317,10 @@ g_key_file_set_double (GKeyFile *key
gchar result[G_ASCII_DTOSTR_BUF_SIZE];
g_return_if_fail (key_file != NULL);
+ /* Debian: restore checks for NULLity */
+ g_return_if_fail (group_name != NULL);
+ g_return_if_fail (key != NULL);
+ /* end of Debian specific block */
g_ascii_dtostr (result, sizeof (result), value);
g_key_file_set_value (key_file, group_name, key, result);
@@ -2366,6 +2422,10 @@ g_key_file_set_double_list (GKeyFile
gsize i;
g_return_if_fail (key_file != NULL);
+ /* Debian: restore checks for NULLity */
+ g_return_if_fail (group_name != NULL);
+ g_return_if_fail (key != NULL);
+ /* end of Debian specific block */
g_return_if_fail (list != NULL);
values = g_string_sized_new (length * 16);
@@ -2459,7 +2519,14 @@ g_key_file_set_group_comment (GKeyFile
{
GKeyFileGroup *group;
+ /* Debian: convert the new checks to warnings */
+ if (G_LIKELY(g_key_file_is_group_name (group_name))) { } else
+ {
+ g_critical(_("Invalid group name: %s"), group_name);
+ }
+#if 0
g_return_if_fail (g_key_file_is_group_name (group_name));
+#endif
group = g_key_file_lookup_group (key_file, group_name);
if (!group)
@@ -2580,7 +2647,14 @@ g_key_file_get_key_comment (GKeyFile
GString *string;
gchar *comment;
+ /* Debian: convert the new checks to warnings */
+ if (G_LIKELY(g_key_file_is_group_name (group_name))) { } else
+ {
+ g_critical(_("Invalid group name: %s"), group_name);
+ }
+#if 0
g_return_val_if_fail (g_key_file_is_group_name (group_name), NULL);
+#endif
group = g_key_file_lookup_group (key_file, group_name);
if (!group)
@@ -2893,7 +2967,16 @@ g_key_file_add_group (GKeyFile *key_f
GKeyFileGroup *group;
g_return_if_fail (key_file != NULL);
+ /* Debian: restore older and simpler checks for NULLity; convert the new
+ * checks to warnings */
+ g_return_if_fail (group_name != NULL);
+ if (G_LIKELY(g_key_file_is_group_name (group_name))) { } else
+ {
+ g_critical(_("Invalid group name: %s"), group_name);
+ }
+#if 0
g_return_if_fail (g_key_file_is_group_name (group_name));
+#endif
group = g_key_file_lookup_group (key_file, group_name);
if (group != NULL)
--- /home/lool/tarballs/gnome/glib/2.12/glib-2.12.6/tests/keyfile-test.c 2006-12-19 22:03:01.000000000 +0100
+++ glib-2.12.6/tests/keyfile-test.c 2006-12-31 19:36:09.000000000 +0100
@@ -972,9 +972,13 @@ test_group_names (void)
keyfile = g_key_file_new ();
g_key_file_load_from_data (keyfile, data, -1, 0, &error);
g_key_file_free (keyfile);
+ /* Debian: tolerate old syntax */
+ check_no_error (&error);
+#if 0
check_error (&error,
G_KEY_FILE_ERROR,
G_KEY_FILE_ERROR_PARSE);
+#endif
/* ] in group name */
data = "[a]b]\n"
@@ -982,9 +986,13 @@ test_group_names (void)
keyfile = g_key_file_new ();
g_key_file_load_from_data (keyfile, data, -1, 0, &error);
g_key_file_free (keyfile);
+ /* Debian: tolerate old syntax */
+ check_no_error (&error);
+#if 0
check_error (&error,
G_KEY_FILE_ERROR,
G_KEY_FILE_ERROR_PARSE);
+#endif
/* control char in group name */
data = "[a\tb]\n"
@@ -992,9 +1000,13 @@ test_group_names (void)
keyfile = g_key_file_new ();
g_key_file_load_from_data (keyfile, data, -1, 0, &error);
g_key_file_free (keyfile);
+ /* Debian: tolerate old syntax */
+ check_no_error (&error);
+#if 0
check_error (&error,
G_KEY_FILE_ERROR,
G_KEY_FILE_ERROR_PARSE);
+#endif
/* Unicode in group name */
data = "[\xc2\xbd]\n"
@@ -1007,25 +1019,37 @@ test_group_names (void)
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);
+ /* Debian: tolerate old syntax */
+ check_no_error (&error);
+#if 0
check_error (&error,
G_KEY_FILE_ERROR,
G_KEY_FILE_ERROR_GROUP_NOT_FOUND);
+#endif
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);
+ /* Debian: tolerate old syntax */
+ check_no_error (&error);
+#if 0
check_error (&error,
G_KEY_FILE_ERROR,
G_KEY_FILE_ERROR_GROUP_NOT_FOUND);
+#endif
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);
+ /* Debian: tolerate old syntax */
+ check_no_error (&error);
+#if 0
check_error (&error,
G_KEY_FILE_ERROR,
G_KEY_FILE_ERROR_GROUP_NOT_FOUND);
+#endif
g_key_file_free (keyfile);
keyfile = g_key_file_new ();
@@ -1048,9 +1072,13 @@ test_key_names (void)
keyfile = g_key_file_new ();
g_key_file_load_from_data (keyfile, data, -1, 0, &error);
g_key_file_free (keyfile);
+ /* Debian: tolerate old syntax */
+ check_no_error (&error);
+#if 0
check_error (&error,
G_KEY_FILE_ERROR,
G_KEY_FILE_ERROR_PARSE);
+#endif
/* control char in key name */
data = "[a]\n"
@@ -1058,9 +1086,13 @@ test_key_names (void)
keyfile = g_key_file_new ();
g_key_file_load_from_data (keyfile, data, -1, 0, &error);
g_key_file_free (keyfile);
+ /* Debian: tolerate old syntax */
+ check_no_error (&error);
+#if 0
check_error (&error,
G_KEY_FILE_ERROR,
G_KEY_FILE_ERROR_PARSE);
+#endif
/* Unicode in key name */
data = "[a]\n"
@@ -1074,37 +1106,53 @@ test_key_names (void)
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);
+ /* Debian: tolerate old syntax */
+ check_no_error (&error);
+#if 0
check_error (&error,
G_KEY_FILE_ERROR,
G_KEY_FILE_ERROR_KEY_NOT_FOUND);
+#endif
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);
+ /* Debian: tolerate old syntax */
+ check_no_error (&error);
+#if 0
check_error (&error,
G_KEY_FILE_ERROR,
G_KEY_FILE_ERROR_KEY_NOT_FOUND);
+#endif
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);
+ /* Debian: tolerate old syntax */
+ check_no_error (&error);
+#if 0
check_error (&error,
G_KEY_FILE_ERROR,
G_KEY_FILE_ERROR_KEY_NOT_FOUND);
+#endif
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);
+ /* Debian: tolerate old syntax */
+ check_no_error (&error);
+#if 0
check_error (&error,
G_KEY_FILE_ERROR,
G_KEY_FILE_ERROR_KEY_NOT_FOUND);
g_key_file_free (keyfile);
+#endif
keyfile = g_key_file_new ();
g_key_file_set_string (keyfile, "a", "x", "123");
--- /home/lool/tarballs/gnome/glib/2.12/glib-2.12.6/glib/gkeyfile.c 2006-12-19 22:03:00.000000000 +0100
+++ glib-2.12.6/glib/gkeyfile.c 2006-12-31 20:38:52.000000000 +0100
@@ -3229,17 +3229,23 @@ static gboolean
g_key_file_is_key_name (const gchar *name)
{
gchar *p, *q;
+ gchar prev_char;
if (name == NULL)
return FALSE;
p = q = (gchar *) name;
/* We accept a little more than the desktop entry spec says,
- * since gnome-vfs uses mime-types as keys in its cache.
+ * since gnome-vfs uses mime-types as keys in its cache, and since some apps
+ * such as Gnucash use space in the middle of key names.
*/
while (*q && (g_unichar_isalnum (g_utf8_get_char (q)) ||
- *q == '-' || *q == '_' || *q == '/' || *q == '+' || *q == '.'))
+ *q == '-' || *q == '_' || *q == '/' || *q == '+' || *q == '.') ||
+ ((q != name) && *q == ' '))
+ {
+ prev_char = g_utf8_get_char (q);
q = g_utf8_next_char (q);
+ }
if (*q == '[')
{
@@ -3256,6 +3262,9 @@ g_key_file_is_key_name (const gchar *nam
if (*q != '\0' || q == p)
return FALSE;
+ if (prev_char == ' ')
+ return FALSE;
+
return TRUE;
}
--- /home/lool/tarballs/gnome/glib/2.12/glib-2.12.6/tests/keyfile-test.c 2006-12-19 22:03:01.000000000 +0100
+++ glib-2.12.6/tests/keyfile-test.c 2006-12-31 20:38:24.000000000 +0100
@@ -418,7 +418,9 @@ test_whitespace (void)
" [ group2 ] \n"
"key3 = value3 \n"
"key4 = value \t4\n"
- " key5 = value5\n";
+ " key5 = value5\n"
+ "[group 3]\n"
+ "key 6 = value6\n";
keyfile = load_data (data, 0);
@@ -427,6 +429,7 @@ test_whitespace (void)
check_string_value (keyfile, " group2 ", "key3", "value3 ");
check_string_value (keyfile, " group2 ", "key4", "value \t4");
check_string_value (keyfile, " group2 ", "key5", "value5");
+ check_string_value (keyfile, "group 3", "key 6", "value6");
g_key_file_free (keyfile);
}
@@ -1108,6 +1111,15 @@ test_key_names (void)
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");
/* Unicode key */
g_key_file_set_string (keyfile, "a", "\xc2\xbd", "123");
@@ -1121,6 +1133,10 @@ test_key_names (void)
g_key_file_set_string (keyfile, "a", "foo.bar", ".");
check_string_value (keyfile, "a", "foo.bar", ".");
+ /* Keys with space (as used by Gnucash) */
+ g_key_file_set_string (keyfile, "a", "foo bar", "baz");
+ check_string_value (keyfile, "a", "foo bar", "baz");
+
g_key_file_free (keyfile);
}
Reply to: