MXS-1220: Add option parsing

The options of a request are now parsed and exposed by the HttpRequest
class.

Added tests for the request options.

Also added missing error handling of invalid requests.

server/core/httprequest.cc

@@ -14,6 +14,7 @@
 #include "maxscale/httprequest.hh"
 
 #include <ctype.h>
+#include <string.h>
 
 /** TODO: Move this to a C++ string utility header */
 namespace maxscale
@@ -52,17 +53,68 @@ static inline string& trim(string& str)
 HttpRequest* HttpRequest::parse(string data)
 {
     size_t pos = data.find("\r\n");
+
+    if (pos == string::npos)
+    {
+        return NULL;
+    }
+
     string request_line = data.substr(0, pos);
     data.erase(0, pos + 2);
 
-    pos = request_line.find(" ");
+    /** Request method */
+    if ((pos = request_line.find(" ")) == string::npos)
+    {
+        return NULL;
+    }
+
     string verb = request_line.substr(0, pos);
     request_line.erase(0, pos + 1);
 
-    pos = request_line.find(" ");
+    /** Get the combined URL/option string */
+    if ((pos = request_line.find(" ")) == string::npos)
+    {
+        return NULL;
+    }
+
     string uri = request_line.substr(0, pos);
     request_line.erase(0, pos + 1);
 
+    /** Process request options */
+    pos = uri.find("?");
+    map<string, string> options;
+
+    if (pos != string::npos)
+    {
+        string optionstr = uri.substr(pos + 1);
+        uri.erase(pos);
+
+        char buf[optionstr.size() + 1];
+        strcpy(buf, optionstr.c_str());
+        char* saved;
+        char* tok = strtok_r(buf, ",", &saved);
+
+        while (tok && *tok)
+        {
+            string opt(tok);
+            pos = opt.find("=");
+
+            if (pos != string::npos)
+            {
+                string key = opt.substr(0, pos - 1);
+                string value = opt.substr(pos + 1);
+                options[key] = value;
+            }
+            else
+            {
+                /** Invalid option */
+                return NULL;
+            }
+
+            tok = strtok_r(NULL, ",", &saved);
+        }
+    }
+
     pos = request_line.find("\r\n");
     string http_version = request_line.substr(0, pos);
     request_line.erase(0, pos + 2);
@@ -86,10 +138,18 @@ HttpRequest* HttpRequest::parse(string data)
             header_line.erase(0, pos + 1);
             headers[key] = mxs::trim(header_line);
         }
+        else
+        {
+            /** Invalid header */
+            return NULL;
+        }
     }
 
-    /** The headers are now processed and consumed. The message body is
+    /**
-     * the only thing left in the request string. */
+     * The headers are now processed and consumed. The message body is
+     * the only thing left in the request string and it should be a JSON object.
+     * Attempt to parse it and return an error if it fails.
+     */
 
     bool ok = false;
     HttpRequest* request = NULL;
@@ -120,11 +180,12 @@ HttpRequest* HttpRequest::parse(string data)
     if (ok)
     {
         request = new HttpRequest();
-        request->m_verb = verb_value;
-        request->m_resource = uri;
         request->m_headers = headers;
+        request->m_options = options;
         request->m_json.reset(body);
         request->m_json_string = data;
+        request->m_resource = uri;
+        request->m_verb = verb_value;
     }
 
     return request;

server/core/maxscale/httprequest.hh

@@ -74,12 +74,32 @@ public:
      *
      * @param header Header to get
      *
-     * @return String value or empty string if no header found
+     * @return Header value or empty string if the header was not found
      */
