2004-04-12 Jon Trowbridge <trow@ximian.com>

* bus/config-parser.c (struct BusConfigParser): Added
	    included_files field.
	    (seen_include): Added.  Checks whether or not a file has already
	    been included by any parent BusConfigParser.
	    (bus_config_parser_new): Copy the parent's included_files.
	    (include_file): Track which files have been included, and fail on
	    circular inclusions.
	    (process_test_valid_subdir): Changed printf to report if we are
	    testing valid or invalid conf files.
	    (all_are_equiv): Changed printf to be a bit clearer about
	    what we are actually doing.
	    (bus_config_parser_test): Test invalid configuration files.

ChangeLog

@@ -1,3 +1,18 @@
+2004-04-12  Jon Trowbridge  <trow@ximian.com>
+
+	* bus/config-parser.c (struct BusConfigParser): Added
+	included_files field.
+	(seen_include): Added.  Checks whether or not a file has already
+	been included by any parent BusConfigParser.
+	(bus_config_parser_new): Copy the parent's included_files.
+	(include_file): Track which files have been included, and fail on
+	circular inclusions.
+	(process_test_valid_subdir): Changed printf to report if we are
+	testing valid or invalid conf files.
+	(all_are_equiv): Changed printf to be a bit clearer about
+	what we are actually doing.
+	(bus_config_parser_test): Test invalid configuration files.
+
 2004-04-09  Jon Trowbridge  <trow@ximian.com>
 
 	* bus/config-parser.c (bus_config_parser_new): Added a 'parent'

bus/config-parser.c

@@ -115,6 +115,8 @@ struct BusConfigParser
 
   char *pidfile;         /**< PID file */
 
+  DBusList *included_files;  /**< Included files stack */
+
   unsigned int fork : 1; /**< TRUE to fork into daemon mode */
 
   unsigned int is_toplevel : 1; /**< FALSE if we are a sub-config-file inside another one */
@@ -277,6 +279,24 @@ merge_included (BusConfigParser *parser,
   return TRUE;
 }
 
