From dfe9a2be1f09f932ce0e36ae13c660cbad5f88a5 Mon Sep 17 00:00:00 2001 From: Anton Kolesnyk <41349689+antkmsft@users.noreply.github.com> Date: Wed, 15 Jun 2022 11:18:23 -0700 Subject: [PATCH] InputSanitizer: rename to HttpSanitizer, remove static member (#3736) * InputSanitizer => HttpSanitizer, remove static * Update cpp * Clang format Co-authored-by: Anton Kolesnyk --- .../test/ut/service_support_test.cpp | 2 +- sdk/core/azure-core/CMakeLists.txt | 4 +- .../inc/azure/core/http/policies/policy.hpp | 15 ++-- .../http_sanitizer.hpp} | 13 ++- .../inc/azure/core/internal/http/pipeline.hpp | 6 +- .../azure-core/src/http/http_sanitizer.cpp | 85 ++++++++++++++++++ sdk/core/azure-core/src/http/log_policy.cpp | 19 ++-- .../src/http/request_activity_policy.cpp | 5 +- .../src/private/input_sanitizer.cpp | 89 ------------------- .../test/ut/request_activity_policy_test.cpp | 6 +- 10 files changed, 118 insertions(+), 126 deletions(-) rename sdk/core/azure-core/inc/azure/core/internal/{input_sanitizer.hpp => http/http_sanitizer.hpp} (83%) create mode 100644 sdk/core/azure-core/src/http/http_sanitizer.cpp delete mode 100644 sdk/core/azure-core/src/private/input_sanitizer.cpp diff --git a/sdk/core/azure-core-tracing-opentelemetry/test/ut/service_support_test.cpp b/sdk/core/azure-core-tracing-opentelemetry/test/ut/service_support_test.cpp index b3c173821..580e623ae 100644 --- a/sdk/core/azure-core-tracing-opentelemetry/test/ut/service_support_test.cpp +++ b/sdk/core/azure-core-tracing-opentelemetry/test/ut/service_support_test.cpp @@ -595,7 +595,7 @@ public: // Add the request ID policy - this adds the x-ms-request-id attribute to the pipeline. policies.emplace_back( - std::make_unique(Azure::Core::_internal::InputSanitizer{})); + std::make_unique(Azure::Core::Http::_internal::HttpSanitizer{})); // Final policy - functions as the HTTP transport policy. policies.emplace_back(std::make_unique([&](Request& request) { diff --git a/sdk/core/azure-core/CMakeLists.txt b/sdk/core/azure-core/CMakeLists.txt index 931dace33..3dfc4aab4 100644 --- a/sdk/core/azure-core/CMakeLists.txt +++ b/sdk/core/azure-core/CMakeLists.txt @@ -80,6 +80,7 @@ set( inc/azure/core/internal/diagnostics/log.hpp inc/azure/core/internal/environment.hpp inc/azure/core/internal/extendable_enumeration.hpp + inc/azure/core/internal/http/http_sanitizer.hpp inc/azure/core/internal/http/pipeline.hpp inc/azure/core/internal/http/user_agent.hpp inc/azure/core/internal/io/null_body_stream.hpp @@ -88,7 +89,6 @@ set( inc/azure/core/internal/json/json_serializable.hpp inc/azure/core/internal/strings.hpp inc/azure/core/internal/tracing/service_tracing.hpp - inc/azure/core/internal/input_sanitizer.hpp inc/azure/core/io/body_stream.hpp inc/azure/core/match_conditions.hpp inc/azure/core/modified_conditions.hpp @@ -120,6 +120,7 @@ set( src/exception.cpp src/http/bearer_token_authentication_policy.cpp src/http/http.cpp + src/http/http_sanitizer.cpp src/http/log_policy.cpp src/http/policy.cpp src/http/raw_response.cpp @@ -136,7 +137,6 @@ set( src/operation_status.cpp src/private/environment_log_level_listener.hpp src/private/package_version.hpp - src/private/input_sanitizer.cpp src/strings.cpp src/tracing/tracing.cpp src/uuid.cpp diff --git a/sdk/core/azure-core/inc/azure/core/http/policies/policy.hpp b/sdk/core/azure-core/inc/azure/core/http/policies/policy.hpp index 46fd7812e..5dcb103e7 100644 --- a/sdk/core/azure-core/inc/azure/core/http/policies/policy.hpp +++ b/sdk/core/azure-core/inc/azure/core/http/policies/policy.hpp @@ -14,8 +14,8 @@ #include "azure/core/dll_import_export.hpp" #include "azure/core/http/http.hpp" #include "azure/core/http/transport.hpp" +#include "azure/core/internal/http/http_sanitizer.hpp" #include "azure/core/internal/http/user_agent.hpp" -#include "azure/core/internal/input_sanitizer.hpp" #include "azure/core/uuid.hpp" #include @@ -395,7 +395,7 @@ namespace Azure { namespace Core { namespace Http { namespace Policies { */ class RequestActivityPolicy final : public HttpPolicy { private: - Azure::Core::_internal::InputSanitizer m_inputSanitizer; + Azure::Core::Http::_internal::HttpSanitizer m_httpSanitizer; public: /** @@ -405,10 +405,11 @@ namespace Azure { namespace Core { namespace Http { namespace Policies { /** * @brief Constructs HTTP Request Activity policy. * - * @param inputSanitizer for sanitizing data before it is logged. + * @param httpSanitizer for sanitizing data before it is logged. */ - explicit RequestActivityPolicy(Azure::Core::_internal::InputSanitizer const& inputSanitizer) - : m_inputSanitizer(inputSanitizer) + explicit RequestActivityPolicy( + Azure::Core::Http::_internal::HttpSanitizer const& httpSanitizer) + : m_httpSanitizer(httpSanitizer) { } @@ -520,7 +521,7 @@ namespace Azure { namespace Core { namespace Http { namespace Policies { */ class LogPolicy final : public HttpPolicy { LogOptions m_options; - Azure::Core::_internal::InputSanitizer m_inputSanitizer; + Azure::Core::Http::_internal::HttpSanitizer m_httpSanitizer; public: /** @@ -529,7 +530,7 @@ namespace Azure { namespace Core { namespace Http { namespace Policies { */ explicit LogPolicy(LogOptions options) : m_options(std::move(options)), - m_inputSanitizer(m_options.AllowedHttpQueryParameters, m_options.AllowedHttpHeaders) + m_httpSanitizer(m_options.AllowedHttpQueryParameters, m_options.AllowedHttpHeaders) { } diff --git a/sdk/core/azure-core/inc/azure/core/internal/input_sanitizer.hpp b/sdk/core/azure-core/inc/azure/core/internal/http/http_sanitizer.hpp similarity index 83% rename from sdk/core/azure-core/inc/azure/core/internal/input_sanitizer.hpp rename to sdk/core/azure-core/inc/azure/core/internal/http/http_sanitizer.hpp index 2322f341a..4068012ea 100644 --- a/sdk/core/azure-core/inc/azure/core/internal/input_sanitizer.hpp +++ b/sdk/core/azure-core/inc/azure/core/internal/http/http_sanitizer.hpp @@ -6,8 +6,8 @@ #include "azure/core/url.hpp" #include -namespace Azure { namespace Core { namespace _internal { - class InputSanitizer final { +namespace Azure { namespace Core { namespace Http { namespace _internal { + class HttpSanitizer final { /** * @brief HTTP header names that are allowed to be logged. */ @@ -18,12 +18,9 @@ namespace Azure { namespace Core { namespace _internal { */ std::set m_allowedHttpQueryParameters; - // Manifest constant indicating a field was redacted. - static const char* m_RedactedPlaceholder; - public: - InputSanitizer() = default; - InputSanitizer( + HttpSanitizer() = default; + HttpSanitizer( std::set const& allowedHttpQueryParameters, Azure::Core::CaseInsensitiveSet const& allowedHttpHeaders) : m_allowedHttpHeaders(allowedHttpHeaders), @@ -47,4 +44,4 @@ namespace Azure { namespace Core { namespace _internal { */ std::string SanitizeHeader(std::string const& headerName, std::string const& headerValue) const; }; -}}} // namespace Azure::Core::_internal +}}}} // namespace Azure::Core::Http::_internal diff --git a/sdk/core/azure-core/inc/azure/core/internal/http/pipeline.hpp b/sdk/core/azure-core/inc/azure/core/internal/http/pipeline.hpp index 7e2e9c831..a44d24cbf 100644 --- a/sdk/core/azure-core/inc/azure/core/internal/http/pipeline.hpp +++ b/sdk/core/azure-core/inc/azure/core/internal/http/pipeline.hpp @@ -14,7 +14,7 @@ #include "azure/core/http/policies/policy.hpp" #include "azure/core/http/transport.hpp" #include "azure/core/internal/client_options.hpp" -#include "azure/core/internal/input_sanitizer.hpp" +#include "azure/core/internal/http/http_sanitizer.hpp" #include #include @@ -52,7 +52,7 @@ namespace Azure { namespace Core { namespace Http { namespace _internal { std::string const& telemetryServiceName = {}, std::string const& telemetryServiceVersion = {}) { - Azure::Core::_internal::InputSanitizer inputSanitizer( + Azure::Core::Http::_internal::HttpSanitizer httpSanitizer( clientOptions.Log.AllowedHttpQueryParameters, clientOptions.Log.AllowedHttpHeaders); auto const& perCallClientPolicies = clientOptions.PerOperationPolicies; @@ -111,7 +111,7 @@ namespace Azure { namespace Core { namespace Http { namespace _internal { // Add a request activity policy which will generate distributed traces for the pipeline. m_policies.emplace_back( std::make_unique( - inputSanitizer)); + httpSanitizer)); // logging - won't update request m_policies.emplace_back( diff --git a/sdk/core/azure-core/src/http/http_sanitizer.cpp b/sdk/core/azure-core/src/http/http_sanitizer.cpp new file mode 100644 index 000000000..1f2e40ce4 --- /dev/null +++ b/sdk/core/azure-core/src/http/http_sanitizer.cpp @@ -0,0 +1,85 @@ + +#include "azure/core/internal/http/http_sanitizer.hpp" +#include "azure/core/url.hpp" +#include +#include + +namespace { +std::string const RedactedPlaceholder = "REDACTED"; +} + +using Azure::Core::Http::_internal::HttpSanitizer; + +Azure::Core::Url HttpSanitizer::SanitizeUrl(Azure::Core::Url const& url) const +{ + std::ostringstream ss; + + // Sanitize the non-query part of the URL (remove username and password). + if (!url.GetScheme().empty()) + { + ss << url.GetScheme() << "://"; + } + ss << url.GetHost(); + if (url.GetPort() != 0) + { + ss << ":" << url.GetPort(); + } + if (!url.GetPath().empty()) + { + ss << "/" << url.GetPath(); + } + + { + auto encodedRequestQueryParams = url.GetQueryParameters(); + + std::remove_const::type>::type + loggedQueryParams; + + if (!encodedRequestQueryParams.empty()) + { + auto const& unencodedAllowedQueryParams = m_allowedHttpQueryParameters; + if (!unencodedAllowedQueryParams.empty()) + { + std::remove_const::type>::type + encodedAllowedQueryParams; + std::transform( + unencodedAllowedQueryParams.begin(), + unencodedAllowedQueryParams.end(), + std::inserter(encodedAllowedQueryParams, encodedAllowedQueryParams.begin()), + [](std::string const& s) { return Url::Encode(s); }); + + for (auto const& encodedRequestQueryParam : encodedRequestQueryParams) + { + if (encodedRequestQueryParam.second.empty() + || (encodedAllowedQueryParams.find(encodedRequestQueryParam.first) + != encodedAllowedQueryParams.end())) + { + loggedQueryParams.insert(encodedRequestQueryParam); + } + else + { + loggedQueryParams.insert( + std::make_pair(encodedRequestQueryParam.first, RedactedPlaceholder)); + } + } + } + else + { + for (auto const& encodedRequestQueryParam : encodedRequestQueryParams) + { + loggedQueryParams.insert( + std::make_pair(encodedRequestQueryParam.first, RedactedPlaceholder)); + } + } + + ss << Azure::Core::_detail::FormatEncodedUrlQueryParameters(loggedQueryParams); + } + } + return Azure::Core::Url(ss.str()); +} + +std::string HttpSanitizer::SanitizeHeader(std::string const& header, std::string const& value) const +{ + return (m_allowedHttpHeaders.find(header) != m_allowedHttpHeaders.end()) ? value + : RedactedPlaceholder; +} diff --git a/sdk/core/azure-core/src/http/log_policy.cpp b/sdk/core/azure-core/src/http/log_policy.cpp index a43704308..7f679e942 100644 --- a/sdk/core/azure-core/src/http/log_policy.cpp +++ b/sdk/core/azure-core/src/http/log_policy.cpp @@ -21,7 +21,7 @@ std::string RedactedPlaceholder = "REDACTED"; inline void AppendHeaders( std::ostringstream& log, - Azure::Core::_internal::InputSanitizer const& inputSanitizer, + Azure::Core::Http::_internal::HttpSanitizer const& httpSanitizer, Azure::Core::CaseInsensitiveMap const& headers) { for (auto const& header : headers) @@ -30,27 +30,27 @@ inline void AppendHeaders( if (!header.second.empty()) { - log << inputSanitizer.SanitizeHeader(header.first, header.second); + log << httpSanitizer.SanitizeHeader(header.first, header.second); } } } inline std::string GetRequestLogMessage( - Azure::Core::_internal::InputSanitizer const& inputSanitizer, + Azure::Core::Http::_internal::HttpSanitizer const& httpSanitizer, Request const& request) { std::ostringstream log; log << "HTTP Request : " << request.GetMethod().ToString() << " "; - Azure::Core::Url urlToLog(inputSanitizer.SanitizeUrl(request.GetUrl())); + Azure::Core::Url urlToLog(httpSanitizer.SanitizeUrl(request.GetUrl())); log << urlToLog.GetAbsoluteUrl(); - AppendHeaders(log, inputSanitizer, request.GetHeaders()); + AppendHeaders(log, httpSanitizer, request.GetHeaders()); return log.str(); } inline std::string GetResponseLogMessage( - Azure::Core::_internal::InputSanitizer const& inputSanitizer, + Azure::Core::Http::_internal::HttpSanitizer const& httpSanitizer, RawResponse const& response, std::chrono::system_clock::duration const& duration) { @@ -61,7 +61,7 @@ inline std::string GetResponseLogMessage( << "ms) : " << static_cast(response.GetStatusCode()) << " " << response.GetReasonPhrase(); - AppendHeaders(log, inputSanitizer, response.GetHeaders()); + AppendHeaders(log, httpSanitizer, response.GetHeaders()); return log.str(); } } // namespace @@ -107,7 +107,7 @@ std::unique_ptr LogPolicy::Send( if (Log::ShouldWrite(Logger::Level::Verbose)) { - Log::Write(Logger::Level::Informational, GetRequestLogMessage(m_inputSanitizer, request)); + Log::Write(Logger::Level::Informational, GetRequestLogMessage(m_httpSanitizer, request)); } else { @@ -119,8 +119,7 @@ std::unique_ptr LogPolicy::Send( auto const end = std::chrono::system_clock::now(); Log::Write( - Logger::Level::Informational, - GetResponseLogMessage(m_inputSanitizer, *response, end - start)); + Logger::Level::Informational, GetResponseLogMessage(m_httpSanitizer, *response, end - start)); return response; } diff --git a/sdk/core/azure-core/src/http/request_activity_policy.cpp b/sdk/core/azure-core/src/http/request_activity_policy.cpp index 2ca84be7c..d2c97bab7 100644 --- a/sdk/core/azure-core/src/http/request_activity_policy.cpp +++ b/sdk/core/azure-core/src/http/request_activity_policy.cpp @@ -3,7 +3,7 @@ #include "azure/core/http/policies/policy.hpp" #include "azure/core/internal/diagnostics/log.hpp" -#include "azure/core/internal/input_sanitizer.hpp" +#include "azure/core/internal/http/http_sanitizer.hpp" #include "azure/core/internal/tracing/service_tracing.hpp" #include @@ -62,8 +62,7 @@ std::unique_ptr RequestActivityPolicy::Send( createOptions.Attributes->AddAttribute( TracingAttributes::HttpMethod.ToString(), request.GetMethod().ToString()); - const std::string sanitizedUrl - = m_inputSanitizer.SanitizeUrl(request.GetUrl()).GetAbsoluteUrl(); + const std::string sanitizedUrl = m_httpSanitizer.SanitizeUrl(request.GetUrl()).GetAbsoluteUrl(); createOptions.Attributes->AddAttribute("http.url", sanitizedUrl); const Azure::Nullable requestId = request.GetHeader("x-ms-client-request-id"); if (requestId.HasValue()) diff --git a/sdk/core/azure-core/src/private/input_sanitizer.cpp b/sdk/core/azure-core/src/private/input_sanitizer.cpp deleted file mode 100644 index be2494e05..000000000 --- a/sdk/core/azure-core/src/private/input_sanitizer.cpp +++ /dev/null @@ -1,89 +0,0 @@ - -#include "azure/core/internal/input_sanitizer.hpp" -#include "azure/core/url.hpp" -#include -#include - -namespace Azure { namespace Core { namespace _internal { - - const char* InputSanitizer::m_RedactedPlaceholder = "REDACTED"; - - Azure::Core::Url InputSanitizer::SanitizeUrl(Azure::Core::Url const& url) const - { - std::ostringstream ss; - - // Sanitize the non-query part of the URL (remove username and password). - if (!url.GetScheme().empty()) - { - ss << url.GetScheme() << "://"; - } - ss << url.GetHost(); - if (url.GetPort() != 0) - { - ss << ":" << url.GetPort(); - } - if (!url.GetPath().empty()) - { - ss << "/" << url.GetPath(); - } - - { - auto encodedRequestQueryParams = url.GetQueryParameters(); - - std::remove_const::type>::type - loggedQueryParams; - - if (!encodedRequestQueryParams.empty()) - { - auto const& unencodedAllowedQueryParams = m_allowedHttpQueryParameters; - if (!unencodedAllowedQueryParams.empty()) - { - std::remove_const::type>:: - type encodedAllowedQueryParams; - std::transform( - unencodedAllowedQueryParams.begin(), - unencodedAllowedQueryParams.end(), - std::inserter(encodedAllowedQueryParams, encodedAllowedQueryParams.begin()), - [](std::string const& s) { return Url::Encode(s); }); - - for (auto const& encodedRequestQueryParam : encodedRequestQueryParams) - { - if (encodedRequestQueryParam.second.empty() - || (encodedAllowedQueryParams.find(encodedRequestQueryParam.first) - != encodedAllowedQueryParams.end())) - { - loggedQueryParams.insert(encodedRequestQueryParam); - } - else - { - loggedQueryParams.insert( - std::make_pair(encodedRequestQueryParam.first, m_RedactedPlaceholder)); - } - } - } - else - { - for (auto const& encodedRequestQueryParam : encodedRequestQueryParams) - { - loggedQueryParams.insert( - std::make_pair(encodedRequestQueryParam.first, m_RedactedPlaceholder)); - } - } - - ss << Azure::Core::_detail::FormatEncodedUrlQueryParameters(loggedQueryParams); - } - } - return Azure::Core::Url(ss.str()); - } - - std::string InputSanitizer::SanitizeHeader(std::string const& header, std::string const& value) - const - { - if (m_allowedHttpHeaders.find(header) != m_allowedHttpHeaders.end()) - { - return value; - } - return m_RedactedPlaceholder; - } - -}}} // namespace Azure::Core::_internal diff --git a/sdk/core/azure-core/test/ut/request_activity_policy_test.cpp b/sdk/core/azure-core/test/ut/request_activity_policy_test.cpp index 1144eea97..e2661f9c7 100644 --- a/sdk/core/azure-core/test/ut/request_activity_policy_test.cpp +++ b/sdk/core/azure-core/test/ut/request_activity_policy_test.cpp @@ -170,7 +170,7 @@ TEST(RequestActivityPolicy, Basic) std::vector> policies; // Add the request ID policy - this adds the x-ms-request-id attribute to the pipeline. policies.emplace_back( - std::make_unique(Azure::Core::_internal::InputSanitizer{})); + std::make_unique(Azure::Core::Http::_internal::HttpSanitizer{})); // Final policy - equivalent to HTTP policy. policies.emplace_back(std::make_unique()); @@ -205,7 +205,7 @@ TEST(RequestActivityPolicy, Basic) policies.emplace_back(std::make_unique()); policies.emplace_back(std::make_unique(RetryOptions{})); policies.emplace_back( - std::make_unique(Azure::Core::_internal::InputSanitizer{})); + std::make_unique(Azure::Core::Http::_internal::HttpSanitizer{})); // Final policy - equivalent to HTTP policy. policies.emplace_back(std::make_unique([&](Request& request) { userAgent = request.GetHeader("user-agent"); // Return success. @@ -248,7 +248,7 @@ TEST(RequestActivityPolicy, TryRetries) // Add the request ID policy - this adds the x-ms-request-id attribute to the pipeline. policies.emplace_back( - std::make_unique(Azure::Core::_internal::InputSanitizer{})); + std::make_unique(Azure::Core::Http::_internal::HttpSanitizer{})); // Final policy - equivalent to HTTP policy. int retryCount = 0; policies.emplace_back(std::make_unique([&](Request&) {