Fix potentially high CPU usage on Windows (#4448)

* Fix potentially high CPU usage on Windows * Undo unnecessary formatting * Undo unnecessary changelog * Undo unnecessary formatting * Undo unnecessary formatting * Uninclude locale * Add issue link to changelog * EXPECT_TRUE(a == b) => EXPECT_EQ(a, b) * Update second changelog with link as well --------- Co-authored-by: Anton Kolesnyk <[email protected]>
Azure · Apr 5, 2023 · dddaa5d · dddaa5d
1 parent b230d8c
commit dddaa5d
Show file tree

Hide file tree

Showing 11 changed files with 96 additions and 23 deletions.
diff --git a/sdk/attestation/azure-security-attestation/samples/policy-certificates/cryptohelpers.hpp b/sdk/attestation/azure-security-attestation/samples/policy-certificates/cryptohelpers.hpp
@@ -18,7 +18,7 @@
 #include <utility>
 #include <vector>
 /**
- * @brief THe Cryptography class provides a set of basic cryptographic primatives required
+ * @brief The Cryptography class provides a set of basic cryptographic primatives required
  * by the attestation samples.
  */
 

diff --git a/sdk/core/azure-core/CHANGELOG.md b/sdk/core/azure-core/CHANGELOG.md
@@ -9,6 +9,7 @@
 ### Bugs Fixed
 
 - [[#4213]](https://github.com/Azure/azure-sdk-for-cpp/issues/4213) Fixed a bug where `Host` request header is not set for non-default port (80, 443).
+- [[#4443]](https://github.com/Azure/azure-sdk-for-cpp/issues/4443) Fixed potentially high CPU usage on Windows.
 
 ### Other Changes
 

diff --git a/sdk/core/azure-core/inc/azure/core/internal/strings.hpp b/sdk/core/azure-core/inc/azure/core/internal/strings.hpp
@@ -30,6 +30,22 @@ namespace Azure { namespace Core { namespace _internal {
       return (c < 'A' || c > 'Z') ? c : c + ('a' - 'A');
     }
 
+    static constexpr bool IsDigit(char c) noexcept { return c >= '0' && c <= '9'; }
+
+    static constexpr bool IsHexDigit(char c) noexcept
+    {
+      return IsDigit(c) || (c >= 'A' && c <= 'F') || (c >= 'a' && c <= 'f');
+    }
+
+    static constexpr bool IsAlphaNumeric(char c) noexcept
+    {
+      return IsDigit(c) || (c >= 'A' && c <= 'Z') || (c >= 'a' && c <= 'z');
+    }
+
+    static constexpr bool IsSpace(char c) noexcept { return c == ' ' || (c >= '\t' && c <= '\r'); }
+
+    static constexpr bool IsPrintable(char c) noexcept { return c >= ' ' && c <= '~'; }
+
     struct CaseInsensitiveComparator final
     {
       bool operator()(std::string const& lhs, std::string const& rhs) const

diff --git a/sdk/core/azure-core/src/datetime.cpp b/sdk/core/azure-core/src/datetime.cpp
@@ -2,13 +2,13 @@
 // SPDX-License-Identifier: MIT
 
 #include "azure/core/datetime.hpp"
+#include "azure/core/internal/strings.hpp"
 #include "azure/core/platform.hpp"
 
 #include <algorithm>
 #include <ctime>
 #include <iomanip>
 #include <limits>
-#include <locale>
 #include <sstream>
 #include <stdexcept>
 
@@ -320,7 +320,7 @@ T ParseNumber(
     for (; i < MaxChars; ++i)
     {
       auto const ch = str[*cursor + i];
-      if (std::isdigit(ch, std::locale::classic()))
+      if (Core::_internal::StringExtensions::IsDigit(ch))
       {
         value = (value * 10) + (static_cast<decltype(value)>(static_cast<unsigned char>(ch)) - '0');
         continue;
@@ -655,7 +655,7 @@ DateTime DateTime::Parse(std::string const& dateTime, DateFormat format)
             if (charsRead == 7 && (DateTimeLength - cursor) > 0)
             {
               auto const ch = dateTime[cursor];
-              if (std::isdigit(ch, std::locale::classic()))
+              if (Core::_internal::StringExtensions::IsDigit(ch))
               {
                 auto const num = static_cast<int>(static_cast<unsigned char>(ch) - '0');
                 if (num > 4)
@@ -677,7 +677,7 @@ DateTime DateTime::Parse(std::string const& dateTime, DateFormat format)
 
           for (auto i = DateTimeLength - cursor; i > 0; --i)
           {
-            if (std::isdigit(dateTime[cursor], std::locale::classic()))
+            if (Core::_internal::StringExtensions::IsDigit(dateTime[cursor]))
             {
               ++minDateTimeLength;
               ++cursor;

diff --git a/sdk/core/azure-core/src/http/curl/curl.cpp b/sdk/core/azure-core/src/http/curl/curl.cpp
@@ -30,6 +30,7 @@
 #include "azure/core/http/policies/policy.hpp"
 #include "azure/core/http/transport.hpp"
 #include "azure/core/internal/diagnostics/log.hpp"
+#include "azure/core/internal/strings.hpp"
 
 // Private include
 #include "curl_connection_pool_private.hpp"
@@ -70,7 +71,6 @@
 #include <algorithm>
 #include <chrono>
 #include <iomanip>
-#include <locale>
 #include <sstream>
 #include <string>
 #include <thread>
@@ -1339,7 +1339,7 @@ void DumpCurlInfoToLog(std::string const& text, uint8_t* ptr, size_t size)
       // Log the contents of the buffer as text, if it's printable, print the character, otherwise
       // print '.'
       auto const ch = static_cast<char>(ptr[i + c]);
-      if (std::isprint(ch, std::locale::classic()))
+      if (Azure::Core::_internal::StringExtensions::IsPrintable(ch))
       {
         ss << ch;
       }

diff --git a/sdk/core/azure-core/src/http/http.cpp b/sdk/core/azure-core/src/http/http.cpp
@@ -3,10 +3,10 @@
 
 #include "azure/core/http/http.hpp"
 #include "azure/core/http/policies/policy.hpp"
+#include "azure/core/internal/strings.hpp"
 #include "azure/core/url.hpp"
 
 #include <algorithm>
-#include <locale>
 #include <unordered_set>
 #include <utility>
 
@@ -32,7 +32,7 @@ bool IsInvalidHeaderNameChar(char c)
   static std::unordered_set<char> const HeaderNameExtraValidChars
       = {' ', '!', '#', '$', '%', '&', '\'', '*', '+', '-', '.', '^', '_', '`', '|', '~'};
 
-  return !std::isalnum(c, std::locale::classic())
+  return !Azure::Core::_internal::StringExtensions::IsAlphaNumeric(c)
       && HeaderNameExtraValidChars.find(c) == HeaderNameExtraValidChars.end();
 }
 } // namespace

diff --git a/sdk/core/azure-core/src/http/url.cpp b/sdk/core/azure-core/src/http/url.cpp
@@ -6,7 +6,6 @@
 
 #include <algorithm>
 #include <limits>
-#include <locale>
 #include <stdexcept>
 #include <unordered_set>
 
@@ -22,7 +21,7 @@ Url::Url(std::string const& url)
 
     if (schemePos != std::string::npos)
     {
-      m_scheme = Azure::Core::_internal::StringExtensions::ToLower(url.substr(0, schemePos));
+      m_scheme = _internal::StringExtensions::ToLower(url.substr(0, schemePos));
       urlIter += schemePos + SchemeEnd.length();
     }
   }
@@ -43,8 +42,8 @@ Url::Url(std::string const& url)
   if (*urlIter == ':')
   {
     ++urlIter;
-    auto const portIter = std::find_if_not(
-        urlIter, url.end(), [](auto c) { return std::isdigit(c, std::locale::classic()); });
+    auto const portIter
+        = std::find_if_not(urlIter, url.end(), _internal::StringExtensions::IsDigit);
 
     auto const portNumber = std::stoi(std::string(urlIter, portIter));
 
@@ -105,8 +104,8 @@ std::string Url::Decode(std::string const& value)
     {
       case '%':
         if ((valueSize - i) < 3 // need at least 3 characters: "%XY"
-            || !std::isxdigit(value[i + 1], std::locale::classic())
-            || !std::isxdigit(value[i + 2], std::locale::classic()))
+            || !_internal::StringExtensions::IsHexDigit(value[i + 1])
+            || !_internal::StringExtensions::IsHexDigit(value[i + 2]))
         {
           throw std::runtime_error("failed when decoding URL component");
         }
@@ -133,7 +132,7 @@ bool ShouldEncode(char c)
 {
   static std::unordered_set<char> const ExtraNonEncodableChars = {'-', '.', '_', '~'};
 
-  return !std::isalnum(c, std::locale::classic())
+  return !_internal::StringExtensions::IsAlphaNumeric(c)
       && ExtraNonEncodableChars.find(c) == ExtraNonEncodableChars.end();
 }
 } // namespace

diff --git a/sdk/core/azure-core/src/http/user_agent.cpp b/sdk/core/azure-core/src/http/user_agent.cpp
@@ -11,9 +11,9 @@
 
 #include "azure/core/context.hpp"
 #include "azure/core/http/policies/policy.hpp"
+#include "azure/core/internal/strings.hpp"
 #include "azure/core/internal/tracing/service_tracing.hpp"
 #include "azure/core/platform.hpp"
-#include <locale>
 #include <sstream>
 
 #if defined(AZ_PLATFORM_WINDOWS)
@@ -133,10 +133,14 @@ std::string GetOSVersion()
 
 std::string TrimString(std::string s)
 {
-  auto const isNotSpace = [](char c) { return !std::isspace(c, std::locale::classic()); };
-
-  s.erase(s.begin(), std::find_if(s.begin(), s.end(), isNotSpace));
-  s.erase(std::find_if(s.rbegin(), s.rend(), isNotSpace).base(), s.end());
+  s.erase(
+      s.begin(),
+      std::find_if_not(s.begin(), s.end(), Azure::Core::_internal::StringExtensions::IsSpace));
+
+  s.erase(
+      std::find_if_not(s.rbegin(), s.rend(), Azure::Core::_internal::StringExtensions::IsSpace)
+          .base(),
+      s.end());
 
   return s;
 }

diff --git a/sdk/core/azure-core/test/ut/string_test.cpp b/sdk/core/azure-core/test/ut/string_test.cpp
@@ -43,6 +43,56 @@ TEST(String, toUpperC)
   }
 }
 
+TEST(String, isDigit)
+{
+  using Azure::Core::_internal::StringExtensions;
+  for (unsigned i = 0; i <= 255; ++i)
+  {
+    auto const c = static_cast<char>(static_cast<unsigned char>(i));
+    EXPECT_EQ(StringExtensions::IsDigit(c), std::isdigit(c, std::locale::classic()));
+  }
+}
+
+TEST(String, isHexDigit)
+{
+  using Azure::Core::_internal::StringExtensions;
+  for (unsigned i = 0; i <= 255; ++i)
+  {
+    auto const c = static_cast<char>(static_cast<unsigned char>(i));
+    EXPECT_EQ(StringExtensions::IsHexDigit(c), std::isxdigit(c, std::locale::classic()));
+  }
+}
+
+TEST(String, isAlphaNumeric)
+{
+  using Azure::Core::_internal::StringExtensions;
+  for (unsigned i = 0; i <= 255; ++i)
+  {
+    auto const c = static_cast<char>(static_cast<unsigned char>(i));
+    EXPECT_EQ(StringExtensions::IsAlphaNumeric(c), std::isalnum(c, std::locale::classic()));
+  }
+}
+
+TEST(String, isSpace)
+{
+  using Azure::Core::_internal::StringExtensions;
+  for (unsigned i = 0; i <= 255; ++i)
+  {
+    auto const c = static_cast<char>(static_cast<unsigned char>(i));
+    EXPECT_EQ(StringExtensions::IsSpace(c), std::isspace(c, std::locale::classic()));
+  }
+}
+
+TEST(String, isPrintable)
+{
+  using Azure::Core::_internal::StringExtensions;
+  for (unsigned i = 0; i <= 255; ++i)
+  {
+    auto const c = static_cast<char>(static_cast<unsigned char>(i));
+    EXPECT_EQ(StringExtensions::IsPrintable(c), std::isprint(c, std::locale::classic()));
+  }
+}
+
 TEST(String, toLower)
 {
   using Azure::Core::_internal::StringExtensions;

diff --git a/sdk/identity/azure-identity/CHANGELOG.md b/sdk/identity/azure-identity/CHANGELOG.md
@@ -8,6 +8,8 @@
 
 ### Bugs Fixed
 
+- [[#4443]](https://github.com/Azure/azure-sdk-for-cpp/issues/4443) Fixed potentially high CPU usage on Windows.
+
 ### Other Changes
 
 ## 1.4.0-beta.3 (2023-01-10)

diff --git a/sdk/identity/azure-identity/src/azure_cli_credential.cpp b/sdk/identity/azure-identity/src/azure_cli_credential.cpp
@@ -6,11 +6,11 @@
 #include "private/token_credential_impl.hpp"
 
 #include <azure/core/internal/environment.hpp>
+#include <azure/core/internal/strings.hpp>
 #include <azure/core/internal/unique_handle.hpp>
 #include <azure/core/platform.hpp>
 
 #include <cstdio>
-#include <locale>
 #include <stdexcept>
 #include <thread>
 #include <type_traits>
@@ -39,6 +39,7 @@ using Azure::Identity::AzureCliCredential;
 using Azure::DateTime;
 using Azure::Core::Context;
 using Azure::Core::_internal::Environment;
+using Azure::Core::_internal::StringExtensions;
 using Azure::Core::Credentials::AccessToken;
 using Azure::Core::Credentials::AuthenticationException;
 using Azure::Core::Credentials::TokenCredentialOptions;
@@ -63,7 +64,7 @@ void ThrowIfNotSafeCmdLineInput(std::string const& input, std::string const& des
         break;
 
       default:
-        if (!std::isalnum(c, std::locale::classic()))
+        if (!StringExtensions::IsAlphaNumeric(c))
         {
           throw AuthenticationException(
               "AzureCliCredential: Unsafe command line input found in " + description + ": "