-    const string get_header(const string header)
+    string get_header(const string header) const
     {
         string rval;
-        map<string, string>::iterator it = m_headers.find(header);
+        map<string, string>::const_iterator it = m_headers.find(header);
+
+        if (it != m_headers.end())
+        {
+            rval = it->second;
+        }
+
+        return rval;
+    }
+
+    /**
+     * @brief Get option value
+     *
+     * @param header Option to get
+     *
+     * @return Option value or empty string if the option was not found
+     */
+    string get_option(const string option) const
+    {
+        string rval;
+        map<string, string>::const_iterator it = m_options.find(option);
 
         if (it != m_headers.end())
         {
@@ -125,6 +145,7 @@ private:
     HttpRequest& operator = (const HttpRequest&);
 
     map<string, string> m_headers;     /**< Request headers */
+    map<string, string> m_options;     /**< Request options */
     Closer<json_t*>     m_json;        /**< Request body */
     string              m_json_string; /**< String version of @c m_json */
     string              m_resource;    /**< Requested resource */

server/core/test/testhttp.cc

@@ -42,18 +42,22 @@ const char* verbs_fail[] =
     NULL
 };
 
-const char* paths_pass[] =
+static struct
 {
-    "/",
+    const char* input;
-    "*",
+    const char* output;
-    "/test/",
+} paths_pass[] =
-    "/test",
+{
-    "/servers/list",
+    { "/", "/" },
-    "/servers/list/",
+    { "*", "*" },
-    "/?test=true",
+    { "/test/", "/test/" },
-    "/test/?test=y",
+    { "/test", "/test" },
-    "/?",
+    { "/servers/list", "/servers/list" },
-    NULL
+    { "/servers/list/", "/servers/list/" },
+    { "/?test=true", "/" },
+    { "/test/?test=y", "/test/" },
+    { "/?", "/" },
+    {}
 };
 
 const char* paths_fail[] =
@@ -84,17 +88,17 @@ int test_basic()
     /** Test parts that should pass */
     for (int i = 0; verbs_pass[i]; i++)
     {
-        for (int j = 0; paths_pass[j]; j++)
+        for (int j = 0; paths_pass[j].input; j++)
         {
             for (int k = 0; proto_pass[k]; k++)
             {
                 stringstream ss;
-                ss << verbs_pass[i] << " " << paths_pass[j] << " " << proto_pass[k] << "\r\n\r\n";
+                ss << verbs_pass[i] << " " << paths_pass[j].input << " " << proto_pass[k] << "\r\n\r\n";
                 SHttpRequest parser(HttpRequest::parse(ss.str()));
                 TEST(parser.get() != NULL, "Valid HTTP request should be parsed: %s", ss.str().c_str());
-                TEST(parser->get_resource() == string(paths_pass[j]),
+                TEST(parser->get_resource() == string(paths_pass[j].output),
                      "The request path '%s' should be correct: %s",
-                     paths_pass[j], parser->get_resource().c_str());
+                     paths_pass[j].output, parser->get_resource().c_str());
             }
         }
     }
@@ -307,6 +311,63 @@ int test_message_body()
     return 0;
 }
 
+static struct
+{
+    const char* input;
+    const char* key;
+    const char* value;
+} options_pass[] =
+{
+    { "/", "", "" },
+    { "*", "", "" },
+    { "/?a=b", "a", "b" },
+    { "/?a=b,c=d", "a", "b" },
+    { "/?a=b,c=d", "c", "d" },
+    { "/test?q=w", "q", "w" },
+    { "/servers/list?all=false", "all", "false" },
+    { "/servers/list/?pretty=true", "pretty", "true"},
+    { "/?test=true", "test", "true" },
+    { "/test/?test=y", "test", "y" },
+    { "/?", "", "" },
+    {}
+};
+
+const char* options_fail[] =
+{
+    "/?,",
+    "/??",
+    "/test?/",
+    "/test/?a,b",
+    "/test?a,",
+    NULL
+};
+
+int test_options()
+{
+    for (int i = 0; options_pass[i].input; i++)
+    {
+        stringstream ss;
+        ss << "GET " << options_pass[i].input << " HTTP/1.1\r\n\r\n";
+        SHttpRequest parser(HttpRequest::parse(ss.str()));
+
+        TEST(parser.get() != NULL, "Valid option should be parsed: %s", ss.str().c_str());
+        TEST(parser->get_option(options_pass[i].key) == options_pass[i].value,
+             "The option value for '%s' should be '%s': %s",
+             options_pass[i].key, options_pass[i].value,
+             parser->get_option(options_pass[i].key).c_str());
+    }
+
+    for (int i = 0; options_fail[i]; i++)
+    {
+        stringstream ss;
+        ss << "GET " << options_fail[i] << " HTTP/1.1\r\n\r\n";
+        SHttpRequest parser(HttpRequest::parse(ss.str()));
+        TEST(parser.get() == NULL, "Invalid option should not be parsed: %s", ss.str().c_str());
+    }
+
+    return 0;
+}
+
 int test_response()
 {
     TEST(HttpResponse().get_response().find("200 OK") != string::npos,