old WiP: make _dbus_validate_path etc. return a complaintwip-syntax-complaints

3 files changed, 101 insertions, 82 deletions

diff --git a/bus/activation-helper.c b/bus/activation-helper.c
index baba8f04..de30cdb4 100644
--- a/bus/activation-helper.c
+++ b/bus/activation-helper.c
@@ -42,6 +42,7 @@
 
 #include <dbus/dbus-shell.h>
 #include <dbus/dbus-marshal-validate.h>
+#include <dbus/dbus-syntax.h>
 
 static BusDesktopFile *
 desktop_file_for_name (BusConfigParser *parser,
@@ -381,24 +382,6 @@ exec_for_correct_user (char *exec, char *user, DBusError *error)
 }
 
 static dbus_bool_t
-check_bus_name (const char *bus_name,
-                DBusError  *error)
-{
-  DBusString str;
-
-  _dbus_string_init_const (&str, bus_name);
-  if (!_dbus_validate_bus_name (&str, 0, _dbus_string_get_length (&str)))
-    {
-      dbus_set_error (error, DBUS_ERROR_SPAWN_SERVICE_NOT_FOUND,
-                      "bus name '%s' is not a valid bus name\n",
-                      bus_name);
-      return FALSE;
-    }
-  
-  return TRUE;
-}
-
-static dbus_bool_t
 get_correct_parser (BusConfigParser **parser, DBusError *error)
 {
   DBusString config_file;
@@ -537,7 +520,7 @@ run_launch_helper (const char *bus_name,
     goto error;
 
   /* check to see if we have a valid bus name */
-  if (!check_bus_name (bus_name, error))
+  if (!dbus_validate_bus_name (bus_name, error))
     goto error;
 
   /* get the correct parser, either the test or default parser */
diff --git a/dbus/dbus-marshal-validate.c b/dbus/dbus-marshal-validate.c
index 9187a3e9..9c17ede5 100644
--- a/dbus/dbus-marshal-validate.c
+++ b/dbus/dbus-marshal-validate.c
@@ -765,6 +765,23 @@ _dbus_validate_body_with_reason (const DBusString *expected_signature,
     ((c) >= 'a' && (c) <= 'z') ||               \
     ((c) == '_') )
 
+/* shared strings for common reasons to be invalid */
+static const char complaint_string_range[] = "length out of bounds";
+static const char complaint_empty[] = "must not be empty";
+static const char complaint_start_dot[] = "must not start with '.'";
+static const char complaint_end_dot[] = "must not end with '.'";
+static const char complaint_no_dot[] = "must contain '.'";
+static const char complaint_empty_component[] =
+  "components must not be empty";
+static const char complaint_name_char[] =
+  "characters in components must be in [A-Za-z0-9_]";
+static const char complaint_initial_name_char[] =
+  "first characters of components must be in [A-Za-z_]";
+static const char complaint_bus_name_char[] =
+  "characters in components must be in [-A-Za-z0-9_]";
+static const char complaint_initial_bus_name_char[] =
+  "first characters of components in well-known bus names must be in [-A-Za-z_]";
+
 /**
  * Checks that the given range of the string is a valid object path
  * name in the D-Bus protocol. Part of the validation ensures that
@@ -779,9 +796,10 @@ _dbus_validate_body_with_reason (const DBusString *expected_signature,
  * @param str the string
  * @param start first byte index to check
  * @param len number of bytes to check
- * @returns #TRUE if the byte range exists and is a valid name
+ * @returns #NULL if the byte range exists and is a valid name, a
+ *    programmer-readable error string otherwise
  */
-dbus_bool_t
+const char *
 _dbus_validate_path (const DBusString  *str,
                      int                start,
                      int                len)
@@ -795,16 +813,17 @@ _dbus_validate_path (const DBusString  *str,
   _dbus_assert (start <= _dbus_string_get_length (str));
   
   if (len > _dbus_string_get_length (str) - start)
-    return FALSE;
+    return complaint_string_range;
 
   if (len == 0)
-    return FALSE;
+    return complaint_empty;
 
   s = _dbus_string_get_const_data (str) + start;
   end = s + len;
 
   if (*s != '/')
-    return FALSE;
+    return "must start with '/'";
+
   last_slash = s;
   ++s;
 
@@ -813,14 +832,14 @@ _dbus_validate_path (const DBusString  *str,
       if (*s == '/')
         {
           if ((s - last_slash) < 2)
-            return FALSE; /* no empty path components allowed */
+            return complaint_empty_component;
 
           last_slash = s;
         }
       else
         {
           if (_DBUS_UNLIKELY (!VALID_NAME_CHARACTER (*s)))
-            return FALSE;
+            return complaint_name_char;
         }
 
       ++s;
@@ -828,9 +847,12 @@ _dbus_validate_path (const DBusString  *str,
 
   if ((end - last_slash) < 2 &&
       len > 1)
-    return FALSE; /* trailing slash not allowed unless the string is "/" */
+    {
+      /* trailing slash not allowed unless the string is "/" */
+      return "must not end with '/' unless it is exactly \"/\"";
+    }
 
-  return TRUE;
+  return NULL;
 }
 
 const char *
@@ -915,9 +937,10 @@ _dbus_validity_to_error_message (DBusValidity validity)
  * @param str the string
  * @param start first byte index to check
  * @param len number of bytes to check
- * @returns #TRUE if the byte range exists and is a valid name
+ * @returns #NULL if the byte range exists and is a valid name, a
+ *    programmer-readable error string otherwise
  */
-dbus_bool_t
+const char *
 _dbus_validate_interface (const DBusString  *str,
                           int                start,
                           int                len)
@@ -932,13 +955,13 @@ _dbus_validate_interface (const DBusString  *str,
   _dbus_assert (start <= _dbus_string_get_length (str));
 
   if (len > _dbus_string_get_length (str) - start)
-    return FALSE;
+    return complaint_string_range;
 
   if (len > DBUS_MAXIMUM_NAME_LENGTH)
-    return FALSE;
+    return complaint_name_length;
 
   if (len == 0)
-    return FALSE;
+    return complaint_empty;
 
   last_dot = NULL;
   iface = _dbus_string_get_const_data (str) + start;
@@ -949,9 +972,9 @@ _dbus_validate_interface (const DBusString  *str,
    * in the loop. Note we know len > 0
    */
   if (_DBUS_UNLIKELY (*s == '.')) /* disallow starting with a . */
-    return FALSE;
+    return complaint_start_dot;
   else if (_DBUS_UNLIKELY (!VALID_INITIAL_NAME_CHARACTER (*s)))
-    return FALSE;
+    return complaint_initial_name_char;
   else
     ++s;
 
@@ -960,24 +983,24 @@ _dbus_validate_interface (const DBusString  *str,
       if (*s == '.')
         {
           if (_DBUS_UNLIKELY ((s + 1) == end))
-            return FALSE;
+            return complaint_end_dot;
           else if (_DBUS_UNLIKELY (!VALID_INITIAL_NAME_CHARACTER (*(s + 1))))
-            return FALSE;
+            return complaint_initial_name_char;
           last_dot = s;
           ++s; /* we just validated the next char, so skip two */
         }
       else if (_DBUS_UNLIKELY (!VALID_NAME_CHARACTER (*s)))
         {
-          return FALSE;
+          return complaint_name_char;
         }
 
       ++s;
     }
 
   if (_DBUS_UNLIKELY (last_dot == NULL))
-    return FALSE;
+    return complaint_no_dot;
 
-  return TRUE;
+  return NULL;
 }
 
 /**
@@ -991,9 +1014,10 @@ _dbus_validate_interface (const DBusString  *str,
  * @param str the string
  * @param start first byte index to check
  * @param len number of bytes to check
- * @returns #TRUE if the byte range exists and is a valid name
+ * @returns #NULL if the byte range exists and is a valid name, a
+ *    programmer-readable error string otherwise
  */
-dbus_bool_t
+const char *
 _dbus_validate_member (const DBusString  *str,
                        int                start,
                        int                len)
@@ -1007,13 +1031,13 @@ _dbus_validate_member (const DBusString  *str,
   _dbus_assert (start <= _dbus_string_get_length (str));
 
   if (len > _dbus_string_get_length (str) - start)
-    return FALSE;
+    return complaint_string_range;
 
   if (len > DBUS_MAXIMUM_NAME_LENGTH)
-    return FALSE;
+    return complaint_name_length;
 
   if (len == 0)
-    return FALSE;
+    return complaint_empty;
 
   member = _dbus_string_get_const_data (str) + start;
   end = member + len;
@@ -1024,7 +1048,7 @@ _dbus_validate_member (const DBusString  *str,
    */
 
   if (_DBUS_UNLIKELY (!VALID_INITIAL_NAME_CHARACTER (*s)))
-    return FALSE;
+    return complaint_initial_name_char;
   else
     ++s;
 
@@ -1032,13 +1056,13 @@ _dbus_validate_member (const DBusString  *str,
     {
       if (_DBUS_UNLIKELY (!VALID_NAME_CHARACTER (*s)))
         {
-          return FALSE;
+          return complaint_name_char;
         }
 
       ++s;
     }
 
-  return TRUE;
+  return NULL;
 }
 
 /**
@@ -1052,9 +1076,10 @@ _dbus_validate_member (const DBusString  *str,
  * @param str the string
  * @param start first byte index to check
  * @param len number of bytes to check
- * @returns #TRUE if the byte range exists and is a valid name
+ * @returns #NULL if the byte range exists and is a valid name, a
+ *    programmer-readable error string otherwise
  */
-dbus_bool_t
+const char *
 _dbus_validate_error_name (const DBusString  *str,
                            int                start,
                            int                len)
@@ -1082,7 +1107,7 @@ _dbus_validate_error_name (const DBusString  *str,
     ((c) >= 'a' && (c) <= 'z') ||               \
     ((c) == '_') || ((c) == '-'))
 
-static dbus_bool_t
+static const char *
 _dbus_validate_bus_name_full (const DBusString  *str,
                               int                start,
                               int                len,
@@ -1098,13 +1123,13 @@ _dbus_validate_bus_name_full (const DBusString  *str,
   _dbus_assert (start <= _dbus_string_get_length (str));
 
   if (len > _dbus_string_get_length (str) - start)
-    return FALSE;
+    return complaint_string_range;
 
   if (len > DBUS_MAXIMUM_NAME_LENGTH)
-    return FALSE;
+    return complaint_name_length;
 
   if (len == 0)
-    return FALSE;
+    return complaint_empty;
 
   last_dot = NULL;
   iface = _dbus_string_get_const_data (str) + start;
@@ -1123,14 +1148,14 @@ _dbus_validate_bus_name_full (const DBusString  *str,
         if (*s == '.')
           {
             if (_DBUS_UNLIKELY ((s + 1) == end))
-              return FALSE;
+              return complaint_end_dot;
             if (_DBUS_UNLIKELY (!VALID_BUS_NAME_CHARACTER (*(s + 1))))
-              return FALSE;
+              return complaint_bus_name_char;
             ++s; /* we just validated the next char, so skip two */
           }
         else if (_DBUS_UNLIKELY (!VALID_BUS_NAME_CHARACTER (*s)))
           {
-            return FALSE;
+            return complaint_bus_name_char;
           }
 
         ++s;
@@ -1139,9 +1164,9 @@ _dbus_validate_bus_name_full (const DBusString  *str,
     return TRUE;
   }
   else if (_DBUS_UNLIKELY (*s == '.')) /* disallow starting with a . */
-    return FALSE;
+    return complaint_start_dot;
   else if (_DBUS_UNLIKELY (!VALID_INITIAL_BUS_NAME_CHARACTER (*s)))
-    return FALSE;
+    return complaint_initial_bus_name_char;
   else
     ++s;
 
@@ -1150,22 +1175,22 @@ _dbus_validate_bus_name_full (const DBusString  *str,
       if (*s == '.')
         {
           if (_DBUS_UNLIKELY ((s + 1) == end))
-            return FALSE;
+            return complaint_end_dot;
           else if (_DBUS_UNLIKELY (!VALID_INITIAL_BUS_NAME_CHARACTER (*(s + 1))))
-            return FALSE;
+            return complaint_initial_bus_name_char;
           last_dot = s;
           ++s; /* we just validated the next char, so skip two */
         }
       else if (_DBUS_UNLIKELY (!VALID_BUS_NAME_CHARACTER (*s)))
         {
-          return FALSE;
+          return complaint_bus_name_char;
         }
 
       ++s;
     }
 
   if (!is_namespace && _DBUS_UNLIKELY (last_dot == NULL))
-    return FALSE;
+    return complaint_end_dot;
 
   return TRUE;
 }
@@ -1181,9 +1206,10 @@ _dbus_validate_bus_name_full (const DBusString  *str,
  * @param str the string
  * @param start first byte index to check
  * @param len number of bytes to check
- * @returns #TRUE if the byte range exists and is a valid name
+ * @returns #NULL if the byte range exists and is a valid name, a
+ *    programmer-readable error string otherwise
  */
-dbus_bool_t
+const char *
 _dbus_validate_bus_name (const DBusString  *str,
                          int                start,
                          int                len)
@@ -1202,9 +1228,10 @@ _dbus_validate_bus_name (const DBusString  *str,
  * @param str the string
  * @param start first byte index to check
  * @param len number of bytes to check
- * @returns #TRUE if the byte range exists and is a valid name
+ * @returns #NULL if the byte range exists and is a valid name, a
+ *    programmer-readable error string otherwise
  */
-dbus_bool_t
+const char *
 _dbus_validate_bus_namespace (const DBusString  *str,
                               int                start,
                               int                len)
@@ -1222,21 +1249,29 @@ _dbus_validate_bus_namespace (const DBusString  *str,
  * @param str the string
  * @param start first byte index to check
  * @param len number of bytes to check
- * @returns #TRUE if the byte range exists and is a valid signature
+ * @returns #NULL if the byte range exists and is a valid name, a
+ *    programmer-readable error string otherwise
  */
-dbus_bool_t
+const char *
 _dbus_validate_signature (const DBusString  *str,
                           int                start,
                           int                len)
 {
+  DBusValidity reason;
+
   _dbus_assert (start >= 0);
   _dbus_assert (start <= _dbus_string_get_length (str));
   _dbus_assert (len >= 0);
 
   if (len > _dbus_string_get_length (str) - start)
-    return FALSE;
+    return complaint_string_range;
 
-  return _dbus_validate_signature_with_reason (str, start, len) == DBUS_VALID;
+  reason = _dbus_validate_signature_with_reason (str, start, len);
+
+  if (reason == DBUS_VALID)
+    return NULL;
+  else
+    return _dbus_validity_to_error_message (reason);
 }
 
 /** define _dbus_check_is_valid_path() */
diff --git a/dbus/dbus-marshal-validate.h b/dbus/dbus-marshal-validate.h
index 06434201..17e86f03 100644
--- a/dbus/dbus-marshal-validate.h
+++ b/dbus/dbus-marshal-validate.h
@@ -129,29 +129,30 @@ DBusValidity _dbus_validate_body_with_reason      (const DBusString *expected_si
 
 const char *_dbus_validity_to_error_message (DBusValidity validity);
 
-dbus_bool_t _dbus_validate_path       (const DBusString *str,
+const char *_dbus_validate_path       (const DBusString *str,
                                        int               start,
                                        int               len);
-dbus_bool_t _dbus_validate_interface  (const DBusString *str,
+const char *_dbus_validate_interface  (const DBusString *str,
                                        int               start,
                                        int               len);
-dbus_bool_t _dbus_validate_member     (const DBusString *str,
+const char *_dbus_validate_member     (const DBusString *str,
                                        int               start,
                                        int               len);
-dbus_bool_t _dbus_validate_error_name (const DBusString *str,
+const char *_dbus_validate_error_name (const DBusString *str,
                                        int               start,
                                        int               len);
-dbus_bool_t _dbus_validate_bus_name   (const DBusString *str,
+const char *_dbus_validate_bus_name   (const DBusString *str,
                                        int               start,
                                        int               len);
-dbus_bool_t _dbus_validate_bus_namespace (const DBusString  *str,
+const char *_dbus_validate_bus_namespace (const DBusString  *str,
                                           int                start,
                                           int                len);
-dbus_bool_t _dbus_validate_signature  (const DBusString *str,
+const char *_dbus_validate_signature  (const DBusString *str,
                                        int               start,
                                        int               len);
-/* just to have a name consistent with the above: */
-#define _dbus_validate_utf8(s,b,e) _dbus_string_validate_utf8 (s, b, e)
+/* just to have a name and signature consistent with the above: */
+#define _dbus_validate_utf8(s,b,e) \
+  (_dbus_string_validate_utf8 (s, b, e) ? NULL : "invalid UTF-8")
 
 #ifdef DBUS_DISABLE_CHECKS
 
@@ -181,8 +182,8 @@ _dbus_check_is_valid_##what (const char *name)                          \
     return FALSE;                                                       \
                                                                         \
   _dbus_string_init_const (&str, name);                                 \
-  return _dbus_validate_##what (&str, 0,                                \
-                                _dbus_string_get_length (&str));        \
+  return (_dbus_validate_##what (&str, 0,                               \
+                                 _dbus_string_get_length (&str)) == NULL); \
 }
 #endif /* !DBUS_DISABLE_CHECKS */