Add IPv6 key item support to request parser (sonic-net#1449)

theasianpianist · web-flow · commit be51ebc533cb · 2020-09-28T15:54:16.000-07:00
**What I did**
Add support for parsing IPv6 key items in request parser when the key separator is `:` (e.g. in AppDB). Note: if a key contains an IPv6 address/prefix, it must be the last item in the key to be properly parsed.

**Why I did it**
Some components that use the request parser are being updated to support IPv6 key items (notably VnetOrch). These components also communicate with AppDB, which uses `:` as the delimiter for key items. Since IPv6 addresses are also separated with `:`, the original parsing logic will fragment an IPv6 address into multiple key items.

**How I verified it**
Added unit tests to verify correct parsing behavior for the following key formats:

* `&lt;string&gt;:&lt;valid ipv6 address or prefix&gt;`
* `&lt;string&gt;:&lt;invalid ipv6 address or prefix&gt;` (an exception should be thrown when the parser attempts to construct an `IpAddress` or `IpPrefix` object from the invalid key)
* `&lt;empty string&gt;:&lt;valid ipv6 address or prefix&gt;`
* `&lt;valid ipv6 address or prefix&gt;`

**Details if related**
The following 3 conditions must be met before request parser will attempt to parse an IPv6 address:
* The key separator being used is `:` (IPv6 addresses can already be parsed with other key separators)
* The number of parsed key items is greater than the number of expected key items (this indicates that the `:` within an IPv6 address may have been interpreted as a key separator).
* The last key item type is either an IP address or an IP prefix (in any current key formats that contain an IP address, the IP address is always the last member of the key)

Signed-off-by: Lawrence Lee &lt;lawlee@microsoft.com&gt;
diff --git a/orchagent/request_parser.cpp b/orchagent/request_parser.cpp
@@ -60,16 +60,47 @@ void Request::parseKey(const KeyOpFieldsValuesTuple& request)
 
     // split the key by separator
     std::vector<std::string> key_items;
-    size_t f_position = 0;
-    size_t e_position = full_key_.find(key_separator_);
-    while (e_position != std::string::npos)
+    size_t key_item_start = 0;
+    size_t key_item_end = full_key_.find(key_separator_);
+    while (key_item_end != std::string::npos)
     {
-        key_items.push_back(full_key_.substr(f_position, e_position - f_position));
-        f_position = e_position + 1;
-        e_position = full_key_.find(key_separator_, f_position);
+        key_items.push_back(full_key_.substr(key_item_start, key_item_end - key_item_start));
+        key_item_start = key_item_end + 1;
+        key_item_end = full_key_.find(key_separator_, key_item_start);
     }
-    key_items.push_back(full_key_.substr(f_position, full_key_.length()));
+    key_items.push_back(full_key_.substr(key_item_start, full_key_.length()));
+
+    /*
+     * Attempt to parse an IPv6 address only if the following conditions are met:
+     * - The key separator is ":" 
+     *     - The above logic will already correctly parse IPv6 addresses using other key separators
+     *     - Special consideration is only needed for ":" key separators since IPv6 addresses also use ":" as the field separator
+     * - The number of parsed key items exceeds the number of expected key items
+     *     - If we have too many key items and the last key item is supposed to be an IP or prefix, there is a chance that it was an 
+     *       IPv6 address that got segmented during parsing
+     *     - The above logic will always break an IPv6 address into at least 2 parts, so if an IPv6 address has been parsed incorrectly it will
+     *       always increase the number of key items
+     * - The last key item is an IP address or prefix
+     *     - This runs under the assumption that an IPv6 address, if present, will always be the last key item
+     */
+    if (key_separator_ == ':' and 
+        key_items.size() > number_of_key_items_ and 
+        (request_description_.key_item_types.back() == REQ_T_IP or request_description_.key_item_types.back() == REQ_T_IP_PREFIX))
+    {
+        // Remove key_items so that key_items.size() is correct, then assemble the removed items into an IPv6 address
+        std::vector<std::string> ip_addr_groups(--key_items.begin() + number_of_key_items_, key_items.end());
+        key_items.erase(--key_items.begin() + number_of_key_items_, key_items.end());
+
+        std::string ip_string;
+
+        for (const auto &i : ip_addr_groups)
+        {
+            ip_string += i + ":";
+        }
+        ip_string.pop_back(); // remove extra ":" from end of string
 
+        key_items.push_back(ip_string);
+    }
     if (key_items.size() != number_of_key_items_)
     {
         throw std::invalid_argument(std::string("Wrong number of key items. Expected ")
diff --git a/tests/request_parser_ut.cpp b/tests/request_parser_ut.cpp
@@ -1298,3 +1298,296 @@ TEST(request_parser, wrong_key_ip_prefix)
         FAIL() << "Expected std::logic_error, not other exception";
     }
 }
+
+const request_description_t request_description_ipv6_addr = {
+    { REQ_T_STRING, REQ_T_IP },
+    { },
+    { }  // no mandatory attributes
+};
+
+class TestRequestIpv6Addr : public Request
+{
+public:
+    TestRequestIpv6Addr() : Request(request_description_ipv6_addr, ':') { }
+};
+
+const request_description_t request_description_ipv6_prefix = {
+    { REQ_T_STRING, REQ_T_IP_PREFIX },
+    { },
+    { }  // no mandatory attributes
+};
+
+class TestRequestIpv6Prefix : public Request
+{
+public:
+    TestRequestIpv6Prefix() : Request(request_description_ipv6_prefix, ':') { }
+};
+
+const request_description_t request_desc_ipv6_addr_only = {
+    { REQ_T_IP },
+    { },
+    { }
+};
+
+class TestRequestIpv6AddrOnly: public Request
+{
+public:
+    TestRequestIpv6AddrOnly() : Request(request_desc_ipv6_addr_only, ':') { }
+};
+
+const request_description_t request_desc_ipv6_prefix_only = {
+    { REQ_T_IP_PREFIX },
+    { },
+    { }
+};
+
+class TestRequestIpv6PrefixOnly: public Request
+{
+public:
+    TestRequestIpv6PrefixOnly() : Request(request_desc_ipv6_prefix_only, ':') { }
+};
+
+std::vector<std::string> ipv6_addresses = {"2001:db8:3c4d:0015:0000:0000:1a2f:1a2b", "2001:db8:3c4d:0015::1a2f:1a2b", "::2001:db8:3c4d:0015:1a2f:1a2b", "2001:db8:3c4d:0015:1a2f:1a2b::", "::"};
+std::vector<std::string> ipv6_addresses_invalid = {"2001:db8:0015:0000:1a2f:1a2b", "5552001:db8:3c4d:0015::1a2f:1a2b", "::2001:zdb8:3c4d:0015:1a2f:1a2b", "2001:db8:3c4d:0015::::1a2f:1aeer2b::"};
+std::vector<std::string> ipv6_prefixes = {"2001:db8:3c4d:0015:0000:0000:1a2f:1a2b/16", "2001:db8:3c4d:0015::1a2f:1a2b/32", "::2001:db8:3c4d:0015:1a2f:1a2b/24", "2001:db8:3c4d:0015:1a2f:1a2b::/8", "::/16"};
+std::vector<std::string> ipv6_prefixes_invalid = {"2001:db8:0015:0000:1a2f:1a2b/16", "5552001:db8:3c4d:0015::1a2f:1a2b/32", "::2001:zdb8:3c4d:0015:1a2f:1a2b/24", "2001:db8:3c4d:0015::::1a2f:1aeer2b::/8"};
+
+TEST(request_parser, ipv6_addr_key_item)
+{
+    for (const std::string &ipv6_addr : ipv6_addresses)
+    {
+        std::string key_string = "key1:" + ipv6_addr;
+        KeyOpFieldsValuesTuple t {key_string, "SET",
+                                    { }
+                                };
+
+        try
+        {
+            TestRequestIpv6Addr request;
+
+            EXPECT_NO_THROW(request.parse(t));
+
+            EXPECT_STREQ(request.getOperation().c_str(), "SET");
+            EXPECT_STREQ(request.getFullKey().c_str(), key_string.c_str());
+            EXPECT_STREQ(request.getKeyString(0).c_str(), "key1");
+            EXPECT_EQ(request.getKeyIpAddress(1), IpAddress(ipv6_addr));
+            EXPECT_FALSE(request.getKeyIpAddress(1).isV4());
+        }
+        catch (const std::exception& e)
+        {
+            FAIL() << "Got unexpected exception " << e.what();
+        }
+        catch (...)
+        {
+            FAIL() << "Got unexpected exception";
+        }
+    }
+}
+
+TEST(request_parser, ipv6_addr_key_item_empty_str)
+{
+    for (const std::string &ipv6_addr : ipv6_addresses)
+    {
+        std::string key_string = ":" + ipv6_addr;
+        KeyOpFieldsValuesTuple t {key_string, "SET",
+                                    { }
+                                };
+
+        try
+        {
+            TestRequestIpv6Addr request;
+
+            EXPECT_NO_THROW(request.parse(t));
+
+            EXPECT_STREQ(request.getOperation().c_str(), "SET");
+            EXPECT_STREQ(request.getFullKey().c_str(), key_string.c_str());
+            EXPECT_STREQ(request.getKeyString(0).c_str(), "");
+            EXPECT_EQ(request.getKeyIpAddress(1), IpAddress(ipv6_addr));
+            EXPECT_FALSE(request.getKeyIpAddress(1).isV4());
+        }
+        catch (const std::exception& e)
+        {
+            FAIL() << "Got unexpected exception " << e.what();
+        }
+        catch (...)
+        {
+            FAIL() << "Got unexpected exception";
+        }
+    }
+}
+
+TEST(request_parser, ipv6_addr_key_item_only)
+{
+    for (const std::string &ipv6_addr : ipv6_addresses)
+    {
+        KeyOpFieldsValuesTuple t {ipv6_addr, "SET",
+                                    { }
+                                };
+
+        try
+        {
+            TestRequestIpv6AddrOnly request;
+
+            EXPECT_NO_THROW(request.parse(t));
+
+            EXPECT_STREQ(request.getOperation().c_str(), "SET");
+            EXPECT_STREQ(request.getFullKey().c_str(), ipv6_addr.c_str());
+            EXPECT_EQ(request.getKeyIpAddress(0), IpAddress(ipv6_addr));
+            EXPECT_FALSE(request.getKeyIpAddress(0).isV4());
+        }
+        catch (const std::exception& e)
+        {
+            FAIL() << "Got unexpected exception " << e.what();
+        }
+        catch (...)
+        {
+            FAIL() << "Got unexpected exception";
+        }
+    }
+}
+
+TEST(request_parser, ipv6_prefix_key_item)
+{
+    for (const std::string &ipv6_prefix : ipv6_prefixes)
+    {
+        std::string key_string = "key1:" + ipv6_prefix;
+        KeyOpFieldsValuesTuple t {key_string, "SET",
+                                    { }
+                                };
+
+        try
+        {
+            TestRequestIpv6Prefix request;
+
+            EXPECT_NO_THROW(request.parse(t));
+
+            EXPECT_STREQ(request.getOperation().c_str(), "SET");
+            EXPECT_STREQ(request.getFullKey().c_str(), key_string.c_str());
+            EXPECT_STREQ(request.getKeyString(0).c_str(), "key1");
+            EXPECT_EQ(request.getKeyIpPrefix(1), IpPrefix(ipv6_prefix));
+            EXPECT_FALSE(request.getKeyIpPrefix(1).isV4());
+        }
+        catch (const std::exception& e)
+        {
+            FAIL() << "Got unexpected exception " << e.what();
+        }
+        catch (...)
+        {
+            FAIL() << "Got unexpected exception";
+        }
+    }
+}
+
+TEST(request_parser, ipv6_prefix_key_item_empty_str)
+{
+    for (const std::string &ipv6_prefix: ipv6_prefixes)
+    {
+        std::string key_string = ":" + ipv6_prefix;
+        KeyOpFieldsValuesTuple t {key_string, "SET",
+                                    { }
+                                };
+
+        try
+        {
+            TestRequestIpv6Prefix request;
+
+            EXPECT_NO_THROW(request.parse(t));
+
+            EXPECT_STREQ(request.getOperation().c_str(), "SET");
+            EXPECT_STREQ(request.getFullKey().c_str(), key_string.c_str());
+            EXPECT_STREQ(request.getKeyString(0).c_str(), "");
+            EXPECT_EQ(request.getKeyIpPrefix(1), IpPrefix(ipv6_prefix));
+            EXPECT_FALSE(request.getKeyIpPrefix(1).isV4());
+        }
+        catch (const std::exception& e)
+        {
+            FAIL() << "Got unexpected exception " << e.what();
+        }
+        catch (...)
+        {
+            FAIL() << "Got unexpected exception";
+        }
+    }
+}
+
+TEST(request_parser, ipv6_prefix_key_item_only)
+{
+    for (const std::string &ipv6_prefix : ipv6_prefixes)
+    {
+        KeyOpFieldsValuesTuple t {ipv6_prefix, "SET",
+                                    { }
+                                };
+
+        try
+        {
+            TestRequestIpv6PrefixOnly request;
+
+            EXPECT_NO_THROW(request.parse(t));
+
+            EXPECT_STREQ(request.getOperation().c_str(), "SET");
+            EXPECT_STREQ(request.getFullKey().c_str(), ipv6_prefix.c_str());
+            EXPECT_EQ(request.getKeyIpPrefix(0), IpPrefix(ipv6_prefix));
+            EXPECT_FALSE(request.getKeyIpPrefix(0).isV4());
+        }
+        catch (const std::exception& e)
+        {
+            FAIL() << "Got unexpected exception " << e.what();
+        }
+        catch (...)
+        {
+            FAIL() << "Got unexpected exception";
+        }
+    }
+}
+
+TEST(request_parser, invalid_ipv6_prefix_key_item)
+{
+    for (const std::string &ipv6_prefix : ipv6_prefixes_invalid)
+    {
+        std::string key_string = "key1:" + ipv6_prefix;
+        KeyOpFieldsValuesTuple t {key_string, "SET",
+                                    { }
+                                };
+
+        try
+        {
+            TestRequestIpv6Prefix request;
+
+            EXPECT_THROW(request.parse(t), std::invalid_argument);
+        }
+        catch (const std::exception& e)
+        {
+            FAIL() << "Got unexpected exception " << e.what();
+        }
+        catch (...)
+        {
+            FAIL() << "Got unexpected exception";
+        }
+    }
+}
+
+TEST(request_parser, invalid_ipv6_addr_key_item)
+{
+    for (const std::string &ipv6_addr : ipv6_addresses_invalid)
+    {
+        std::string key_string = "key1:" + ipv6_addr;
+        KeyOpFieldsValuesTuple t {key_string, "SET",
+                                    { }
+                                };
+
+        try
+        {
+            TestRequestIpv6Addr request;
+
+            EXPECT_THROW(request.parse(t), std::invalid_argument);
+        }
+        catch (const std::exception& e)
+        {
+            FAIL() << "Got unexpected exception " << e.what();
+        }
+        catch (...)
+        {
+            FAIL() << "Got unexpected exception";
+        }
+    }
+}
