lib: Use a common function to validate strings.

python/Makefile.am

@@ -95,6 +95,7 @@ setup-install: setup.py stamp-extra-files
 # Python's crappy MANIFEST file cannot graft single files, so we have
 # to hard-link any extra files we need into the local directory.
 stamp-extra-files: \
+	  c-ctype.h \
 	  config.h \
 	  guestfs-internal-all.h \
 	  guestfs-internal-frontend-cleanups.h \
@@ -106,6 +107,9 @@ stamp-extra-files: \
 config.h:
 	ln ../config.h $@
 
+c-ctype.h:
+	ln $(top_srcdir)/gnulib/lib/c-ctype.h $@
+
 ignore-value.h:
 	ln $(top_srcdir)/gnulib/lib/ignore-value.h $@
 
@@ -138,6 +142,7 @@ CLEANFILES += \
 	*.pyc \
 	examples/*~ examples/*.pyc \
 	t/*~ t/*.pyc \
+	c-ctype.h \
 	config.h \
 	guestfs-internal-all.h \
 	guestfs-internal-frontend-cleanups.h \

src/appliance-kcmdline.c

@@ -36,23 +36,9 @@
  * Check that the $TERM environment variable is reasonable before
  * we pass it through to the appliance.
  */
-static bool
-valid_term (const char *term)
-{
-  size_t len = strlen (term);
-
-  if (len == 0 || len > 16)
-    return false;
-
-  while (len > 0) {
-    char c = *term++;
-    len--;
-    if (!c_isalnum (c) && c != '-' && c != '_')
-      return false;
-  }
-
-  return true;
-}
+#define VALID_TERM(term) \
+  guestfs_int_string_is_valid ((term), 1, 16, \
+                               VALID_FLAG_ALPHA|VALID_FLAG_DIGIT, "-_")
 
 #if defined(__powerpc64__)
 #define SERIAL_CONSOLE "console=hvc0 console=ttyS0"
