2003-04-08 Havoc Pennington <hp@redhat.com>

	* test/data/invalid-messages/array-with-mixed-types.message:
	regression test that fails for the moment

	* dbus/dbus-test.c (dbus_internal_do_not_use_run_tests): reorder
	tests for convenience

	* dbus/dbus-marshal.c (_dbus_marshal_validate_arg): don't allow
	array of nil, it broke things.

	* test/data/invalid-messages/array-of-nil.message: regression test

	* test/data/valid-messages/array-of-array-of-uint32.message:
	happened to write this so added it to suite

6 files changed, 176 insertions, 33 deletions

diff --git a/ChangeLog b/ChangeLog
index a81b2d9e..471eb8ae 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,5 +1,21 @@
 2003-04-08  Havoc Pennington  <hp@redhat.com>
 
+	* test/data/invalid-messages/array-with-mixed-types.message:
+	regression test that fails for the moment
+
+	* dbus/dbus-test.c (dbus_internal_do_not_use_run_tests): reorder
+	tests for convenience
+
+	* dbus/dbus-marshal.c (_dbus_marshal_validate_arg): don't allow
+	array of nil, it broke things.
+
+	* test/data/invalid-messages/array-of-nil.message: regression test
+
+	* test/data/valid-messages/array-of-array-of-uint32.message:
+	happened to write this so added it to suite
+	
+2003-04-08  Havoc Pennington  <hp@redhat.com>
+
 	* bus/driver.c (bus_driver_handle_acquire_service): init
 	retval/reply before checking name
 
diff --git a/dbus/dbus-marshal.c b/dbus/dbus-marshal.c
index 83a3e6f2..b1eb6fc7 100644
--- a/dbus/dbus-marshal.c
+++ b/dbus/dbus-marshal.c
@@ -1,7 +1,8 @@
 /* -*- mode: C; c-file-style: "gnu" -*- */
 /* dbus-marshal.c  Marshalling routines
  *
- * Copyright (C) 2002  CodeFactory AB
+ * Copyright (C) 2002 CodeFactory AB
+ * Copyright (C) 2003 Red Hat, Inc.
  *
  * Licensed under the Academic Free License version 1.2
  * 
@@ -1344,7 +1345,17 @@ _dbus_marshal_validate_arg (const DBusString *str,
 	    _dbus_verbose ("invalid array type\n");
 	    return FALSE;
 	  }
-	
+
+        /* NIL values take up no space, so you couldn't iterate over an array of them.
+         * array of nil seems useless anyway; the useful thing might be array of
+         * (nil OR string) but we have no framework for that.
+         */
+        if (array_type == DBUS_TYPE_NIL)
+          {
+            _dbus_verbose ("array of NIL is not allowed\n");
+            return FALSE;
+          }
+        
 	len = demarshal_and_validate_len (str, byte_order, pos, &pos);
         if (len < 0)
           return FALSE;
@@ -1356,17 +1367,27 @@ _dbus_marshal_validate_arg (const DBusString *str,
           }
 	
 	end = pos + len;
-	
+        
 	while (pos < end)
 	  {
 	    if (!_dbus_marshal_validate_arg (str, byte_order, depth + 1,
 					     array_type, pos, &pos))
 	      return FALSE;
 	  }
-	
+
+        if (pos < end)
+          {
+            /* This should not be able to happen, as long as validate_arg moves forward;
+             * but the check is here just to be paranoid.
+             */
+            _dbus_verbose ("array length %d specified was longer than actual array contents by %d\n",
+                    len, end - pos);
+            return FALSE;
+          }
+        
 	if (pos > end)
 	  {
-	    _dbus_verbose ("array contents exceeds array length\n");
+	    _dbus_verbose ("array contents exceeds array length %d by %d\n", len, pos - end);
 	    return FALSE;
 	  }
 
@@ -1374,7 +1395,7 @@ _dbus_marshal_validate_arg (const DBusString *str,
       }
       break;
 
