Libguestfs - Sep 2018 - Re: [PATCH nbdkit v3 2/6] main: Tighten up characters permitted in config keys.

v1: https://www.redhat.com/archives/libguestfs/2018-September/msg00024.html

v2: https://www.redhat.com/archives/libguestfs/2018-September/msg00034.html

v3:

 - Fixed is_config_key zero length test.

 - Fixed is_config_key so it uses strspn and is not O(n^2).

 - Changed >= 1.7 to >= 1.8 in the documentation.

Rich.

This extends the concept of the magic script parameter.  In existing
nbdkit plugins if the first parameter is "bare" (does not contain
'='), then it is parsed magically to allow scripting languages to
work:

  nbdkit perl foo.pl

is parsed as:

  nbdkit perl script=foo.pl

This generalises the concept, allowing a plugin.magic_config_key to be
defined.  If this is non-NULL then any bare parameters on the command
line:

  nbdkit file foo

where file_plugin.magic_config_key = "file" is parsed as:

  nbdkit file file=foo

If magic_config_key == NULL then the previous behaviour is used.

There is a small (but hopefully not noticable) change to the behaviour
of ‘--dump-plugin’.  It is now handled after all command line
parameters have been parsed but before the .config_complete method is
called (whereas previously it would be called after the first command
line parameter was parsed).

Note this change only applies to plugin parameters.
---
 docs/nbdkit-plugin.pod  | 17 +++++++++++
 docs/nbdkit.pod         |  8 +++---
 include/nbdkit-plugin.h |  2 ++
 src/filters.c           | 12 ++++++++
 src/internal.h          |  1 +
 src/main.c              | 62 ++++++++++++++++++++++++-----------------
 src/plugins.c           |  9 ++++++
 7 files changed, 81 insertions(+), 30 deletions(-)

diff --git a/docs/nbdkit-plugin.pod b/docs/nbdkit-plugin.pod
index 515c032..570a142 100644
--- a/docs/nbdkit-plugin.pod
+++ b/docs/nbdkit-plugin.pod
@@ -396,6 +396,23 @@ with an error.
 If there is an error, C<.config> should call C<nbdkit_error> with
an
 error message and return C<-1>.
 
+=head2 C<.magic_config_key>
+
+ const char *magic_config_key;
+
+This optional string can be used to set a "magic" key used when
+parsing plugin parameters.  It affects how "bare parameters" (those
+which do not contain an C<=> character) are parsed on the command
+line.
+
+If C<magic_config_key != NULL> then any bare parameters are passed to
+the C<.config> method as: S<C<config (magic_config_key,
argv[i]);>>.
+
+If C<magic_config_key> is not set then we behave as in nbdkit E<lt>
+1.7: If the first parameter on the command line is bare then it is
+passed to the C<.config> method as: S<C<config ("script",
value);>>.
+Any other bare parameters give errors.
+
 =head2 C<.config_complete>
 
  int config_complete (void);
diff --git a/docs/nbdkit.pod b/docs/nbdkit.pod
index 981e571..cf1c5ca 100644
--- a/docs/nbdkit.pod
+++ b/docs/nbdkit.pod
@@ -404,11 +404,11 @@ To dump information about a plugin, do:
 
 =head2 Magic script parameter
 
-As a special case, if the first plugin argument does not contain an
-C<'='> character then it is assumed to be C<script=value>.
+For some plugins, especially those supporting scripting languages like
+Perl, if the first plugin argument does not contain an C<'='>
+character then it is assumed to be C<script=value>.
 
-That allows scripting language plugins like L<nbdkit-perl-plugin(1)>
-to do:
+That allows scripting language plugins to do:
 
  nbdkit perl foo.pl [args...]
 
diff --git a/include/nbdkit-plugin.h b/include/nbdkit-plugin.h
index c95e26c..31aa6c7 100644
--- a/include/nbdkit-plugin.h
+++ b/include/nbdkit-plugin.h
@@ -121,6 +121,8 @@ struct nbdkit_plugin {
   int (*trim) (void *handle, uint32_t count, uint64_t offset, uint32_t flags);
   int (*zero) (void *handle, uint32_t count, uint64_t offset, uint32_t flags);
 #endif
+
+  const char *magic_config_key;
 };
 
 extern void nbdkit_set_error (int err);
diff --git a/src/filters.c b/src/filters.c
index e6826f2..3626742 100644
--- a/src/filters.c
+++ b/src/filters.c
@@ -205,6 +205,17 @@ filter_config_complete (struct backend *b)
     f->backend.next->config_complete (f->backend.next);
 }
 
