[Qemu-devel] [PATCH 1/4] curl: Fix parsing of readahead option from filename

[Matthew Booth <mbooth@redhat.com>]

curl_parse_filename wasn't removing the option string from the url,
resulting in a 404.

This change is a rewrite of the previous parsing behaviour, and
also changes the option syntax. The new syntax is:

  http://example.com/path?query[sslverify=off:readahead=64k]

The new syntax is well defined as long as square brackets in the URI
are escaped.

This change is also preparation for the addition of more options.

Signed-off-by: Matthew Booth <mbooth@redhat.com>
---
 block/curl.c | 101 +++++++++++++++++++++++++++++++++++++++--------------------
 1 file changed, 67 insertions(+), 34 deletions(-)

diff --git a/block/curl.c b/block/curl.c
index d2f1084..e31b6f3 100644
--- a/block/curl.c
+++ b/block/curl.c
@@ -46,12 +46,15 @@
 #define CURL_NUM_STATES 8
 #define CURL_NUM_ACB    8
 #define SECTOR_SIZE     512
-#define READ_AHEAD_SIZE (256 * 1024)
+#define READ_AHEAD_DEFAULT (256 * 1024)
 
 #define FIND_RET_NONE   0
 #define FIND_RET_OK     1
 #define FIND_RET_WAIT   2
 
+#define CURL_BLOCK_OPT_URL       "url"
+#define CURL_BLOCK_OPT_READAHEAD "readahead"
+
 struct BDRVCURLState;
 
 typedef struct CURLAIOCB {
@@ -393,45 +396,74 @@ static void curl_clean_state(CURLState *s)
     s->in_use = 0;
 }
 
+/* Parse options out of @filename, which should be a URI.
+ *
+ * Options are given as a sequence of key=value pairs separated by colons and
+ * enclosed in square brackets at the end of the URI. e.g.:
+ *
+ * http://example.com/path?query[sslverify=off:readahead=64k]
+ *
+ * Consequently, if a valid URI ends with a block enclosed by square brackets,
+ * those brackets MUST be URI encoded.
+ */
 static void curl_parse_filename(const char *filename, QDict *options,
                                 Error **errp)
 {
+    char *file = g_strdup(filename);
+    char *end = file + strlen(file) - 1;
+
+    /* Don't fail if we've been passed an empty string.
+     * Only examine strings with a trailing ']'
+     */
+    if (end >= file && ']' == *end) {
+        /* Look for the opening bracket */
+        char *open = memrchr(file, '[', end - file);
+
+        if (NULL != open) {
+            /* NULL terminate the preceding URI by overwriting the opening
+             * bracket */
+            *open = '\0';
+            char *opt_block = open + 1;
+
+            /* NULL terminate the option string by overwriting the closing
+             * bracket */
+            *end = '\0';
+
+            /* Parse each colon-separated option */
+            char *saveptr;
+            char *opt = strtok_r(opt_block, ":", &saveptr);
+            while (NULL != opt) {
+                size_t opt_len = strlen(opt);
+
+                /* Look for an equals sign */
+                char *equals = memchr(opt, '=', opt_len);
+                if (NULL == equals) {
+                    error_set(errp, QERR_INVALID_PARAMETER_VALUE, opt,
+                              "a value");
+                    goto out;
+                }
 
-    #define RA_OPTSTR ":readahead="
-    char *file;
-    char *ra;
-    const char *ra_val;
-    int parse_state = 0;
-
-    file = g_strdup(filename);
-
-    /* Parse a trailing ":readahead=#:" param, if present. */
-    ra = file + strlen(file) - 1;
-    while (ra >= file) {
-        if (parse_state == 0) {
-            if (*ra == ':') {
-                parse_state++;
-            } else {
-                break;
-            }
-        } else if (parse_state == 1) {
-            if (*ra > '9' || *ra < '0') {
-                char *opt_start = ra - strlen(RA_OPTSTR) + 1;
-                if (opt_start > file &&
-                    strncmp(opt_start, RA_OPTSTR, strlen(RA_OPTSTR)) == 0) {
-                    ra_val = ra + 1;
-                    ra -= strlen(RA_OPTSTR) - 1;
-                    *ra = '\0';
-                    qdict_put(options, "readahead", qstring_from_str(ra_val));
+                size_t key_len = equals - opt;
+                char *value = equals + 1;
+
+                if (key_len == strlen(CURL_BLOCK_OPT_READAHEAD) &&
+                    memcmp(opt, CURL_BLOCK_OPT_READAHEAD, key_len) == 0) {
+                    qdict_put(options, CURL_BLOCK_OPT_READAHEAD,
+                              qstring_from_str(value));
+                } else {
+                    *equals = '\0';
+                    error_set(errp, QERR_INVALID_PARAMETER, opt);
+                    goto out;
                 }
-                break;
+
+                opt = strtok_r(NULL, ":", &saveptr);
             }
         }
-        ra--;
     }
 
-    qdict_put(options, "url", qstring_from_str(file));
+    qdict_put(options, CURL_BLOCK_OPT_URL, qstring_from_str(file));
 
+out:
     g_free(file);
 }
 
@@ -440,12 +472,12 @@ static QemuOptsList runtime_opts = {
     .head = QTAILQ_HEAD_INITIALIZER(runtime_opts.head),
     .desc = {
         {
-            .name = "url",
+            .name = CURL_BLOCK_OPT_URL,
             .type = QEMU_OPT_STRING,
             .help = "URL to open",
         },
         {
-            .name = "readahead",
+            .name = CURL_BLOCK_OPT_READAHEAD,
             .type = QEMU_OPT_SIZE,
             .help = "Readahead size",
         },
@@ -477,14 +509,15 @@ static int curl_open(BlockDriverState *bs, QDict *options, int flags,
         goto out_noclean;
     }
 
-    s->readahead_size = qemu_opt_get_size(opts, "readahead", READ_AHEAD_SIZE);
+    s->readahead_size = qemu_opt_get_size(opts, CURL_BLOCK_OPT_READAHEAD,
+                                          READ_AHEAD_DEFAULT);
     if ((s->readahead_size & 0x1ff) != 0) {
         error_setg(errp, "HTTP_READAHEAD_SIZE %zd is not a multiple of 512",
                    s->readahead_size);
         goto out_noclean;
     }
 
-    file = qemu_opt_get(opts, "url");
+    file = qemu_opt_get(opts, CURL_BLOCK_OPT_URL);
     if (file == NULL) {
         error_setg(errp, "curl block driver requires an 'url' option");
         goto out_noclean;
-- 
1.9.0