+static dbus_bool_t
+seen_include (BusConfigParser  *parser,
+	      const DBusString *file)
+{
+  DBusList *iter;
+
+  iter = parser->included_files;
+  while (iter != NULL)
+    {
+      if (! strcmp (_dbus_string_get_const_data (file), iter->data))
+	return TRUE;
+
+      iter = _dbus_list_get_next_link (&parser->included_files, iter);
+    }
+
+  return FALSE;
+}
+
 BusConfigParser*
 bus_config_parser_new (const DBusString      *basedir,
                        dbus_bool_t            is_toplevel,
@@ -311,6 +331,10 @@ bus_config_parser_new (const DBusString      *basedir,
     {
       /* Initialize the parser's limits from the parent. */
       parser->limits = parent->limits;
+
+      /* Use the parent's list of included_files to avoid
+	 circular inclusions. */
+      parser->included_files = parent->included_files;
     }
   else
     {
@@ -403,7 +427,7 @@ bus_config_parser_unref (BusConfigParser *parser)
 
       if (parser->policy)
         bus_policy_unref (parser->policy);
-      
+
       dbus_free (parser);
     }
 }
@@ -1635,14 +1659,34 @@ include_file (BusConfigParser   *parser,
    * that the result is the same
    */
   BusConfigParser *included;
+  const char *filename_str;
   DBusError tmp_error;
         
   dbus_error_init (&tmp_error);
 
+  filename_str = _dbus_string_get_const_data (filename);
+
+  /* Check to make sure this file hasn't already been included. */
+  if (seen_include (parser, filename))
+    {
+      dbus_set_error (error, DBUS_ERROR_FAILED,
+		      "Circular inclusion of file '%s'",
+		      filename_str);
+      return FALSE;
+    }
+  
+  if (! _dbus_list_append (&parser->included_files, (void *) filename_str))
+    {
+      BUS_SET_OOM (error);
+      return FALSE;
+    }
+
   /* Since parser is passed in as the parent, included
      inherits parser's limits. */
   included = bus_config_load (filename, FALSE, parser, &tmp_error);
 
+  _dbus_list_pop_last (&parser->included_files);
+
   if (included == NULL)
     {
       _DBUS_ASSERT_ERROR_IS_SET (&tmp_error);
@@ -2207,7 +2251,12 @@ process_test_valid_subdir (const DBusString *test_base_dir,
       goto failed;
     }
 
-  printf ("Testing:\n");
+  if (validity == VALID)
+    printf ("Testing valid files:\n");
+  else if (validity == INVALID)
+    printf ("Testing invalid files:\n");
+  else
+    printf ("Testing unknown files:\n");
 
  next:
   while (_dbus_directory_get_next_file (dir, &filename, &error))
@@ -2458,7 +2507,7 @@ all_are_equiv (const DBusString *target_directory)
       goto finished;
     }
 
-  printf ("Comparing:\n");
+  printf ("Comparing equivalent files:\n");
 
  next:
   while (_dbus_directory_get_next_file (dir, &filename, &error))
@@ -2612,6 +2661,9 @@ bus_config_parser_test (const DBusString *test_data_dir)
   if (!process_test_valid_subdir (test_data_dir, "valid-config-files", VALID))
     return FALSE;
 
+  if (!process_test_valid_subdir (test_data_dir, "invalid-config-files", INVALID))
+    return FALSE;
+
   if (!process_test_equiv_subdir (test_data_dir, "equiv-config-files"))
     return FALSE;
 

doc/TODO

@@ -47,11 +47,6 @@
    has rules for it anyway, or something. it's conceptually 
    screwy at the moment.
 
- - <limit> elements are not merged in from included configuration 
-   files; they have to be in the toplevel file. when loading 
-   a child file, we could just init its DBusLimits from the parent, 
-   then after parsing copy its DBusLimits back to the parent
-
  - when making a method call, if the call serial were globally unique,
    we could forward the call serial along with any method calls made
    as a result of the first method call, and allow reentrancy that was

test/Makefile.am

@@ -69,7 +69,14 @@ TESTDIRS=					\
 	data/valid-config-files			\
 	data/valid-config-files/basic.d		\
 	data/valid-config-files/system.d	\
-	data/valid-service-files
+	data/valid-service-files		\
+	data/invalid-config-files		\
+	data/equiv-config-files			\
+	data/equiv-config-files/basic		\
+	data/equiv-config-files/basic/basic.d	\
+	data/equiv-config-files/entities	\
+	data/equiv-config-files/entities/basic.d
+
 
 FIND_TESTS=find -name "*.message" -o -name "*.message-raw" -o -name "*.auth-script" -o -name "*.sha1" -o -name "*.txt" -o -name "*.conf" -o -name "*.service"
 

test/data/invalid-config-files/circular-1.conf

@@ -0,0 +1,4 @@
+<!DOCTYPE busconfig PUBLIC "-//freedesktop//DTD D-BUS Bus Configuration 1.0//EN" "http://www.freedesktop.org/standards/dbus/1.0/busconfig.dtd">
+<busconfig>
+<include>circular-1.conf</include>
+</busconfig>

test/data/invalid-config-files/circular-2.conf

@@ -0,0 +1,4 @@
+<!DOCTYPE busconfig PUBLIC "-//freedesktop//DTD D-BUS Bus Configuration 1.0//EN" "http://www.freedesktop.org/standards/dbus/1.0/busconfig.dtd">
+<busconfig>
+<include>circular-3.conf</include>
+</busconfig>

test/data/invalid-config-files/circular-3.conf

@@ -0,0 +1,4 @@
+<!DOCTYPE busconfig PUBLIC "-//freedesktop//DTD D-BUS Bus Configuration 1.0//EN" "http://www.freedesktop.org/standards/dbus/1.0/busconfig.dtd">
+<busconfig>
+<include>circular-2.conf</include>
+</busconfig>