From be51ebc533cbe92a885c584b4611ffc92d10ea36 Mon Sep 17 00:00:00 2001 From: Lawrence Lee Date: Mon, 28 Sep 2020 15:54:16 -0700 Subject: [PATCH] Add IPv6 key item support to request parser (#1449) **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: * `:` * `:` (an exception should be thrown when the parser attempts to construct an `IpAddress` or `IpPrefix` object from the invalid key) * `:` * `` **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 --- orchagent/request_parser.cpp | 45 +++++- tests/request_parser_ut.cpp | 293 +++++++++++++++++++++++++++++++++++ 2 files changed, 331 insertions(+), 7 deletions(-) diff --git a/orchagent/request_parser.cpp b/orchagent/request_parser.cpp index 59676a01f34d..a08777ad1fb5 100644 --- 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 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 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 index 46f404ff8c55..6f6c822731e8 100644 --- 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 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 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 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 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"; + } + } +} \ No newline at end of file