Be stricter for the MIME type check. It's actually a bit too strict right

2008-04-28  Vincent Untz  <vuntz@gnome.org>

	Be stricter for the MIME type check. It's actually a bit too strict
	right now, see the TODO at the beginning of mimeutils.c to know how to
	improve things a bit.

	* src/Makefile.am:
	* src/mimeutils.[ch]: add new files
	* src/update-desktop-database.c: (process_desktop_file): use the
	improved mu_mime_type_is_valid() function instead of
	is_valid_mime_type()
	* src/validate.c: (handle_mime_key): use the improved
	mu_mime_type_is_valid() function instead of a trivial check

6 files changed, 301 insertions, 96 deletions

diff --git a/ChangeLog b/ChangeLog
index e36bc57..b86d50d 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,17 @@
+2008-04-28  Vincent Untz  <vuntz@gnome.org>
+
+	Be stricter for the MIME type check. It's actually a bit too strict
+	right now, see the TODO at the beginning of mimeutils.c to know how to
+	improve things a bit.
+
+	* src/Makefile.am:
+	* src/mimeutils.[ch]: add new files
+	* src/update-desktop-database.c: (process_desktop_file): use the
+	improved mu_mime_type_is_valid() function instead of
+	is_valid_mime_type()
+	* src/validate.c: (handle_mime_key): use the improved
+	mu_mime_type_is_valid() function instead of a trivial check
+
 2008-04-26  Vincent Untz  <vuntz@gnome.org>
 
 	* src/validate.c: (handle_comment_key): check that the Comment does not
diff --git a/src/Makefile.am b/src/Makefile.am
index 1e242da..260cdd5 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -12,6 +12,8 @@ bin_PROGRAMS =					\
 desktop_file_validate_SOURCES=			\
 	keyfileutils.c				\
 	keyfileutils.h				\
+	mimeutils.c				\
+	mimeutils.h				\
 	validate.c				\
 	validate.h				\
 	validator.c
@@ -19,11 +21,15 @@ desktop_file_validate_SOURCES=			\
 desktop_file_install_SOURCES=			\
 	keyfileutils.c				\
 	keyfileutils.h				\
+	mimeutils.c				\
+	mimeutils.h				\
 	validate.c				\
 	validate.h				\
 	install.c
 
 update_desktop_database_SOURCES =		\
+	mimeutils.c				\
+	mimeutils.h				\
 	update-desktop-database.c
 
 desktop_file_validate_LDADD = $(DESKTOP_FILE_UTILS_LIBS)
