[dconf] engine: improve robustness of profile parsing



commit bc02d65a6c8defcee64022ba2f9a2607495281f7
Author: Ryan Lortie <desrt desrt ca>
Date:   Thu Jul 19 10:47:50 2012 -0400

    engine: improve robustness of profile parsing
    
    Checking for 'u' or 's' is really insanely silly from a robustness
    standpoint.  Ensure that we properly have "user-db:" or "system-db:" and
    that a non-empty database name is given, warning if not.
    
    This was specifically causing annoying problems with the profile file
    that gdm was installing, so add a copy of that file to our testcases.

 engine/dconf-engine-profile.c |    8 +++++-
 engine/dconf-engine-source.c  |   55 ++++++++++++++++++++++++++--------------
 tests/Makefile.am             |    1 +
 tests/engine.c                |   12 +++++++++
 tests/profile/gdm             |    2 +
 5 files changed, 58 insertions(+), 20 deletions(-)
---
diff --git a/engine/dconf-engine-profile.c b/engine/dconf-engine-profile.c
index 0413f32..7415406 100644
--- a/engine/dconf-engine-profile.c
+++ b/engine/dconf-engine-profile.c
@@ -100,6 +100,7 @@ dconf_engine_default_profile (gint *n_sources)
 static DConfEngineSource *
 dconf_engine_profile_handle_line (gchar *line)
 {
+  DConfEngineSource *source;
   gchar *end;
 
   /* remove whitespace at the front */
@@ -119,7 +120,12 @@ dconf_engine_profile_handle_line (gchar *line)
 
   *end = '\0';
 
-  return dconf_engine_source_new (line);
+  source = dconf_engine_source_new (line);
+
+  if (source == NULL)
+    g_warning ("unknown dconf database description: %s", line);
+
+  return source;
 }
 
 static DConfEngineSource **
diff --git a/engine/dconf-engine-source.c b/engine/dconf-engine-source.c
index 834644c..2fe58f4 100644
--- a/engine/dconf-engine-source.c
+++ b/engine/dconf-engine-source.c
@@ -76,27 +76,44 @@ dconf_engine_source_new (const gchar *description)
 {
   const DConfEngineSourceVTable *vtable;
   DConfEngineSource *source;
-
-  switch (description[0])
-    {
-    case 's':
-      vtable = &dconf_engine_source_system_vtable;
-      break;
-
-    case 'u':
-      vtable = &dconf_engine_source_user_vtable;
-      break;
-
-    default:
-      g_warning ("unknown dconf database description: %s", description);
-      return NULL;
-    }
-
+  const gchar *colon;
+
+  /* Source descriptions are of the form
+   *
+   *   type:name
+   *
+   * Where type must currently be one of "user-db" or "system-db".
+   *
+   * We first find the colon.
+   */
+  colon = strchr (description, ':');
+
+  /* Ensure that we have a colon and that a database name follows it. */
+  if (colon == NULL || colon[1] == '\0')
+    return NULL;
+
+  /* Check if the part before the colon is "user-db"... */
+  if ((colon == description + 7) && memcmp (description, "user-db", 7) == 0)
+    vtable = &dconf_engine_source_user_vtable;
+
+  /* ...or "system-db" */
+  else if ((colon == description + 9) && memcmp (description, "system-db", 9) == 0)
+    vtable = &dconf_engine_source_system_vtable;
+
+  /* If it's not either of those, we have failed. */
+  else
+    return NULL;
+
+  /* We have had a successful parse.
+   *
+   *  - either user-db: or system-db:
+   *  - non-NULL and non-empty database name
+   *
+   * Create the source.
+   */
   source = g_malloc0 (vtable->instance_size);
   source->vtable = vtable;
-  source->name = strchr (description, ':');
-  if (source->name)
-    source->name = g_strdup (source->name + 1);
+  source->name = g_strdup (colon + 1);
   source->vtable->init (source);
 
   return source;
diff --git a/tests/Makefile.am b/tests/Makefile.am
index 1f15f83..fc77755 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -86,6 +86,7 @@ EXTRA_DIST += \
 	profile/colourful			\
 	profile/dos				\
 	profile/empty-profile			\
+	profile/gdm				\
 	profile/many-sources			\
 	profile/no-newline-longline		\
 	profile/test-profile			\
diff --git a/tests/engine.c b/tests/engine.c
index 5d22f27..20c050f 100644
--- a/tests/engine.c
+++ b/tests/engine.c
@@ -161,6 +161,18 @@ test_profile_parser (void)
   g_test_trap_assert_passed ();
   g_test_trap_assert_stderr ("*WARNING*: unknown dconf database*unknown dconf database*");
 
+  if (g_test_trap_fork (0, G_TEST_TRAP_SILENCE_STDERR))
+    {
+      g_log_set_always_fatal (G_LOG_LEVEL_ERROR);
+
+      sources = dconf_engine_profile_open (SRCDIR "/profile/gdm", &n_sources);
+      g_assert_cmpint (n_sources, ==, 0);
+      g_assert (sources == NULL);
+      exit (0);
+    }
+  g_test_trap_assert_passed ();
+  g_test_trap_assert_stderr ("*WARNING*: unknown dconf database*unknown dconf database*");
+
   test_five_times (SRCDIR "/profile/empty-profile", 0);
   test_five_times (SRCDIR "/profile/test-profile", 1, "test");
   test_five_times (SRCDIR "/profile/colourful", 4,
diff --git a/tests/profile/gdm b/tests/profile/gdm
new file mode 100644
index 0000000..d5a90e5
--- /dev/null
+++ b/tests/profile/gdm
@@ -0,0 +1,2 @@
+user
+gdm



[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]