@@ -196,7 +182,7 @@ guestfs_int_appliance_command_line (guestfs_h *g, const char *appliance_dev,
     guestfs_int_add_string (g, &argv, "guestfs_network=1");
 
   /* TERM environment variable. */
-  if (term && valid_term (term))
+  if (term && VALID_TERM (term))
     guestfs_int_add_sprintf (g, &argv, "TERM=%s", term);
   else
     guestfs_int_add_string (g, &argv, "TERM=linux");

src/drives.c

@@ -588,65 +588,24 @@ guestfs_int_free_drives (guestfs_h *g)
  * Check string parameter matches regular expression
  * C<^[-_[:alnum:]]+$> (in C locale).
  */
-static int
-valid_format_iface (const char *str)
-{
-  size_t len = strlen (str);
-
-  if (len == 0)
-    return 0;
-
-  while (len > 0) {
-    char c = *str++;
-    len--;
-    if (c != '-' && c != '_' && !c_isalnum (c))
-      return 0;
-  }
-  return 1;
-}
+#define VALID_FORMAT_IFACE(str) \
+  guestfs_int_string_is_valid ((str), 1, 0, \
+                               VALID_FLAG_ALPHA|VALID_FLAG_DIGIT, "-_")
 
 /**
  * Check the disk label is reasonable.  It can't contain certain
  * characters, eg. C<'/'>, C<','>.  However be stricter here and
  * ensure it's just alphabetic and E<le> 20 characters in length.
  */
-static int
-valid_disk_label (const char *str)
-{
-  size_t len = strlen (str);
-
-  if (len == 0 || len > 20)
-    return 0;
-
-  while (len > 0) {
-    char c = *str++;
-    len--;
-    if (!c_isalpha (c))
-      return 0;
-  }
-  return 1;
-}
+#define VALID_DISK_LABEL(str) \
+  guestfs_int_string_is_valid ((str), 1, 20, VALID_FLAG_ALPHA, NULL)
 
 /**
  * Check the server hostname is reasonable.
  */
-static int
-valid_hostname (const char *str)
-{
-  size_t len = strlen (str);
-
-  if (len == 0 || len > 255)
-    return 0;
-
-  while (len > 0) {
-    char c = *str++;
-    len--;
-    if (!c_isalnum (c) &&
-        c != '-' && c != '.' && c != ':' && c != '[' && c != ']')
-      return 0;
-  }
-  return 1;
-}
+#define VALID_HOSTNAME(str) \
+  guestfs_int_string_is_valid ((str), 1, 255, \
+                               VALID_FLAG_ALPHA|VALID_FLAG_DIGIT, "-.:[]")
 
 /**
  * Check the port number is reasonable.
@@ -700,7 +659,7 @@ parse_one_server (guestfs_h *g, const char *server, struct drive_server *ret)
       return -1;
     }
     free (port_str);
-    if (!valid_hostname (hostname)) {
+    if (!VALID_HOSTNAME (hostname)) {
       error (g, _("invalid hostname '%s'"), hostname);
       free (hostname);
       return -1;
@@ -711,7 +670,7 @@ parse_one_server (guestfs_h *g, const char *server, struct drive_server *ret)
   }
 
   /* Doesn't match anything above, so assume it's a bare hostname. */
-  if (!valid_hostname (server)) {
+  if (!VALID_HOSTNAME (server)) {
     error (g, _("invalid hostname or server string '%s'"), server);
     return -1;
   }
@@ -814,19 +773,19 @@ guestfs_impl_add_drive_opts (guestfs_h *g, const char *filename,
     return -1;
   }
 
-  if (data.format && !valid_format_iface (data.format)) {
+  if (data.format && !VALID_FORMAT_IFACE (data.format)) {
     error (g, _("%s parameter is empty or contains disallowed characters"),
            "format");
     free_drive_servers (data.servers, data.nr_servers);
     return -1;
   }
-  if (data.iface && !valid_format_iface (data.iface)) {
+  if (data.iface && !VALID_FORMAT_IFACE (data.iface)) {
     error (g, _("%s parameter is empty or contains disallowed characters"),
            "iface");
     free_drive_servers (data.servers, data.nr_servers);
     return -1;
   }
-  if (data.disk_label && !valid_disk_label (data.disk_label)) {
+  if (data.disk_label && !VALID_DISK_LABEL (data.disk_label)) {
     error (g, _("label parameter is empty, too long, or contains disallowed characters"));
     free_drive_servers (data.servers, data.nr_servers);
     return -1;

src/guestfs-internal-frontend.h

@@ -32,6 +32,8 @@
 #ifndef GUESTFS_INTERNAL_FRONTEND_H_
 #define GUESTFS_INTERNAL_FRONTEND_H_
 
+#include <stdbool.h>
+
 #include "guestfs-internal-all.h"
 
 #define _(str) dgettext(PACKAGE, (str))
@@ -89,6 +91,9 @@ extern int guestfs_int_random_string (char *ret, size_t len);
 extern char *guestfs_int_drive_name (size_t index, char *ret);
 extern ssize_t guestfs_int_drive_index (const char *);
 extern int guestfs_int_is_true (const char *str);
+extern bool guestfs_int_string_is_valid (const char *str, size_t min_length, size_t max_length, int flags, const char *extra);
+#define VALID_FLAG_ALPHA 1
+#define VALID_FLAG_DIGIT 2
 //extern void guestfs_int_fadvise_normal (int fd);
 extern void guestfs_int_fadvise_sequential (int fd);
 extern void guestfs_int_fadvise_random (int fd);

src/unit-tests.c

@@ -433,6 +433,47 @@ test_stringsbuf (void)
   guestfs_close (g);
 }
 
+/* Use the same macros as in src/drives.c */
+#define VALID_FORMAT_IFACE(str) \
+  guestfs_int_string_is_valid ((str), 1, 0, \
+                               VALID_FLAG_ALPHA|VALID_FLAG_DIGIT, "-_")
+#define VALID_DISK_LABEL(str) \
+  guestfs_int_string_is_valid ((str), 1, 20, VALID_FLAG_ALPHA, NULL)
+#define VALID_HOSTNAME(str) \
+  guestfs_int_string_is_valid ((str), 1, 255, \
+                               VALID_FLAG_ALPHA|VALID_FLAG_DIGIT, "-.:[]")
+
+static void
+test_valid (void)
+{
+  assert (!VALID_FORMAT_IFACE (""));
+  assert (!VALID_DISK_LABEL (""));
+  assert (!VALID_HOSTNAME (""));
+
+  assert (!VALID_DISK_LABEL ("012345678901234567890"));
+
+  assert (VALID_FORMAT_IFACE ("abc"));
+  assert (VALID_FORMAT_IFACE ("ABC"));
+  assert (VALID_FORMAT_IFACE ("abc123"));
+  assert (VALID_FORMAT_IFACE ("abc123-"));
+  assert (VALID_FORMAT_IFACE ("abc123_"));
+  assert (!VALID_FORMAT_IFACE ("abc123."));
+
+  assert (VALID_DISK_LABEL ("abc"));
+  assert (VALID_DISK_LABEL ("ABC"));
+  assert (!VALID_DISK_LABEL ("abc123"));
+  assert (!VALID_DISK_LABEL ("abc123-"));
+
+  assert (VALID_HOSTNAME ("abc"));
+  assert (VALID_HOSTNAME ("ABC"));
+  assert (VALID_HOSTNAME ("abc123"));
+  assert (VALID_HOSTNAME ("abc-123"));
+  assert (VALID_HOSTNAME ("abc.123"));
+  assert (VALID_HOSTNAME ("abc:123"));
+  assert (VALID_HOSTNAME ("abc[123]"));
+  assert (!VALID_HOSTNAME ("abc/def"));
+}
+
 int
 main (int argc, char *argv[])
 {
@@ -448,6 +489,7 @@ main (int argc, char *argv[])
   test_timeval_diff ();
   test_match ();
   test_stringsbuf ();
+  test_valid ();
 
   exit (EXIT_SUCCESS);
 }

src/utils.c

@@ -27,6 +27,7 @@
 
 #include <stdio.h>
 #include <stdlib.h>
+#include <stdbool.h>
 #include <string.h>
 #include <unistd.h>
 #include <fcntl.h>
@@ -37,9 +38,11 @@
 
 /* NB: MUST NOT require linking to gnulib, because that will break the
  * Python 'sdist' which includes a copy of this file.  It's OK to
- * include "ignore-value.h" here (since it is a header only with no
- * other code), but we also had to copy this file to the Python sdist.
+ * include "c-ctype.h" and "ignore-value.h" here (since it is a header
+ * only with no other code), but we also had to copy these files to
+ * the Python sdist.
  */
+#include "c-ctype.h"
 #include "ignore-value.h"
 
 /* NB: MUST NOT include "guestfs-internal.h". */
@@ -360,6 +363,62 @@ guestfs_int_is_true (const char *str)
   return -1;
 }
 
+/**
+ * Check a string for validity, that it contains only certain
+ * characters, and minimum and maximum length.  This function is
+ * usually wrapped in a VALID_* macro, see F<src/drives.c> for an
+ * example.
+ *
+ * C<str> is the string to check.
+ *
+ * C<min_length> and C<max_length> are the minimum and maximum
+ * length checks.  C<0> means no check.
+ *
+ * The flags control:
+ *
+ * =over 4
+ *
+ * =item C<VALID_FLAG_ALPHA>
+ *
+ * 7-bit ASCII-only alphabetic characters are permitted.
+ *
+ * =item C<VALID_FLAG_DIGIT>
+ *
+ * 7-bit ASCII-only digits are permitted.
+ *
+ * =back
+ *
+ * C<extra> is a set of extra characters permitted, in addition
+ * to alphabetic and/or digits.  (C<extra = NULL> for no extra).
+ *
+ * Returns boolean C<true> if the string is valid (passes all the
+ * tests), or C<false> if not.
+ */
+bool
+guestfs_int_string_is_valid (const char *str,
+                             size_t min_length, size_t max_length,
+                             int flags, const char *extra)
+{
+  size_t i, len = strlen (str);
+
+  if ((min_length > 0 && len < min_length) ||
+      (max_length > 0 && len > max_length))
+    return false;
+
+  for (i = 0; i < len; ++i) {
+    bool valid_char;
+
+    valid_char =
+      ((flags & VALID_FLAG_ALPHA) && c_isalpha (str[i])) ||
+      ((flags & VALID_FLAG_DIGIT) && c_isdigit (str[i])) ||
+      (extra && strchr (extra, str[i]));
+
+    if (!valid_char) return false;
+  }
+
+  return true;
+}
+
 #if 0 /* not used yet */
 /**
  * Hint that we will read or write the file descriptor normally.