diff --git a/src/mimeutils.c b/src/mimeutils.c
new file mode 100644
index 0000000..7fbc336
--- /dev/null
+++ b/src/mimeutils.c
@@ -0,0 +1,198 @@
+/* mimeutils.c: useful functions related to mime types
+ *
+ * Copyright 2004  Red Hat, Inc. 
+ * Copyright (C) 2008 Novell, Inc.
+ * 
+ * Written by Vincent Untz <vuntz@gnome.org>, based on code from
+ * update-desktop-base.c which was originally written by Ray Strode
+ * <rstrode@redhat.com>
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * as published by the Free Software Foundation; either version 2
+ * of the License, or (at your option) any later version.
+ * 
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ * 
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301,
+ * USA.
+ */
+
+/* Related documentation:
+ *   + Registered media types: http://www.iana.org/assignments/media-types/
+ *   + RFC about MIME, with the BNF grammar: http://tools.ietf.org/html/rfc2045
+ *   + RFC about media types: http://tools.ietf.org/html/rfc2046
+ *   + RFC about the model media type: http://tools.ietf.org/html/rfc2077
+ */
+
+#include <string.h>
+
+#include <glib.h>
+
+#include "mimeutils.h"
+
+/* Defined in RFC 2045/2046 and RFC 2077 */
+static const char *registered_discrete_media_types[] = {
+  "application", "audio", "image", "model", "text", "video"
+};
+
+/* Defined in RFC 2045/2046 */
+static const char *registered_composite_media_types[] = {
+  "message", "multipart"
+};
+
+static const char *registered_not_used_media_types[] = {
+  "example"
+};
+
+/* TODO: we will break in a few cases, since we're only IANA-aware so far.
+ * Known cases:
+ *  inode/*: added by the fdo spec
+ *  flv-application/octet-stream: alias in the fdo db
+ *  zz-application/zz-winassoc-cdr: alias in the fdo db
+ *  misc/ultravox: widely used?
+ *
+ * TODO: might actually be nice to download at distcheck time all the
+ * registered subtypes and warn when using a non-registered non-experimental
+ * subtype.
+ */
+
+/* From the BNF grammar:
+ *
+ *  token := 1*<any (US-ASCII) CHAR except SPACE, CTLs,
+ *              or tspecials>
+ *
+ *  tspecials :=  "(" / ")" / "<" / ">" / "@" /
+ *                "," / ";" / ":" / "\" / <">
+ *                "/" / "[" / "]" / "?" / "="
+ */
+static gboolean
+is_valid_mime_type_char (const guchar c)
+{
+  char invalid_chars[] = "()<>@,;:\\\"/[]?=";
+  
+  /* Filter out control chars and space */
+  if ((c <= 32) || (c == 127))
+    return FALSE;
+  
+  if (memchr (invalid_chars, c, sizeof (invalid_chars)) != NULL)
+    return FALSE;
+  
+  return TRUE;
+}
+
+/* From the BNF grammar:
+ *
+ *    x-token := <The two characters "X-" or "x-" followed, with
+ *                no  intervening white space, by any token>
+ *
+ * From RFC 2046:
+ *   In general, the use of "X-" top-level types is strongly discouraged.
+ *   Implementors should invent subtypes of the existing types whenever
+ *   possible. In many cases, a subtype of "application" will be more
+ *   appropriate than a new top-level type.
+ */
+static MimeUtilsValidity
+is_valid_media_type (const char  *media_type,
+                     char       **error)
+{
+  unsigned int i;
+
+  if (g_ascii_strncasecmp (media_type, "X-", 2) == 0) {
+    for (i = 2; media_type[i]; i++) {
+      if (!is_valid_mime_type_char (media_type[i]))
+        if (error)
+          *error = g_strdup_printf ("\"%s\" a media type that contains "
+                                    "an invalid character", media_type);
+        return FALSE;
+    }
+
+    if (error)
+      *error = g_strdup_printf ("the use of \"%s\" as media type is strongly "
+                                "discouraged in favor of a subtype of the "
+                                "\"application\" media type", media_type);
+
+    return MU_DISCOURAGED;
+  }
+
+#define IF_IS_MEDIA_TYPE_IN(list)             \
+  for (i = 0; i < G_N_ELEMENTS (list); i++) { \
+    if (strcmp (media_type, list[i]) == 0)       \
+      break;                                  \
+  }                                           \
+  if (i < G_N_ELEMENTS (list))
+
+  IF_IS_MEDIA_TYPE_IN (registered_discrete_media_types)
+    return MU_VALID;
+
+  IF_IS_MEDIA_TYPE_IN (registered_composite_media_types) {
+    if (error)
+      *error = g_strdup_printf ("\"%s\" is a media type that probably does "
+                                "not make sense in this context", media_type);
+    return MU_DISCOURAGED;
+  }
+
+  IF_IS_MEDIA_TYPE_IN (registered_not_used_media_types) {
+    if (error)
+      *error = g_strdup_printf ("\"%s\" is a media type that must not "
+                                "be used", media_type);
+    return MU_INVALID;
+  }
+
+  if (error)
+    *error = g_strdup_printf ("\"%s\" is an unregistered media type",
+                              media_type);
+
+  return MU_INVALID;
+}
+
+MimeUtilsValidity
+mu_mime_type_is_valid (const char  *mime_type,
+                       char       **error)
+{
+  char *media_type;
+  char *subtype;
+  MimeUtilsValidity media_type_validity;
+  
+  media_type = g_strdup (mime_type);
+  subtype = strchr (media_type, '/');
+
+  if (!subtype) {
+    if (error)
+      *error = g_strdup_printf ("\"%s\" does not contain a subtype",
+                                mime_type);
+    g_free (media_type);
+    return MU_INVALID;
+  }
+
+  subtype[0] = '\0';
+  subtype++;
+
+  if (subtype[0] == '\0') {
+    if (error)
+      *error = g_strdup_printf ("\"%s\" contains an empty subtype",
+                                mime_type);
+    g_free (media_type);
+    return MU_INVALID;
+  }
+
+  for (; subtype[0] != '\0'; subtype++) {
+      if (!is_valid_mime_type_char (subtype[0])) {
+      if (error)
+        *error = g_strdup_printf ("\"%s\" contains an invalid character in "
+                                  "the subtype", mime_type);
+        g_free (media_type);
+        return MU_INVALID;
+      }
+  }
+
+  media_type_validity = is_valid_media_type (media_type, error);
+  g_free (media_type);
+
+  return media_type_validity;
+}
diff --git a/src/mimeutils.h b/src/mimeutils.h
new file mode 100644
index 0000000..cb3d31a
--- /dev/null
+++ b/src/mimeutils.h
@@ -0,0 +1,32 @@
+/* mimeutils.h: useful functions related to mime types
+ * 
+ * Copyright (C) 2008 Novell, Inc.
+ * 
+ * Written by Vincent Untz <vuntz@gnome.org>.
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * as published by the Free Software Foundation; either version 2
+ * of the License, or (at your option) any later version.
+ * 
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ * 
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301,
+ * USA.
+ */
+
+#include <glib.h>
+
+typedef enum {
+  MU_VALID,
+  MU_DISCOURAGED,
+  MU_INVALID
+} MimeUtilsValidity;
+
+MimeUtilsValidity mu_mime_type_is_valid (const char  *mime_type,
+                                         char       **error);
diff --git a/src/update-desktop-database.c b/src/update-desktop-database.c
index 4086437..a2e49ab 100644
--- a/src/update-desktop-database.c
+++ b/src/update-desktop-database.c
@@ -34,6 +34,7 @@
 #include <glib/gi18n.h>
 
 #include "keyfileutils.h"