+/* magic_config_key only applies to plugins, so this passes the
+ * request through to the plugin (hence the name).
+ */
+static const char *
+plugin_magic_config_key (struct backend *b)
+{
+  struct backend_filter *f = container_of (b, struct backend_filter, backend);
+
+  return f->backend.next->magic_config_key (f->backend.next);
+}
+
 static int
 next_open (void *nxdata, int readonly)
 {
@@ -619,6 +630,7 @@ static struct backend filter_functions = {
   .dump_fields = filter_dump_fields,
   .config = filter_config,
   .config_complete = filter_config_complete,
+  .magic_config_key = plugin_magic_config_key,
   .open = filter_open,
   .prepare = filter_prepare,
   .finalize = filter_finalize,
diff --git a/src/internal.h b/src/internal.h
index 1c5aa72..d4fc534 100644
--- a/src/internal.h
+++ b/src/internal.h
@@ -171,6 +171,7 @@ struct backend {
   void (*dump_fields) (struct backend *);
   void (*config) (struct backend *, const char *key, const char *value);
   void (*config_complete) (struct backend *);
+  const char *(*magic_config_key) (struct backend *);
   int (*open) (struct backend *, struct connection *conn, int readonly);
   int (*prepare) (struct backend *, struct connection *conn);
   int (*finalize) (struct backend *, struct connection *conn);
diff --git a/src/main.c b/src/main.c
index 8a83a32..9c18d6f 100644
--- a/src/main.c
+++ b/src/main.c
@@ -231,6 +231,7 @@ main (int argc, char *argv[])
     const char *filename;
   } *filter_filenames = NULL;
   size_t i;
+  const char *magic_config_key;
 
   threadlocal_init ();
 
@@ -680,37 +681,46 @@ main (int argc, char *argv[])
     exit (EXIT_SUCCESS);
   }
 
-  /* Find key=value configuration parameters for this plugin.
-   * The first one is magical in that if it doesn't contain '='
then
-   * we assume it is 'script=...'.
+  /* Call config and config_complete to parse the parameters.
+   *
+   * If the plugin provides magic_config_key then any "bare" values
+   * (ones not containing "=") are prefixed with this key.
+   *
+   * For backwards compatibility with old plugins, and to support
+   * scripting languages, if magic_config_key == NULL then if the
+   * first parameter is bare it is prefixed with the key "script",
and
+   * any other bare parameters are errors.
    */
-  if (optind < argc && (p = strchr (argv[optind], '=')) ==
NULL) {
-    backend->config (backend, "script", argv[optind]);
-    ++optind;
-  }
-
-  /* This must run after parsing the possible script parameter so that
-   * the script can be loaded for scripting languages.  Note that all
-   * scripting languages load the script as soon as they see the
-   * script=... parameter (and do not wait for config_complete).
-   */
-  if (dump_plugin) {
-    backend->dump_fields (backend);
-    exit (EXIT_SUCCESS);
-  }
-
-  while (optind < argc) {
-    if ((p = strchr (argv[optind], '=')) != NULL) {
+  magic_config_key = backend->magic_config_key (backend);
+  for (i = 0; optind < argc; ++i, ++optind) {
+    p = strchr (argv[optind], '=');
+    if (p) {                    /* key=value */
       *p = '\0';
       backend->config (backend, argv[optind], p+1);
-      ++optind;
     }
-    else {
-      fprintf (stderr,
-               "%s: expecting key=value on the command line but got:
%s\n",
-               program_name, argv[optind]);
-      exit (EXIT_FAILURE);
+    else if (magic_config_key == NULL) {
+      if (i == 0)               /* magic script parameter */
+        backend->config (backend, "script", argv[optind]);
+      else {
+        fprintf (stderr,
+                 "%s: expecting key=value on the command line but got:
%s\n",
+                 program_name, argv[optind]);
+        exit (EXIT_FAILURE);
+      }
     }
+    else {                      /* magic config key */
+      backend->config (backend, magic_config_key, argv[optind]);
+    }
+  }
+
+  /* This must run after parsing the parameters so that the script can
+   * be loaded for scripting languages.  But it must be called before
+   * config_complete so that the plugin doesn't check for missing
+   * parameters.
+   */
+  if (dump_plugin) {
+    backend->dump_fields (backend);
+    exit (EXIT_SUCCESS);
   }
 
   backend->config_complete (backend);
diff --git a/src/plugins.c b/src/plugins.c
index 780bbd9..2bea6ac 100644
--- a/src/plugins.c
+++ b/src/plugins.c
@@ -232,6 +232,14 @@ plugin_config_complete (struct backend *b)
     exit (EXIT_FAILURE);
 }
 
+static const char *
+plugin_magic_config_key (struct backend *b)
+{
+  struct backend_plugin *p = container_of (b, struct backend_plugin, backend);
+
+  return p->plugin.magic_config_key;
+}
+
 static int
 plugin_open (struct backend *b, struct connection *conn, int readonly)
 {
@@ -630,6 +638,7 @@ static struct backend plugin_functions = {
   .dump_fields = plugin_dump_fields,
   .config = plugin_config,
   .config_complete = plugin_config_complete,
+  .magic_config_key = plugin_magic_config_key,
   .open = plugin_open,
   .prepare = plugin_prepare,
   .finalize = plugin_finalize,
-- 
2.18.0

Previously key=value on the command line allowed the key to be pretty
much anything that didn't contain an '=' character.  Even empty
strings were permitted.

This tightens up the permitted keys so they must contain only ASCII
alphanumeric, period, underscore or dash characters; must not be an
empty string; and must start with an ASCII alphabetic character.
---
 docs/nbdkit-plugin.pod | 24 +++++++++++++++---------
 src/main.c             | 36 +++++++++++++++++++++++++++++++++++-
 2 files changed, 50 insertions(+), 10 deletions(-)

diff --git a/docs/nbdkit-plugin.pod b/docs/nbdkit-plugin.pod
index 570a142..fb5c7d7 100644
--- a/docs/nbdkit-plugin.pod
+++ b/docs/nbdkit-plugin.pod
@@ -376,15 +376,21 @@ optional list of C<key=value> arguments.  These are
passed to the
 plugin through this callback when the plugin is first loaded and
 before any connections are accepted.
 
-This callback may be called zero or more times.  Both C<key> and
-C<value> parameters will be non-NULL, but it is possible for either to
-be empty strings.  The strings are owned by nbdkit but will remain
-valid for the lifetime of the plugin, so the plugin does not need to
-copy them.
-
-The format of the C<key> accepted by plugins is up to the plugin, but
-you should probably look at other plugins and follow the same
-conventions.
+This callback may be called zero or more times.
+
+Both C<key> and C<value> parameters will be non-NULL.
+
+They key will be a non-empty string beginning with an ASCII alphabetic
+character (C<A-Z> C<a-z>).  The rest of the key must contain only
+ASCII alphanumeric plus period, underscore or dash characters (C<A-Z>
+C<a-z> C<0-9> C<.> C<_> C<->).
+
+The value may be an arbitrary string, including an empty string.  The
+strings are owned by nbdkit but will remain valid for the lifetime of
+the plugin, so the plugin does not need to copy them.
+
+The names of C<key>s accepted by plugins is up to the plugin, but you
+should probably look at other plugins and follow the same conventions.
 
 If the value is a relative path, then note that the server changes
 directory when it starts up.  See L</FILENAMES AND PATHS> above.
diff --git a/src/main.c b/src/main.c
index 9c18d6f..f4a800f 100644
--- a/src/main.c
+++ b/src/main.c
@@ -72,6 +72,7 @@ static void fork_into_background (void);
 static uid_t parseuser (const char *);
 static gid_t parsegroup (const char *);
 static unsigned int get_socket_activation (void);
+static int is_config_key (const char *key, size_t len);
 
 struct debug_flag *debug_flags; /* -D */
 int exit_with_parent;           /* --exit-with-parent */
@@ -694,7 +695,7 @@ main (int argc, char *argv[])
   magic_config_key = backend->magic_config_key (backend);
   for (i = 0; optind < argc; ++i, ++optind) {
     p = strchr (argv[optind], '=');
-    if (p) {                    /* key=value */
+    if (p && is_config_key (argv[optind], p - argv[optind])) { /*
key=value */
       *p = '\0';
       backend->config (backend, argv[optind], p+1);
     }
@@ -1281,3 +1282,36 @@ get_socket_activation (void)
 
   return nr_fds;
 }
+
+/* When parsing plugin and filter config key=value from the command
+ * line, check that the key is a simple alphanumeric with period,
+ * underscore or dash.
+ *
+ * Note this doesn't return an error.  If the key is not valid then we
+ * return false and the parsing code will assume that this is a bare
+ * value instead.
+ */
+static int
+is_config_key (const char *key, size_t len)
+{
+  static const char allowed_first[] +    "abcdefghijklmnopqrstuvwxyz"
+    "ABCDEFGHIJKLMNOPQRSTUVWXYZ";
+  static const char allowed[] +    "abcdefghijklmnopqrstuvwxyz"
+    "ABCDEFGHIJKLMNOPQRSTUVWXYZ"
+    "0123456789"
+    "._-";
+
+  if (len == 0)
+    return 0;
+
+  if (strchr (allowed_first, key[0]) == NULL)
+    return 0;
+
+  /* This works in context of the caller since key[len] == '='. */
+  if (strspn (key, allowed) != len)
+    return 0;
+
+  return 1;
+}
-- 
2.18.0

---
 docs/nbdkit-captive.pod                       |  4 ++--
 docs/nbdkit-plugin.pod                        |  2 +-
 docs/nbdkit-service.pod                       |  2 +-
 docs/nbdkit-tls.pod                           |  2 +-
 filters/cow/nbdkit-cow-filter.pod             |  4 ++--
 filters/delay/nbdkit-delay-filter.pod         |  2 +-
 filters/error/nbdkit-error-filter.pod         |  6 +++---
 filters/fua/nbdkit-fua-filter.pod             |  8 ++++----
 filters/log/nbdkit-log-filter.pod             |  4 ++--
 filters/nozero/nbdkit-nozero-filter.pod       |  4 ++--
 filters/offset/nbdkit-offset-filter.pod       |  4 ++--
 filters/partition/nbdkit-partition-filter.pod |  2 +-
 plugins/file/file.c                           |  1 +
 plugins/file/nbdkit-file-plugin.pod           |  7 ++++++-
 tests/test-blocksize.sh                       |  4 ++--
 tests/test-cache.sh                           |  2 +-
 tests/test-cow.sh                             |  2 +-
 tests/test-file-block.c                       |  3 +--
 tests/test-file.c                             |  2 +-
 tests/test-fua.sh                             |  8 ++++----
 tests/test-log.sh                             |  2 +-
 tests/test-nbd.c                              |  2 +-
 tests/test-newstyle.c                         |  2 +-
 tests/test-nozero.sh                          | 10 +++++-----
 tests/test-offset.c                           |  2 +-
 tests/test-oldstyle.c                         |  2 +-
 tests/test-parallel-file.sh                   |  4 ++--
 tests/test-parallel-nbd.sh                    |  2 +-
 tests/test-partition.c                        |  2 +-
 tests/test-single.sh                          |  2 +-
 30 files changed, 54 insertions(+), 49 deletions(-)

diff --git a/docs/nbdkit-captive.pod b/docs/nbdkit-captive.pod
index 6db9520..340374f 100644
--- a/docs/nbdkit-captive.pod
+++ b/docs/nbdkit-captive.pod
@@ -26,7 +26,7 @@ Some examples should make this clear.
 
 To run nbdkit captive under qemu:
 
- nbdkit file file=disk.img --run 'qemu -drive file=$nbd,if=virtio'
+ nbdkit file disk.img --run 'qemu -drive file=$nbd,if=virtio'
 
 On the qemu command line, C<$nbd> is substituted automatically with
 the right NBD path so it can connect to nbdkit.  When qemu exits,
@@ -34,7 +34,7 @@ nbdkit is killed and cleaned up automatically.
 
 Running nbdkit captive under guestfish:
 
- nbdkit file file=disk.img --run 'guestfish --format=raw -a $nbd -i'
+ nbdkit file disk.img --run 'guestfish --format=raw -a $nbd -i'
 
 When guestfish exits, nbdkit is killed.
 
diff --git a/docs/nbdkit-plugin.pod b/docs/nbdkit-plugin.pod
index fb5c7d7..380565b 100644
--- a/docs/nbdkit-plugin.pod
+++ b/docs/nbdkit-plugin.pod
@@ -267,7 +267,7 @@ an indication of failure.  It has the following prototype:
 The server usually (not always) changes directory to C</> before it
 starts serving connections.  This means that any relative paths passed
 during configuration will not work when the server is running
-(example: S<C<nbdkit plugin.so file=disk.img>>).
+(example: S<C<nbdkit plugin.so disk.img>>).
 
 To avoid problems, prepend relative paths with the current directory
 before storing them in the handle.  Or open files and store the file
diff --git a/docs/nbdkit-service.pod b/docs/nbdkit-service.pod
index 956b4ab..c0d275f 100644
--- a/docs/nbdkit-service.pod
+++ b/docs/nbdkit-service.pod
@@ -45,7 +45,7 @@ Also create a service unit (eg.
C</etc/systemd/system/nbdkit.service>)
 containing:
 
  [Service]
- ExecStart=/usr/sbin/nbdkit file file=/path/to/serve
+ ExecStart=/usr/sbin/nbdkit file /path/to/serve
 
 For more information on systemd and socket activation, see
 L<http://0pointer.de/blog/projects/socket-activation.html>
diff --git a/docs/nbdkit-tls.pod b/docs/nbdkit-tls.pod
index 2e70739..ecca87a 100644
--- a/docs/nbdkit-tls.pod
+++ b/docs/nbdkit-tls.pod
@@ -194,7 +194,7 @@ client which can read this file will be able to connect to
the server.
 
 Use the nbdkit I<--tls-psk> option to start the server:
 
- nbdkit --tls=require --tls-psk=/tmp/keys/keys.psk -e / file file=disk.img
+ nbdkit --tls=require --tls-psk=/tmp/keys/keys.psk -e / file disk.img
 
 This option overrides X.509 certificate authentication.
 
diff --git a/filters/cow/nbdkit-cow-filter.pod
b/filters/cow/nbdkit-cow-filter.pod
index ed48f2b..80100e7 100644
--- a/filters/cow/nbdkit-cow-filter.pod
+++ b/filters/cow/nbdkit-cow-filter.pod
@@ -59,7 +59,7 @@ normal way.
 Serve the file F<disk.img>, allowing writes, but do not save any
 changes into the file:
 
- nbdkit --filter=cow file file=disk.img
+ nbdkit --filter=cow file disk.img
 
 L<nbdkit-xz-plugin(1)> only supports read access, but you can provide
 temporary write access by doing (although this does B<not> save
@@ -76,7 +76,7 @@ otherwise all changes will be lost>.
 
 Run nbdkit:
 
- nbdkit --filter=cow file file=disk.img
+ nbdkit --filter=cow file disk.img
 
 and then connect with a client and make whatever changes you need.
 At the end, disconnect the client.
diff --git a/filters/delay/nbdkit-delay-filter.pod
b/filters/delay/nbdkit-delay-filter.pod
index 3bfb039..62c0093 100644
--- a/filters/delay/nbdkit-delay-filter.pod
+++ b/filters/delay/nbdkit-delay-filter.pod
@@ -16,7 +16,7 @@ remote server, or to test certain kinds of race conditions in
Linux.
 
 =head1 EXAMPLE
 
- nbdkit --filter=delay file file=disk.img rdelay=100ms wdelay=100ms
+ nbdkit --filter=delay file disk.img rdelay=100ms wdelay=100ms
 
 =head1 PARAMETERS
 
diff --git a/filters/error/nbdkit-error-filter.pod
b/filters/error/nbdkit-error-filter.pod
index 0f02282..76003c7 100644
--- a/filters/error/nbdkit-error-filter.pod
+++ b/filters/error/nbdkit-error-filter.pod
@@ -27,12 +27,12 @@ B<otherwise this filter will do nothing>.
 
 Inject a low rate of errors randomly into the connection:
 
- nbdkit --filter=error file file=disk.img error-rate=1%
+ nbdkit --filter=error file disk.img error-rate=1%
 
 Reading and trimming requests will be successful, but all
 writes and zeroing will return "No space left on device":
 
- nbdkit --filter=error file file=disk.img \
+ nbdkit --filter=error file disk.img \
                             error=ENOSPC \
                             error-pwrite-rate=100% \
                             error-zero-rate=100%
@@ -41,7 +41,7 @@ To make all connections fail hard 60 seconds after the server
is
 started, use:
 
  rm -f /tmp/inject
- nbdkit --filter=error file file=disk.img \
+ nbdkit --filter=error file disk.img \
                             error-rate=100% \
                             error-file=/tmp/inject
  sleep 60; touch /tmp/inject
diff --git a/filters/fua/nbdkit-fua-filter.pod
b/filters/fua/nbdkit-fua-filter.pod
index 8822a45..e0b3c4d 100644
--- a/filters/fua/nbdkit-fua-filter.pod
+++ b/filters/fua/nbdkit-fua-filter.pod
@@ -42,23 +42,23 @@ C<NBDKIT_FUA_NONE>).
 Serve the file F<disk.img>, but force the client to submit explicit
 flush requests instead of using C<NBD_CMD_FLAG_FUA>:
 
- nbdkit --filter=fua file file=disk.img
+ nbdkit --filter=fua file disk.img
 
 Observe that the blocksize filter optimizes its handling of the FUA
 flag based on whether it knows nbdkit will be emulating FUA with a
 flush, by comparing the log filter output on top of different fua
 filter modes:
 
- nbdkit --filter=blocksize --filter=log --filter=fua file file=disk.img \
+ nbdkit --filter=blocksize --filter=log --filter=fua file disk.img \
    maxlen=4k logfile=fua_emulated fuamode=emulate
- nbdkit --filter=blocksize --filter=log --filter=fua file file=disk.img \
+ nbdkit --filter=blocksize --filter=log --filter=fua file disk.img \
    maxlen=4k logfile=fua_native fuamode=native
 
 Serve the file F<disk.img> in write-through mode, where all writes
 from the client are immediately flushed to disk as if the client had
 always requested FUA:
 
- nbdkit --filter=fua file fuamode=force file=disk.img
+ nbdkit --filter=fua file fuamode=force disk.img
 
 =head1 SEE ALSO
 
diff --git a/filters/log/nbdkit-log-filter.pod
b/filters/log/nbdkit-log-filter.pod
index 89ea551..0903329 100644
--- a/filters/log/nbdkit-log-filter.pod
+++ b/filters/log/nbdkit-log-filter.pod
@@ -29,12 +29,12 @@ already exists, it will be truncated.
 Serve the file F<disk.img>, and log each client transaction in the
 file F<disk.log>:
 
- nbdkit --filter=log file file=disk.img logfile=disk.log
+ nbdkit --filter=log file disk.img logfile=disk.log
 
 Repeat the task, but with the cow (copy-on-write) filter to perform
 local caching of data served from the original plugin:
 
- nbdkit --filter=cow --filter=log file file=disk.img logfile=disk.log2
+ nbdkit --filter=cow --filter=log file disk.img logfile=disk.log2
 
 After running a client that performs the same operations under each of
 the two servers, you can compare F<disk.log> and F<disk.log2> to
see
diff --git a/filters/nozero/nbdkit-nozero-filter.pod
b/filters/nozero/nbdkit-nozero-filter.pod
index 1301432..715605f 100644
--- a/filters/nozero/nbdkit-nozero-filter.pod
+++ b/filters/nozero/nbdkit-nozero-filter.pod
@@ -34,13 +34,13 @@ way to write zeros.
 Serve the file F<disk.img>, but force the client to write zeroes
 explicitly rather than with C<NBD_CMD_WRITE_ZEROES>:
 
- nbdkit --filter=nozero file file=disk.img
+ nbdkit --filter=nozero file disk.img
 
 Serve the file F<disk.img>, allowing the client to take advantage of
 less network traffic via C<NBD_CMD_WRITE_ZEROES>, but still forcing
 the data to be written explicitly rather than punching any holes:
 
- nbdkit --filter=nozero file zeromode=emulate file=disk.img
+ nbdkit --filter=nozero file zeromode=emulate disk.img
 
 =head1 SEE ALSO
 
diff --git a/filters/offset/nbdkit-offset-filter.pod
b/filters/offset/nbdkit-offset-filter.pod
index 61496d2..a232e46 100644
--- a/filters/offset/nbdkit-offset-filter.pod
+++ b/filters/offset/nbdkit-offset-filter.pod
@@ -44,7 +44,7 @@ Using L<nbdkit-file-plugin(1)>, serve the file
C<disk.img> starting at
 offset C<1M>.  The total length served is C<100M> (the underlying
file
 must therefore be at least C<101M> in length):
 
- nbdkit --filter=offset file file=disk.img offset=1M range=100M
+ nbdkit --filter=offset file disk.img offset=1M range=100M
 
 =head2 Serve a single partition
 
@@ -60,7 +60,7 @@ and length of the partition, eg using:
 
 You can then serve the partition only using:
 
- nbdkit --filter=offset file file=disk.img offset=65536 range=104727040
+ nbdkit --filter=offset file disk.img offset=65536 range=104727040
 
 =head1 SEE ALSO
 
diff --git a/filters/partition/nbdkit-partition-filter.pod
b/filters/partition/nbdkit-partition-filter.pod
index 0256e52..ae72f3f 100644
--- a/filters/partition/nbdkit-partition-filter.pod
+++ b/filters/partition/nbdkit-partition-filter.pod
@@ -38,7 +38,7 @@ This parameter is required.
 F<disk.img> is a partitioned disk image (eg. a virtual machine disk
 image).  To serve the first partition only use:
 
- nbdkit --filter=partition file file=disk.img partition=1
+ nbdkit --filter=partition file disk.img partition=1
 
 =head1 SEE ALSO
 
diff --git a/plugins/file/file.c b/plugins/file/file.c
index e7cbff6..9d03f18 100644
--- a/plugins/file/file.c
+++ b/plugins/file/file.c
@@ -476,6 +476,7 @@ static struct nbdkit_plugin plugin = {
   .config            = file_config,
   .config_complete   = file_config_complete,
   .config_help       = file_config_help,
+  .magic_config_key  = "file",
   .dump_plugin       = file_dump_plugin,
   .open              = file_open,
   .close             = file_close,
diff --git a/plugins/file/nbdkit-file-plugin.pod
b/plugins/file/nbdkit-file-plugin.pod
index b2c25d1..7d5c71b 100644
--- a/plugins/file/nbdkit-file-plugin.pod
+++ b/plugins/file/nbdkit-file-plugin.pod
@@ -4,7 +4,7 @@ nbdkit-file-plugin - nbdkit file plugin
 
 =head1 SYNOPSIS
 
- nbdkit file file=FILENAME
+ nbdkit file [file=]FILENAME
 
 =head1 DESCRIPTION
 
@@ -26,6 +26,11 @@ be used here.
 
 This parameter is required.
 
+In nbdkit E<ge> 1.8, C<file=> may be omitted.  To ensure that the
+filename does not end up being parsed accidentally as C<key=value>,
+prefix relative paths with C<./> (absolute paths do not need
+modification).
+
 =item B<rdelay>
 
 =item B<wdelay>
diff --git a/tests/test-blocksize.sh b/tests/test-blocksize.sh
index d239c9b..cb9b09f 100755
--- a/tests/test-blocksize.sh
+++ b/tests/test-blocksize.sh
@@ -73,9 +73,9 @@ trap cleanup INT QUIT TERM EXIT ERR
 
 # Run two parallel nbdkit; to compare the logs and see what changes.
 nbdkit -P blocksize1.pid -U blocksize1.sock \
-       --filter=log file logfile=blocksize1.log file=blocksize1.img
+       --filter=log file logfile=blocksize1.log blocksize1.img
 nbdkit -P blocksize2.pid -U blocksize2.sock --filter=blocksize \
-       --filter=log file logfile=blocksize2.log file=blocksize2.img \
+       --filter=log file logfile=blocksize2.log blocksize2.img \
        minblock=1024 maxdata=512k maxlen=1M
 
 # We may have to wait a short time for the pid files to appear.
diff --git a/tests/test-cache.sh b/tests/test-cache.sh
index 6f058b8..215bab1 100755
--- a/tests/test-cache.sh
+++ b/tests/test-cache.sh
@@ -41,7 +41,7 @@ rm -f $files
 truncate -s 1G cache.img
 
 # Run nbdkit with the caching filter.
-nbdkit -P cache.pid -U cache.sock --filter=cache file file=cache.img
+nbdkit -P cache.pid -U cache.sock --filter=cache file cache.img
 
 # We may have to wait a short time for the pid file to appear.
 for i in `seq 1 10`; do
diff --git a/tests/test-cow.sh b/tests/test-cow.sh
index 5bde829..f9b0649 100755
--- a/tests/test-cow.sh
+++ b/tests/test-cow.sh
@@ -42,7 +42,7 @@ guestfish -N cow-base.img=fs exit
 lastmod="$(stat -c "%y" cow-base.img)"
 
 # Run nbdkit with a COW overlay.
-nbdkit -P cow.pid -U cow.sock --filter=cow file file=cow-base.img
+nbdkit -P cow.pid -U cow.sock --filter=cow file cow-base.img
 
 # We may have to wait a short time for the pid file to appear.
 for i in `seq 1 10`; do
diff --git a/tests/test-file-block.c b/tests/test-file-block.c
index f053242..e2ea068 100644
--- a/tests/test-file-block.c
+++ b/tests/test-file-block.c
@@ -124,9 +124,8 @@ main (int argc, char *argv[])
   atexit (detach_loopdev);
 
   /* Start nbdkit. */
-  snprintf (buf, sizeof buf, "file=%s", loopdev);
   if (test_start_nbdkit ("-D", "file.zero=1",
-                         "file", buf, NULL) == -1)
+                         "file", loopdev, NULL) == -1)
     exit (EXIT_FAILURE);
 
   g = guestfs_create ();
diff --git a/tests/test-file.c b/tests/test-file.c
index 65a2568..9382ed2 100644
--- a/tests/test-file.c
+++ b/tests/test-file.c
@@ -52,7 +52,7 @@ main (int argc, char *argv[])
   char *data;
   size_t i, size;
 
-  if (test_start_nbdkit ("file", "file=file-data", NULL) ==
-1)
+  if (test_start_nbdkit ("file", "file-data", NULL) == -1)
     exit (EXIT_FAILURE);
 
   g = guestfs_create ();
diff --git a/tests/test-fua.sh b/tests/test-fua.sh
index 98b1e84..f95aa18 100755
--- a/tests/test-fua.sh
+++ b/tests/test-fua.sh
@@ -84,13 +84,13 @@ trap cleanup INT QUIT TERM EXIT ERR
 # 3: fuamode=native: log shows that blocksize preserves fua
 # 4: fuamode=force: log shows that fua is always enabled
 nbdkit -P fua1.pid -U fua1.sock --filter=log --filter=fua \
-       file logfile=fua1.log file=fua.img
+       file logfile=fua1.log fua.img
 nbdkit -P fua2.pid -U fua2.sock --filter=blocksize --filter=log --filter=fua \
-       file logfile=fua2.log file=fua.img fuamode=emulate maxdata=4k maxlen=4k
+       file logfile=fua2.log fua.img fuamode=emulate maxdata=4k maxlen=4k
 nbdkit -P fua3.pid -U fua3.sock --filter=blocksize --filter=log --filter=fua \
-       file logfile=fua3.log file=fua.img fuamode=native maxdata=4k maxlen=4k
+       file logfile=fua3.log fua.img fuamode=native maxdata=4k maxlen=4k
 nbdkit -P fua4.pid -U fua4.sock --filter=fua --filter=log \
-       file logfile=fua4.log file=fua.img fuamode=force
+       file logfile=fua4.log fua.img fuamode=force
 
 # We may have to wait a short time for the pid files to appear.
 for i in `seq 1 10`; do
diff --git a/tests/test-log.sh b/tests/test-log.sh
index 877f9cc..f811de4 100755
--- a/tests/test-log.sh
+++ b/tests/test-log.sh
@@ -44,7 +44,7 @@ if ! qemu-io -f raw -c 'w 1M 2M' log.img; then
 fi
 
 # Run nbdkit with logging enabled to file.
-nbdkit -P log.pid -U log.sock --filter=log file file=log.img logfile=log.log
+nbdkit -P log.pid -U log.sock --filter=log file log.img logfile=log.log
 
 # We may have to wait a short time for the pid file to appear.
 for i in `seq 1 10`; do
diff --git a/tests/test-nbd.c b/tests/test-nbd.c
index 646c0c3..a09a0d5 100644
--- a/tests/test-nbd.c
+++ b/tests/test-nbd.c
@@ -55,7 +55,7 @@ main (int argc, char *argv[])
 
   /* If wrapping once is good, why not do it twice!  Shows that we can
    * convert between either style of server options. */
-  if (test_start_nbdkit ("-o", "file",
"file=file-data", NULL) == -1)
+  if (test_start_nbdkit ("-o", "file",
"file-data", NULL) == -1)
     exit (EXIT_FAILURE);
 
   if (asprintf (&sockarg, "socket=%s", sock) < 0) {
diff --git a/tests/test-newstyle.c b/tests/test-newstyle.c
index cd0ba72..cadccec 100644
--- a/tests/test-newstyle.c
+++ b/tests/test-newstyle.c
@@ -55,7 +55,7 @@ main (int argc, char *argv[])
   size_t i, size;
 
   if (test_start_nbdkit ("-e", EXPORTNAME,
-                         "-n", "file",
"file=file-data", NULL) == -1)
+                         "-n", "file",
"file-data", NULL) == -1)
     exit (EXIT_FAILURE);
 
   g = guestfs_create ();
diff --git a/tests/test-nozero.sh b/tests/test-nozero.sh
index 781b196..904d822 100755
--- a/tests/test-nozero.sh
+++ b/tests/test-nozero.sh
@@ -102,15 +102,15 @@ trap cleanup INT QUIT TERM EXIT ERR
 # 5a/b: both sides of nbd plugin: even though server side does not advertise
 # ZERO, the client side still exposes it, and just skips calling nbd's
.zero
 nbdkit -P nozero1.pid -U nozero1.sock --filter=log \
-       file logfile=nozero1.log file=nozero1.img
+       file logfile=nozero1.log nozero1.img
 nbdkit -P nozero2.pid -U nozero2.sock --filter=log --filter=nozero \
-       file logfile=nozero2.log file=nozero2.img
+       file logfile=nozero2.log nozero2.img
 nbdkit -P nozero3.pid -U nozero3.sock --filter=log --filter=nozero \
-       file logfile=nozero3.log file=nozero3.img zeromode=emulate
+       file logfile=nozero3.log nozero3.img zeromode=emulate
 nbdkit -P nozero4.pid -U nozero4.sock --filter=nozero --filter=log \
-       file logfile=nozero4.log file=nozero4.img zeromode=emulate
+       file logfile=nozero4.log nozero4.img zeromode=emulate
 nbdkit -P nozero5a.pid -U nozero5a.sock --filter=log --filter=nozero \
-       file logfile=nozero5a.log file=nozero5.img
+       file logfile=nozero5a.log nozero5.img
 nbdkit -P nozero5b.pid -U nozero5b.sock --filter=log \
        nbd logfile=nozero5b.log socket=nozero5a.sock
 
diff --git a/tests/test-offset.c b/tests/test-offset.c
index d47066f..a7f467f 100644
--- a/tests/test-offset.c
+++ b/tests/test-offset.c
@@ -78,7 +78,7 @@ main (int argc, char *argv[])
    * has not been overwritten.
    */
   if (test_start_nbdkit ("--filter", "offset",
-                         "file", "file=offset-data",
+                         "file", "offset-data",
                          "offset=1M", "range=8M",
                          NULL) == -1)
     exit (EXIT_FAILURE);
diff --git a/tests/test-oldstyle.c b/tests/test-oldstyle.c
index bcffbe4..b3f3c4e 100644
--- a/tests/test-oldstyle.c
+++ b/tests/test-oldstyle.c
@@ -52,7 +52,7 @@ main (int argc, char *argv[])
   char *data;
   size_t i, size;
 
-  if (test_start_nbdkit ("-o", "file",
"file=file-data", NULL) == -1)
+  if (test_start_nbdkit ("-o", "file",
"file-data", NULL) == -1)
     exit (EXIT_FAILURE);
 
   g = guestfs_create ();
diff --git a/tests/test-parallel-file.sh b/tests/test-parallel-file.sh
index fdc8a1b..fd88faa 100755
--- a/tests/test-parallel-file.sh
+++ b/tests/test-parallel-file.sh
@@ -56,7 +56,7 @@ qemu-io -f raw -c "aio_write -P 1 0 512" -c
"aio_write -P 2 512 512" \
 # tuning the delays may help.
 
 # With --threads=1, the write should complete first because it was issued first
-nbdkit -v -t 1 -U - --filter=delay file file=test-parallel-file.data \
+nbdkit -v -t 1 -U - --filter=delay file test-parallel-file.data \
   wdelay=2 rdelay=1 --run 'qemu-io -f raw -c "aio_write -P 2 512
512" \
                            -c "aio_read -P 1 0 512" -c aio_flush
$nbd' |
     tee test-parallel-file.out
@@ -67,7 +67,7 @@ read 512/512 bytes at offset 0"; then
 fi
 
 # With default --threads, the faster read should complete first
-nbdkit -v -U - --filter=delay file file=test-parallel-file.data \
+nbdkit -v -U - --filter=delay file test-parallel-file.data \
   wdelay=2 rdelay=1 --run 'qemu-io -f raw -c "aio_write -P 2 512
512" \
                            -c "aio_read -P 1 0 512" -c aio_flush
$nbd' |
     tee test-parallel-file.out
diff --git a/tests/test-parallel-nbd.sh b/tests/test-parallel-nbd.sh
index e793c11..cb70f46 100755
--- a/tests/test-parallel-nbd.sh
+++ b/tests/test-parallel-nbd.sh
@@ -64,7 +64,7 @@ qemu-io -f raw -c "aio_write -P 1 0 512" -c
"aio_write -P 2 512 512" \
 nbdkit --exit-with-parent -v \
     -U test-parallel-nbd.sock -P test-parallel-nbd.pid \
     --filter=delay \
-    file file=test-parallel-nbd.data wdelay=2 rdelay=1 &
+    file test-parallel-nbd.data wdelay=2 rdelay=1 &
 # We may have to wait a short time for the pid file to appear.
 for i in `seq 1 10`; do
     if test -f test-parallel-nbd.pid; then
diff --git a/tests/test-partition.c b/tests/test-partition.c
index 0087548..1064293 100644
--- a/tests/test-partition.c
+++ b/tests/test-partition.c
@@ -53,7 +53,7 @@ main (int argc, char *argv[])
 
   if (test_start_nbdkit ("-r",
                          "--filter", "partition",
-                         "file", "file=disk",
+                         "file", "disk",
                          "partition=1",
                          NULL) == -1)
     exit (EXIT_FAILURE);
diff --git a/tests/test-single.sh b/tests/test-single.sh
index dc6ce34..35017c8 100755
--- a/tests/test-single.sh
+++ b/tests/test-single.sh
@@ -51,7 +51,7 @@ rm -f $files
 truncate -s 1G single.disk
 
 socat unix-listen:single.sock,reuseaddr,fork \
-    exec:'nbdkit -r -s file file=single.disk' &
+    exec:'nbdkit -r -s file single.disk' &
 pid=$!
 
 cleanup ()
-- 
2.18.0

---
 plugins/split/nbdkit-split-plugin.pod | 9 +++++++--
 plugins/split/split.c                 | 3 ++-
 tests/test-split.c                    | 3 ++-
 3 files changed, 11 insertions(+), 4 deletions(-)

diff --git a/plugins/split/nbdkit-split-plugin.pod
b/plugins/split/nbdkit-split-plugin.pod
index 6a6a01a..6f62eb3 100644
--- a/plugins/split/nbdkit-split-plugin.pod
+++ b/plugins/split/nbdkit-split-plugin.pod
@@ -4,12 +4,12 @@ nbdkit-split-plugin - nbdkit plugin to concatenate split files
into one disk
 
 =head1 SYNOPSIS
 
- nbdkit split file=file1 [file=file2 file=file3 ...]
+ nbdkit split [file=]file1 [[file=]file2 [file=]file3 ...]
 
 =head1 DESCRIPTION
 
 C<nbdkit-split-plugin> is a file plugin for L<nbdkit(1)>.  One or
more
-filenames may be given using the C<file=FILENAME> parameter.  These
+filenames may be given using the C<FILENAME> parameter.  These
 files are logically concatenated into a single disk image.
 
 =head2 Differences from nbdkit-file-plugin
@@ -62,6 +62,11 @@ the order they appear on the command line.
 
 This parameter must appear at least once.
 
+In nbdkit E<ge> 1.8, C<file=> may be omitted.  To ensure that the
+filename does not end up being parsed accidentally as C<key=value>,
+prefix relative paths with C<./> (absolute paths do not need
+modification).
+
 =back
 
 =head1 SEE ALSO
diff --git a/plugins/split/split.c b/plugins/split/split.c
index bdcdcf7..7572316 100644
--- a/plugins/split/split.c
+++ b/plugins/split/split.c
@@ -1,5 +1,5 @@
 /* nbdkit
- * Copyright (C) 2017 Red Hat Inc.
+ * Copyright (C) 2017-2018 Red Hat Inc.
  * All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
@@ -286,6 +286,7 @@ static struct nbdkit_plugin plugin = {
   .unload            = split_unload,
   .config            = split_config,
   .config_help       = split_config_help,
+  .magic_config_key  = "file",
   .open              = split_open,
   .close             = split_close,
   .get_size          = split_get_size,
diff --git a/tests/test-split.c b/tests/test-split.c
index f20e64b..c4f2a74 100644
--- a/tests/test-split.c
+++ b/tests/test-split.c
@@ -53,7 +53,8 @@ main (int argc, char *argv[])
   size_t i, size;
 
   if (test_start_nbdkit ("split",
-                         "file=split1", "file=split2",
"file=split3",
+                         "split1", "split2",
+                         "file=split3" /* leave file= to test */,
                          NULL) == -1)
     exit (EXIT_FAILURE);
 
-- 
2.18.0

---
 plugins/gzip/gzip.c                 |  3 ++-
 plugins/gzip/nbdkit-gzip-plugin.pod | 19 ++++++++++++++++++-
 2 files changed, 20 insertions(+), 2 deletions(-)

diff --git a/plugins/gzip/gzip.c b/plugins/gzip/gzip.c
index 09dd629..26d5e3c 100644
--- a/plugins/gzip/gzip.c
+++ b/plugins/gzip/gzip.c
@@ -1,5 +1,5 @@
 /* nbdkit
- * Copyright (C) 2013 Red Hat Inc.
+ * Copyright (C) 2013-2018 Red Hat Inc.
  * All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
@@ -217,6 +217,7 @@ static struct nbdkit_plugin plugin = {
   .config            = gzip_config,
   .config_complete   = gzip_config_complete,
   .config_help       = gzip_config_help,
+  .magic_config_key  = "file",
   .open              = gzip_open,
   .close             = gzip_close,
   .get_size          = gzip_get_size,
diff --git a/plugins/gzip/nbdkit-gzip-plugin.pod
b/plugins/gzip/nbdkit-gzip-plugin.pod
index a5f6b79..bd8dbda 100644
--- a/plugins/gzip/nbdkit-gzip-plugin.pod
+++ b/plugins/gzip/nbdkit-gzip-plugin.pod
@@ -4,7 +4,7 @@ nbdkit-gzip-plugin - nbdkit gzip plugin
 
 =head1 SYNOPSIS
 
- nbdkit gzip file=FILENAME.gz
+ nbdkit gzip [file=]FILENAME.gz
 
 =head1 DESCRIPTION
 
@@ -18,6 +18,23 @@ files because seeking to a position in the gzip file involves
 uncompressing lots of data.  A more practical method to compress large
 disk images is to use the L<xz(1)> format and
L<nbdkit-xz-plugin(1)>.
 
+=head1 PARAMETERS
+
+=over 4
+
+=item B<file=>FILENAME.gz
+
+Serve the compressed file named C<FILENAME.gz>.
+
+This parameter is required.
+
+In nbdkit E<ge> 1.8, C<file=> may be omitted.  To ensure that the
+filename does not end up being parsed accidentally as C<key=value>,
+prefix relative paths with C<./> (absolute paths do not need
+modification).
+
+=back
+
 =head1 SEE ALSO
 
 L<nbdkit-xz-plugin(1)>,
-- 
2.18.0

---
 docs/nbdkit.pod                   | 8 ++++----
 filters/cow/nbdkit-cow-filter.pod | 2 +-
 plugins/xz/nbdkit-xz-plugin.pod   | 7 ++++++-
 plugins/xz/xz.c                   | 3 ++-
 tests/test-xz.c                   | 2 +-
 5 files changed, 14 insertions(+), 8 deletions(-)

diff --git a/docs/nbdkit.pod b/docs/nbdkit.pod
index cf1c5ca..d650283 100644
--- a/docs/nbdkit.pod
+++ b/docs/nbdkit.pod
@@ -75,7 +75,7 @@ Serve only the first partition from compressed disk image
 F<disk.img.xz>, combining L<nbdkit-xz-plugin(1)> and
 L<nbdkit-partition-filter(1)>:
 
- nbdkit --filter=partition  xz file=disk.img.xz  partition=1
+ nbdkit --filter=partition  xz disk.img.xz  partition=1
 
 To understand this command line:
 
@@ -83,9 +83,9 @@ To understand this command line:
                                   │
                             ┌─────┴────┐
                             │          │
- nbdkit --filter=partition  xz file=disk.img.xz  partition=1
-                │                                   │
-                └───────────────┬───────────────────┘
+ nbdkit --filter=partition  xz disk.img.xz  partition=1
+                  │                               │
+                  └─────────────┬─────────────────┘
                                 │
                 filter name and filter parameter
 
diff --git a/filters/cow/nbdkit-cow-filter.pod
b/filters/cow/nbdkit-cow-filter.pod
index 80100e7..d5cd735 100644
--- a/filters/cow/nbdkit-cow-filter.pod
+++ b/filters/cow/nbdkit-cow-filter.pod
@@ -65,7 +65,7 @@ L<nbdkit-xz-plugin(1)> only supports read access, but
you can provide
 temporary write access by doing (although this does B<not> save
 changes to the file):
 
- nbdkit --filter=cow xz file=disk.xz
+ nbdkit --filter=cow xz disk.xz
 
 =head1 CREATING A DIFF WITH QEMU-IMG
 
diff --git a/plugins/xz/nbdkit-xz-plugin.pod b/plugins/xz/nbdkit-xz-plugin.pod
index 2c9ab6e..2bd0a1c 100644
--- a/plugins/xz/nbdkit-xz-plugin.pod
+++ b/plugins/xz/nbdkit-xz-plugin.pod
@@ -4,7 +4,7 @@ nbdkit-xz-plugin - nbdkit xz plugin
 
 =head1 SYNOPSIS
 
- nbdkit xz file=FILENAME.xz
+ nbdkit xz [file=]FILENAME.xz
 
 =head1 DESCRIPTION
 
@@ -59,6 +59,11 @@ Serve the file named C<FILENAME.xz>.
 
 This parameter is required.
 
+In nbdkit E<ge> 1.8, C<file=> may be omitted.  To ensure that the
+filename does not end up being parsed accidentally as C<key=value>,
+prefix relative paths with C<./> (absolute paths do not need
+modification).
+
 =item B<maxblock=>SIZE
 
 The maximum block size that the plugin will read.  The plugin will
diff --git a/plugins/xz/xz.c b/plugins/xz/xz.c
index f45e489..a88d713 100644
--- a/plugins/xz/xz.c
+++ b/plugins/xz/xz.c
@@ -1,5 +1,5 @@
 /* nbdkit
- * Copyright (C) 2013 Red Hat Inc.
+ * Copyright (C) 2013-2018 Red Hat Inc.
  * All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
@@ -250,6 +250,7 @@ static struct nbdkit_plugin plugin = {
   .config            = xz_config,
   .config_complete   = xz_config_complete,
   .config_help       = xz_config_help,
+  .magic_config_key  = "file",
   .open              = xz_open,
   .close             = xz_close,
   .get_size          = xz_get_size,
diff --git a/tests/test-xz.c b/tests/test-xz.c
index ef58d59..46b1d04 100644
--- a/tests/test-xz.c
+++ b/tests/test-xz.c
@@ -51,7 +51,7 @@ main (int argc, char *argv[])
   int r;
   char *data;
 
-  if (test_start_nbdkit ("xz", "-r",
"file=disk.xz", NULL) == -1)
+  if (test_start_nbdkit ("xz", "-r", "disk.xz",
NULL) == -1)
     exit (EXIT_FAILURE);
 
   g = guestfs_create ();
-- 
2.18.0

On 9/10/18 10:27 AM, Richard W.M. Jones wrote:
> Previously key=value on the command line allowed the key to be pretty
> much anything that didn't contain an '=' character.  Even empty
> strings were permitted.
> 
> This tightens up the permitted keys so they must contain only ASCII
> alphanumeric, period, underscore or dash characters; must not be an
> empty string; and must start with an ASCII alphabetic character.
> ---
>   docs/nbdkit-plugin.pod | 24 +++++++++++++++---------
>   src/main.c             | 36 +++++++++++++++++++++++++++++++++++-
>   2 files changed, 50 insertions(+), 10 deletions(-)
> 
> +Both C<key> and C<value> parameters will be non-NULL.
> +
> +They key will be a non-empty string beginning with an ASCII alphabetic
s/They/The/
> +character (C<A-Z> C<a-z>).  The rest of the key must contain
only
> +ASCII alphanumeric plus period, underscore or dash characters
(C<A-Z>
> +C<a-z> C<0-9> C<.> C<_> C<->).
No leading underscore? I'm okay with that, but it is slightly stricter 
than C and Shell variables (then again, accepting '.' and '-'
already
makes us different).  And we can always add stuff later if it proves 
useful (it's easier to start strict and relax later, compared to 
starting relaxed and then tightening down when we realize the problems 
it caused).
> +/* When parsing plugin and filter config key=value from the command
> + * line, check that the key is a simple alphanumeric with period,
> + * underscore or dash.
> + *
> + * Note this doesn't return an error.  If the key is not valid then we
> + * return false and the parsing code will assume that this is a bare
> + * value instead.
> + */
> +static int
> +is_config_key (const char *key, size_t len)
> +{
> +  static const char allowed_first[] > +   
"abcdefghijklmnopqrstuvwxyz"
> +    "ABCDEFGHIJKLMNOPQRSTUVWXYZ";
> +  static const char allowed[] > +   
"abcdefghijklmnopqrstuvwxyz"
> +    "ABCDEFGHIJKLMNOPQRSTUVWXYZ"
> +    "0123456789"
> +    "._-";
Crazy me is thinking of the micro-optimization:

static const char allowed[]    "0123456789"
   ".-_"
   "abc..z"
   "ABC..Z";

...
> +
> +  if (len == 0)
> +    return 0;
> +
> +  if (strchr (allowed_first, key[0]) == NULL)
> +    return 0;
where this is: 'strchr(allowed + 13, key[0])' (or allowed + 12, if you 
allow leading underscore).  But please don't - while it's clever, I 
don't think it's friendly to future readers, and the optimization is not
in hot code, nor is shaving 52 bytes of .data by avoiding repetition 
between the two 'allowed' constants really a deal-breaker.
> +
> +  /* This works in context of the caller since key[len] == '='. */
> +  if (strspn (key, allowed) != len)
> +    return 0;
I like it. With the one typo fix,
Reviewed-by: Eric Blake <eblake@redhat.com>

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org