From ae575b0c0838f73ef666270260c3c0385d097e78 Mon Sep 17 00:00:00 2001 From: Anton Kolesnyk <41349689+antkmsft@users.noreply.github.com> Date: Fri, 26 Feb 2021 08:38:01 -0800 Subject: [PATCH] CaseInsensitiveMap (#1731) * Add CaseInsensitiveMap * Drop unneccessary namespace qualification * Comment working: more accurate description * GCC and Clang fix (typename specification) + doxy comments for template parameters * Remove Allocator template parameter - we can add it later if we need it, currently no need to commit to having it there * Drop template parameter. We can add it later with default value of std::string without breaking change * Unit test Co-authored-by: Anton Kolesnyk --- sdk/core/azure-core/CHANGELOG.md | 3 +- sdk/core/azure-core/CMakeLists.txt | 1 + .../inc/azure/core/case_insensitive_map.hpp | 25 ++++++++++++++ .../azure-core/inc/azure/core/http/http.hpp | 25 +++++++------- .../azure-core/inc/azure/core/http/policy.hpp | 3 +- .../inc/azure/core/internal/strings.hpp | 12 +++++++ sdk/core/azure-core/src/http/http.cpp | 2 +- sdk/core/azure-core/src/http/raw_response.cpp | 5 +-- sdk/core/azure-core/src/http/request.cpp | 8 ++--- sdk/core/azure-core/test/ut/CMakeLists.txt | 1 + .../test/ut/case_insensitive_map.cpp | 33 +++++++++++++++++++ sdk/core/azure-core/test/ut/http.cpp | 21 ++++-------- .../src/keyvault_exception.cpp | 2 +- .../azure/storage/common/storage_common.hpp | 15 ++------- 14 files changed, 105 insertions(+), 51 deletions(-) create mode 100644 sdk/core/azure-core/inc/azure/core/case_insensitive_map.hpp create mode 100644 sdk/core/azure-core/test/ut/case_insensitive_map.cpp diff --git a/sdk/core/azure-core/CHANGELOG.md b/sdk/core/azure-core/CHANGELOG.md index 9f79fcafb..d9638cbb5 100644 --- a/sdk/core/azure-core/CHANGELOG.md +++ b/sdk/core/azure-core/CHANGELOG.md @@ -9,9 +9,10 @@ - Removed `TransportKind` enum from `Azure::Core::Http`. - Renamed `NoRevoke` to `EnableCertificateRevocationListCheck` for `Azure::Core::Http::CurlTransportSSLOptions`. - Renamed `GetString()` to `ToString()` in `Azure::Core::DateTime`. -- Renamed `GetUuidString()` tp `ToString()` in `Azure::Core::Uuid`. +- Renamed `GetUuidString()` to `ToString()` in `Azure::Core::Uuid`. - Moved `NullBodyStream` to internal usage only. It is not meant for public use. - Removed `LimitBodyStream`. +- Introduced `Azure::Core::CaseInsensitiveMap` which is now used to store headers in `Azure::Core::Http::Request` and `Azure::Core::Http::RawResponse`. ### Bug Fixes diff --git a/sdk/core/azure-core/CMakeLists.txt b/sdk/core/azure-core/CMakeLists.txt index 225296417..b8275103d 100644 --- a/sdk/core/azure-core/CMakeLists.txt +++ b/sdk/core/azure-core/CMakeLists.txt @@ -60,6 +60,7 @@ set( inc/azure/core/internal/strings.hpp inc/azure/core/logging/logging.hpp inc/azure/core/base64.hpp + inc/azure/core/case_insensitive_map.hpp inc/azure/core/context.hpp inc/azure/core/credentials.hpp inc/azure/core/datetime.hpp diff --git a/sdk/core/azure-core/inc/azure/core/case_insensitive_map.hpp b/sdk/core/azure-core/inc/azure/core/case_insensitive_map.hpp new file mode 100644 index 000000000..dc1119ba7 --- /dev/null +++ b/sdk/core/azure-core/inc/azure/core/case_insensitive_map.hpp @@ -0,0 +1,25 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// SPDX-License-Identifier: MIT + +/** + * @file + * @brief A `map` with case-insensitive key comparison. + */ + +#pragma once + +#include "azure/core/internal/strings.hpp" + +#include +#include + +namespace Azure { namespace Core { + + /** + * @brief A type alias of `std::map` with case-insensitive key + * comparison. + */ + using CaseInsensitiveMap + = std::map; + +}} // namespace Azure::Core diff --git a/sdk/core/azure-core/inc/azure/core/http/http.hpp b/sdk/core/azure-core/inc/azure/core/http/http.hpp index 4ab0edc04..31e1d2cbd 100644 --- a/sdk/core/azure-core/inc/azure/core/http/http.hpp +++ b/sdk/core/azure-core/inc/azure/core/http/http.hpp @@ -8,6 +8,7 @@ #pragma once +#include "azure/core/case_insensitive_map.hpp" #include "azure/core/exception.hpp" #include "azure/core/http/body_stream.hpp" #include "azure/core/internal/contract.hpp" @@ -45,7 +46,7 @@ namespace Azure { namespace Core { namespace Http { * @throw if \p headerName is invalid. */ void InsertHeaderWithValidation( - std::map& headers, + CaseInsensitiveMap& headers, std::string const& headerName, std::string const& headerValue); } // namespace Details @@ -409,11 +410,11 @@ namespace Azure { namespace Core { namespace Http { uint16_t GetPort() const { return m_port; } /** - * @brief Provides a copy to the list of query parameters from the URL. + * @brief Get a copy of the list of query parameters from the URL. * * @remark The query parameters are URL-encoded. * - * @return const std::map& + * @return A copy of the query parameters map. */ std::map GetQueryParameters() const { @@ -421,16 +422,16 @@ namespace Azure { namespace Core { namespace Http { } /** - * @brief Gets the path and query parameters. + * @brief Get the path and query parameters. * - * @return std::string The string is URL encoded. + * @return Relative URL with URL-encoded query parameters. */ std::string GetRelativeUrl() const; /** - * @brief Gets Scheme, host, path and query parameters. + * @brief Get Scheme, host, path and query parameters. * - * @return std::string The string is URL encoded. + * @return Absolute URL with URL-encoded query parameters. */ std::string GetAbsoluteUrl() const; }; @@ -449,8 +450,8 @@ namespace Azure { namespace Core { namespace Http { private: HttpMethod m_method; Url m_url; - std::map m_headers; - std::map m_retryHeaders; + CaseInsensitiveMap m_headers; + CaseInsensitiveMap m_retryHeaders; BodyStream* m_bodyStream; @@ -542,7 +543,7 @@ namespace Azure { namespace Core { namespace Http { /** * @brief Get HTTP headers. */ - std::map GetHeaders() const; + CaseInsensitiveMap GetHeaders() const; /** * @brief Get HTTP body as #Azure::Core::Http::BodyStream. @@ -593,7 +594,7 @@ namespace Azure { namespace Core { namespace Http { int32_t m_minorVersion; HttpStatusCode m_statusCode; std::string m_reasonPhrase; - std::map m_headers; + CaseInsensitiveMap m_headers; std::unique_ptr m_bodyStream; std::vector m_body; @@ -727,7 +728,7 @@ namespace Azure { namespace Core { namespace Http { /** * @brief Get HTTP response headers. */ - std::map const& GetHeaders() const; + CaseInsensitiveMap const& GetHeaders() const; /** * @brief Get HTTP response body as #Azure::Core::Http::BodyStream. diff --git a/sdk/core/azure-core/inc/azure/core/http/policy.hpp b/sdk/core/azure-core/inc/azure/core/http/policy.hpp index f3a4dc28d..cab0c880d 100644 --- a/sdk/core/azure-core/inc/azure/core/http/policy.hpp +++ b/sdk/core/azure-core/inc/azure/core/http/policy.hpp @@ -8,6 +8,7 @@ #pragma once +#include "azure/core/case_insensitive_map.hpp" #include "azure/core/context.hpp" #include "azure/core/credentials.hpp" #include "azure/core/http/http.hpp" @@ -388,7 +389,7 @@ namespace Azure { namespace Core { namespace Http { */ struct ValuePolicyOptions { - std::map HeaderValues; + CaseInsensitiveMap HeaderValues; std::map QueryValues; }; diff --git a/sdk/core/azure-core/inc/azure/core/internal/strings.hpp b/sdk/core/azure-core/inc/azure/core/internal/strings.hpp index 85e697b90..012dce125 100644 --- a/sdk/core/azure-core/inc/azure/core/internal/strings.hpp +++ b/sdk/core/azure-core/inc/azure/core/internal/strings.hpp @@ -8,6 +8,7 @@ */ #pragma once +#include #include namespace Azure { namespace Core { namespace Internal { namespace Strings { @@ -16,4 +17,15 @@ namespace Azure { namespace Core { namespace Internal { namespace Strings { std::string const ToLower(const std::string& src) noexcept; unsigned char ToLower(const unsigned char src) noexcept; + struct CaseInsensitiveComparator + { + bool operator()(const std::string& lhs, const std::string& rhs) const + { + return std::lexicographical_compare( + lhs.begin(), lhs.end(), rhs.begin(), rhs.end(), [](char c1, char c2) { + return ToLower(c1) < ToLower(c2); + }); + } + }; + }}}} // namespace Azure::Core::Internal::Strings diff --git a/sdk/core/azure-core/src/http/http.cpp b/sdk/core/azure-core/src/http/http.cpp index f08c418f8..2077b59ea 100644 --- a/sdk/core/azure-core/src/http/http.cpp +++ b/sdk/core/azure-core/src/http/http.cpp @@ -10,7 +10,7 @@ using namespace Azure::Core::Http; using namespace Azure::Core::Internal::Http; void Azure::Core::Http::Details::InsertHeaderWithValidation( - std::map& headers, + Azure::Core::CaseInsensitiveMap& headers, std::string const& headerName, std::string const& headerValue) { diff --git a/sdk/core/azure-core/src/http/raw_response.cpp b/sdk/core/azure-core/src/http/raw_response.cpp index 6ab0e6d09..9fd607830 100644 --- a/sdk/core/azure-core/src/http/raw_response.cpp +++ b/sdk/core/azure-core/src/http/raw_response.cpp @@ -15,10 +15,7 @@ HttpStatusCode RawResponse::GetStatusCode() const { return m_statusCode; } std::string const& RawResponse::GetReasonPhrase() const { return m_reasonPhrase; } -std::map const& RawResponse::GetHeaders() const -{ - return this->m_headers; -} +Azure::Core::CaseInsensitiveMap const& RawResponse::GetHeaders() const { return this->m_headers; } void RawResponse::AddHeader(uint8_t const* const first, uint8_t const* const last) { diff --git a/sdk/core/azure-core/src/http/request.cpp b/sdk/core/azure-core/src/http/request.cpp index f539a03f2..79f950b9f 100644 --- a/sdk/core/azure-core/src/http/request.cpp +++ b/sdk/core/azure-core/src/http/request.cpp @@ -13,9 +13,9 @@ using namespace Azure::Core::Http; namespace { // returns left map plus all items in right // when duplicates, left items are preferred -static std::map MergeMaps( - std::map left, - std::map const& right) +static Azure::Core::CaseInsensitiveMap MergeMaps( + Azure::Core::CaseInsensitiveMap left, + Azure::Core::CaseInsensitiveMap const& right) { left.insert(right.begin(), right.end()); return left; @@ -51,7 +51,7 @@ void Request::StartTry() HttpMethod Request::GetMethod() const { return this->m_method; } -std::map Request::GetHeaders() const +Azure::Core::CaseInsensitiveMap Request::GetHeaders() const { // create map with retry headers which are the most important and we don't want // to override them with any duplicate header diff --git a/sdk/core/azure-core/test/ut/CMakeLists.txt b/sdk/core/azure-core/test/ut/CMakeLists.txt index a707121ef..8c966b6ca 100644 --- a/sdk/core/azure-core/test/ut/CMakeLists.txt +++ b/sdk/core/azure-core/test/ut/CMakeLists.txt @@ -31,6 +31,7 @@ add_executable ( azure-core-test base64.cpp bodystream.cpp + case_insensitive_map.cpp context.cpp ${CURL_CONNECTION_POOL_TESTS} ${CURL_OPTIONS_TESTS} diff --git a/sdk/core/azure-core/test/ut/case_insensitive_map.cpp b/sdk/core/azure-core/test/ut/case_insensitive_map.cpp new file mode 100644 index 000000000..797895e41 --- /dev/null +++ b/sdk/core/azure-core/test/ut/case_insensitive_map.cpp @@ -0,0 +1,33 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// SPDX-License-Identifier: MIT + +#include + +#include + +using namespace Azure::Core; + +TEST(CaseInsensitiveMap, Find) +{ + CaseInsensitiveMap imap; + + imap["Content-Length"] = "X"; + auto pos = imap.find("content-length"); + + EXPECT_NE(pos, imap.end()); + EXPECT_EQ(pos->second, "X"); + EXPECT_EQ(pos->first, "Content-Length"); +} + +TEST(CaseInsensitiveMap, Modify) +{ + CaseInsensitiveMap imap; + + imap["Content-Length"] = "X"; + imap["content-length"] = "Y"; + auto pos = imap.find("CONTENT-LENGTH"); + + EXPECT_NE(pos, imap.end()); + EXPECT_EQ(pos->second, "Y"); + EXPECT_EQ(pos->first, "Content-Length"); +} diff --git a/sdk/core/azure-core/test/ut/http.cpp b/sdk/core/azure-core/test/ut/http.cpp index a4d959cb4..9d886a1b1 100644 --- a/sdk/core/azure-core/test/ut/http.cpp +++ b/sdk/core/azure-core/test/ut/http.cpp @@ -25,8 +25,7 @@ namespace Azure { namespace Core { namespace Test { EXPECT_NO_THROW(req.AddHeader(expected.first, expected.second)); EXPECT_PRED2( - [](std::map headers, - std::pair expected) { + [](Azure::Core::CaseInsensitiveMap headers, std::pair expected) { auto firstHeader = headers.begin(); return firstHeader->first == expected.first && firstHeader->second == expected.second && headers.size() == 1; @@ -40,8 +39,7 @@ namespace Azure { namespace Core { namespace Test { std::pair expectedOverride("valid", "override"); EXPECT_NO_THROW(req.AddHeader(expectedOverride.first, expectedOverride.second)); EXPECT_PRED2( - [](std::map headers, - std::pair expected) { + [](Azure::Core::CaseInsensitiveMap headers, std::pair expected) { auto firstHeader = headers.begin(); return firstHeader->first == expected.first && firstHeader->second == expected.second && headers.size() == 1; @@ -53,8 +51,7 @@ namespace Azure { namespace Core { namespace Test { std::pair expected2("valid2", "header2"); EXPECT_NO_THROW(req.AddHeader(expected2.first, expected2.second)); EXPECT_PRED2( - [](std::map headers, - std::pair expected) { + [](Azure::Core::CaseInsensitiveMap headers, std::pair expected) { auto secondHeader = headers.begin(); secondHeader++; return secondHeader->first == expected.first && secondHeader->second == expected.second @@ -73,8 +70,7 @@ namespace Azure { namespace Core { namespace Test { EXPECT_NO_THROW(response.AddHeader(expected.first, expected.second)); EXPECT_PRED2( - [](std::map headers, - std::pair expected) { + [](Azure::Core::CaseInsensitiveMap headers, std::pair expected) { auto firstHeader = headers.begin(); return firstHeader->first == expected.first && firstHeader->second == expected.second && headers.size() == 1; @@ -89,8 +85,7 @@ namespace Azure { namespace Core { namespace Test { std::pair expectedOverride("valid", "override"); EXPECT_NO_THROW(response.AddHeader(expectedOverride.first, expectedOverride.second)); EXPECT_PRED2( - [](std::map headers, - std::pair expected) { + [](Azure::Core::CaseInsensitiveMap headers, std::pair expected) { auto firstHeader = headers.begin(); return firstHeader->first == expected.first && firstHeader->second == expected.second && headers.size() == 1; @@ -102,8 +97,7 @@ namespace Azure { namespace Core { namespace Test { std::pair expected2("valid2", "header2"); EXPECT_NO_THROW(response.AddHeader(expected2.first, expected2.second)); EXPECT_PRED2( - [](std::map headers, - std::pair expected) { + [](Azure::Core::CaseInsensitiveMap headers, std::pair expected) { auto secondtHeader = headers.begin(); secondtHeader++; return secondtHeader->first == expected.first && secondtHeader->second == expected.second @@ -120,8 +114,7 @@ namespace Azure { namespace Core { namespace Test { // adding header after previous error just happened on add from string EXPECT_NO_THROW(response.AddHeader("valid3: header3")); EXPECT_PRED2( - [](std::map headers, - std::pair expected) { + [](Azure::Core::CaseInsensitiveMap headers, std::pair expected) { auto secondtHeader = headers.begin(); secondtHeader++; secondtHeader++; diff --git a/sdk/keyvault/azure-security-keyvault-common/src/keyvault_exception.cpp b/sdk/keyvault/azure-security-keyvault-common/src/keyvault_exception.cpp index 97aba2645..a11d90f65 100644 --- a/sdk/keyvault/azure-security-keyvault-common/src/keyvault_exception.cpp +++ b/sdk/keyvault/azure-security-keyvault-common/src/keyvault_exception.cpp @@ -14,7 +14,7 @@ using namespace Azure::Security::KeyVault::Common; namespace { inline std::string GetHeaderOrEmptyString( - std::map const& headers, + Azure::Core::CaseInsensitiveMap const& headers, std::string const& headerName) { auto header = headers.find(headerName); diff --git a/sdk/storage/azure-storage-common/inc/azure/storage/common/storage_common.hpp b/sdk/storage/azure-storage-common/inc/azure/storage/common/storage_common.hpp index 4b7846a79..439fdfcc5 100644 --- a/sdk/storage/azure-storage-common/inc/azure/storage/common/storage_common.hpp +++ b/sdk/storage/azure-storage-common/inc/azure/storage/common/storage_common.hpp @@ -10,8 +10,8 @@ #include #include +#include #include -#include #include "azure/storage/common/constants.hpp" #include "azure/storage/common/storage_per_retry_policy.hpp" @@ -52,21 +52,10 @@ namespace Azure { namespace Storage { }; namespace Details { - struct CaseInsensitiveComparator - { - bool operator()(const std::string& lhs, const std::string& rhs) const - { - return std::lexicographical_compare( - lhs.begin(), lhs.end(), rhs.begin(), rhs.end(), [](char c1, char c2) { - return Core::Internal::Strings::ToLower(c1) < Core::Internal::Strings::ToLower(c2); - }); - } - }; - ContentHash FromBase64String(const std::string& base64String, HashAlgorithm algorithm); std::string ToBase64String(const ContentHash& hash); } // namespace Details - using Metadata = std::map; + using Metadata = Azure::Core::CaseInsensitiveMap; namespace Details {