+#include "mimeutils.h"
 
 #define NAME "update-desktop-database"
 #define CACHE_FILENAME "mimeinfo.cache"
@@ -50,8 +51,6 @@ static void sync_database (const char *dir, GError **error);
 static void cache_desktop_file (const char  *desktop_file, 
                                 const char  *mime_type,
                                 GError     **error);
-static gboolean is_valid_mime_type (const char *desktop_file, 
-                                    const char *mime_type);
 static void process_desktop_file (const char  *desktop_file, 
                                   const char  *name,
                                   GError     **error);
@@ -86,88 +85,6 @@ cache_desktop_file (const char  *desktop_file,
 }
 
 
-static gboolean
-is_valid_mime_type_char (const guchar c)
-{
-  char invalid_chars [] = "()<>@,;:\\/[]?=\"";
-  
-  if ((c <= 32) || (c == 127)) 
-    {
-      /* Filter out control chars and space */
-      return FALSE;
-    }
-  
-  if (memchr (invalid_chars, c, sizeof (invalid_chars)) != NULL) 
-    {
-      return FALSE;
-    }
-  
-  return TRUE;
-}
-
-
-static gboolean
-is_valid_mime_type (const char *desktop_file, 
-                    const char *mime_type)
-{
-  gulong subtype_offset;
-  gulong valid_chars;
-  
-  valid_chars = 0;
-  subtype_offset = 0;
-  
-  while (mime_type[valid_chars] != '\0') 
-    {
-      if (mime_type[valid_chars] == '/') 
-	{
-	  if (valid_chars == 0) 
-	    {
-	      /* We encountered a / before any valid char */
-              udd_print ("File '%s' contains invalid MIME type '%s' "
-                         "that starts with a slash\n",
-                         desktop_file, mime_type);
-	      return FALSE;
-	    }
-	  if (subtype_offset != 0) 
-	    {
-	      /* We already encountered a '/' previously */
-              udd_print ("File '%s' contains invalid MIME type '%s' "
-                         "that has more than one slash\n", 
-                         desktop_file, mime_type);
-              return FALSE;
-            }
-          subtype_offset = valid_chars;
-        } 
-      else if (!is_valid_mime_type_char (mime_type[valid_chars])) 
-        {
-          udd_print ("File '%s' contains invalid MIME type '%s' "
-                     "that contains invalid characters\n", 
-                     desktop_file, mime_type);
-          return FALSE;
-        }
-
-      valid_chars++;			    
-    }
-
-  if (subtype_offset == 0) 
-    {
-      /* The mime type didn't contain any / */
-      udd_print ("File '%s' contains invalid MIME type '%s' that is "
-                 "missing a slash\n", desktop_file, mime_type);
-      return FALSE;
-    }
-
-  if ((subtype_offset != 0) && (subtype_offset == valid_chars)) 
-    {
-      /* Missing subtype name */
-      udd_print ("File '%s' contains invalid MIME type '%s' that is "
-                 "missing a subtype\n", desktop_file, mime_type);
-      return FALSE;
-    }
-  
-  return TRUE;
-}
-
 static void
 process_desktop_file (const char  *desktop_file, 
                       const char  *name,
@@ -205,10 +122,30 @@ process_desktop_file (const char  *desktop_file,
   for (i = 0; mime_types[i] != NULL; i++)
     {
       char *mime_type;
+      MimeUtilsValidity valid;
+      char *valid_error;
 
       mime_type = g_strchomp (mime_types[i]);
-      if (!is_valid_mime_type (desktop_file, mime_types[i])) 
-	continue;
+      valid = mu_mime_type_is_valid (mime_types[i], &valid_error);
+      switch (valid)
+      {
+        case MU_VALID:
+          break;
+        case MU_DISCOURAGED:
+          udd_print ("Warning in file \"%s\": usage of MIME type \"%s\" is "
+                     "discouraged (%s)\n",
+                     desktop_file, mime_types[i], valid_error);
+          g_free (valid_error);
+          break;
+        case MU_INVALID:
+          udd_print ("Error in file \"%s\": \"%s\" is an invalid MIME type "
+                     "(%s)\n", desktop_file, mime_types[i], valid_error);
+          g_free (valid_error);
+          /* not a break: we continue to the next mime type */
+          continue;
+        default:
+          g_assert_not_reached ();
+      }
 
       cache_desktop_file (name, mime_type, &load_error);
 
diff --git a/src/validate.c b/src/validate.c
index d1fd959..43e77ef 100644
--- a/src/validate.c
+++ b/src/validate.c
@@ -40,6 +40,7 @@
 #include <glib/gstdio.h>
 
 #include "keyfileutils.h"
+#include "mimeutils.h"
 #include "validate.h"
 
 /*FIXME: document where we are stricter than the spec
@@ -1225,8 +1226,6 @@ handle_path_key (kf_validator *kf,
 /* + The MIME type(s) supported by this application. Check they are valid
  *   MIME types.
  *   Checked.
- *   FIXME: need to verify what is the exact definition of a MIME type.
- *   Look at is_valid_mime_type()
  */
 static gboolean
 handle_mime_key (kf_validator *kf,
@@ -1235,9 +1234,10 @@ handle_mime_key (kf_validator *kf,
 {
   gboolean       retval;
   char         **types;
-  char          *slash;
   GHashTable    *hashtable;
   int            i;
+  char          *valid_error;
+  MimeUtilsValidity valid;
 
   handle_key_for_application (kf, locale_key, value);
 
@@ -1261,13 +1261,31 @@ handle_mime_key (kf_validator *kf,
 
     g_hash_table_insert (hashtable, types[i], types[i]);
 
-    slash = strchr (types[i], '/');
-    if (!slash || strchr (slash + 1, '/')) {
-      print_fatal (kf, "value \"%s\" for key \"%s\" in group \"%s\" "
-                       "contains value \"%s\" which does not look like "
-                       "a MIME type\n",
-                       value, locale_key, kf->current_group, types[i]);
-      retval = FALSE;
+    valid = mu_mime_type_is_valid (types[i], &valid_error);
+    switch (valid) {
+      case MU_VALID:
+        break;
+      case MU_DISCOURAGED:
+        print_warning (kf, "value \"%s\" for key \"%s\" in group \"%s\" "
+                           "contains value \"%s\" which is a MIME type that "
+                           "should probably not be used: %s\n",
+                           value, locale_key, kf->current_group,
+                           types[i], valid_error);
+
+        g_free (valid_error);
+        break;
+      case MU_INVALID:
+        print_fatal (kf, "value \"%s\" for key \"%s\" in group \"%s\" "
+                         "contains value \"%s\" which is an invalid MIME "
+                         "type: %s\n",
+                         value, locale_key, kf->current_group,
+                         types[i], valid_error);
+
+        retval = FALSE;
+        g_free (valid_error);
+        break;
+      default:
+        g_assert_not_reached ();
     }
   }