-      case DBUS_TYPE_DICT:
+    case DBUS_TYPE_DICT:
       {
 	int dict_type;
 	int len;
@@ -1413,9 +1434,10 @@ _dbus_marshal_validate_arg (const DBusString *str,
 	
 	if (pos > end)
 	  {
-	    _dbus_verbose ("dict contents exceeds array length\n");
+	    _dbus_verbose ("dict contents exceed stated dict length\n");
 	    return FALSE;
 	  }
+        
 	*end_pos = pos;
       }
       break;
diff --git a/dbus/dbus-test.c b/dbus/dbus-test.c
index 2d1b5477..f7d1c4ef 100644
--- a/dbus/dbus-test.c
+++ b/dbus/dbus-test.c
@@ -100,32 +100,6 @@ dbus_internal_do_not_use_run_tests (const char *test_data_dir)
 
   check_memleaks ();
   
-  printf ("%s: running keyring tests\n", "dbus-test");
-  if (!_dbus_keyring_test ())
-    die ("keyring");
-
-  check_memleaks ();
-  
-#if 0
-  printf ("%s: running md5 tests\n", "dbus-test");
-  if (!_dbus_md5_test ())
-    die ("md5");
-
-  check_memleaks ();
-#endif
-  
-  printf ("%s: running SHA-1 tests\n", "dbus-test");
-  if (!_dbus_sha_test (test_data_dir))
-    die ("SHA-1");
-
-  check_memleaks ();
-  
-  printf ("%s: running auth tests\n", "dbus-test");
-  if (!_dbus_auth_test (test_data_dir))
-    die ("auth");
-
-  check_memleaks ();
-  
   printf ("%s: running address parse tests\n", "dbus-test");
   if (!_dbus_address_test ())
     die ("address parsing");
@@ -162,6 +136,32 @@ dbus_internal_do_not_use_run_tests (const char *test_data_dir)
 
   check_memleaks ();
   
+  printf ("%s: running keyring tests\n", "dbus-test");
+  if (!_dbus_keyring_test ())
+    die ("keyring");
+
+  check_memleaks ();
+  
+#if 0
+  printf ("%s: running md5 tests\n", "dbus-test");
+  if (!_dbus_md5_test ())
+    die ("md5");
+
+  check_memleaks ();
+#endif
+  
+  printf ("%s: running SHA-1 tests\n", "dbus-test");
+  if (!_dbus_sha_test (test_data_dir))
+    die ("SHA-1");
+
+  check_memleaks ();
+  
+  printf ("%s: running auth tests\n", "dbus-test");
+  if (!_dbus_auth_test (test_data_dir))
+    die ("auth");
+
+  check_memleaks ();
+  
   printf ("%s: completed successfully\n", "dbus-test");
 #else
   printf ("Not compiled with unit tests, not running any\n");
diff --git a/test/data/invalid-messages/array-of-nil.message b/test/data/invalid-messages/array-of-nil.message
new file mode 100644
index 00000000..e86e6a10
--- /dev/null
+++ b/test/data/invalid-messages/array-of-nil.message
@@ -0,0 +1,22 @@
+# Message with an array of NIL (not allowed)
+
+VALID_HEADER
+FIELD_NAME name
+TYPE STRING
+STRING 'org.freedesktop.Foo'
+END_LENGTH Header
+ALIGN 8
+START_LENGTH Body
+TYPE ARRAY
+TYPE NIL
+UINT32 5
+
+## we want it to fail because of type nil, not because the length is no good
+## so pad out the message with 5 bytes
+BYTE 1
+BYTE 2
+BYTE 3
+BYTE 4
+BYTE 5
+
+END_LENGTH Body
diff --git a/test/data/invalid-messages/array-with-mixed-types.message b/test/data/invalid-messages/array-with-mixed-types.message
new file mode 100644
index 00000000..a7ff1e69
--- /dev/null
+++ b/test/data/invalid-messages/array-with-mixed-types.message
@@ -0,0 +1,47 @@
+# Message with an array of array where the child arrays are of
+# different types
+
+VALID_HEADER
+FIELD_NAME name
+TYPE STRING
+STRING 'org.freedesktop.Foo'
+END_LENGTH Header
+ALIGN 8
+START_LENGTH Body
+
+TYPE ARRAY
+TYPE ARRAY
+
+LENGTH Array
+START_LENGTH Array
+
+## array of uint32
+TYPE UINT32
+LENGTH SubArray1
+START_LENGTH SubArray1
+UINT32 1
+UINT32 2
+UINT32 3
+END_LENGTH SubArray1
+
+## array of uint32
+TYPE UINT32
+LENGTH SubArray2
+START_LENGTH SubArray2
+UINT32 4
+UINT32 5
+END_LENGTH SubArray2
+
+## array of boolean
+TYPE BOOLEAN
+LENGTH SubArray3
+START_LENGTH SubArray3
+BOOLEAN false
+BOOLEAN true
+BOOLEAN false
+BOOLEAN true
+END_LENGTH SubArray3
+
+END_LENGTH Array
+
+END_LENGTH Body
diff --git a/test/data/valid-messages/array-of-array-of-uint32.message b/test/data/valid-messages/array-of-array-of-uint32.message
new file mode 100644
index 00000000..6baf4c36
--- /dev/null
+++ b/test/data/valid-messages/array-of-array-of-uint32.message
@@ -0,0 +1,36 @@
+# Message with an array of array of uint32
+
+VALID_HEADER
+FIELD_NAME name
+TYPE STRING
+STRING 'org.freedesktop.Foo'
+END_LENGTH Header
+ALIGN 8
+START_LENGTH Body
+
+TYPE ARRAY
+TYPE ARRAY
+
+LENGTH Array
+START_LENGTH Array
+
+## array of uint32
+TYPE UINT32
+LENGTH SubArray1
+START_LENGTH SubArray1
+UINT32 1
+UINT32 2
+UINT32 3
+END_LENGTH SubArray1
+
+## array of uint32
+TYPE UINT32
+LENGTH SubArray2
+START_LENGTH SubArray2
+UINT32 4
+UINT32 5
+END_LENGTH SubArray2
+
+END_LENGTH Array
+
+END_LENGTH Body