Use base::StringPrintf in constexrp contexts

Upstream has made this function constexpr, in order to force callers to change its call sites to use a constexpr string, in order to be able to validate at compile time the format string. There is an alternative `base::StringPrintfNonConstexpr`, but obviously this unsafe variant should be avoided whenever possible. This change goes over the different places where the `constexpr` constrained was being violated, and corrects each callsite. This has also revealed a buggy test in `SimulationResponseParserUnitTest`, where the formatting string wasn't correct. Chromium change: https://chromium.googlesource.com/chromium/src/+/8bf39fbefcd2963b3647c24f2b27dfb5c40875c4 commit 8bf39fbefcd2963b3647c24f2b27dfb5c40875c4 Author: Peter Kasting <pkasting@chromium.org> Date: Fri Sep 13 19:07:33 2024 +0000 Disallow StringPrintf() with non-constexpr format strings. Bug: 365705855
brave · Sep 19, 2024 · 133e626 · 133e626
1 parent a2054d5
commit 133e626
Show file tree

Hide file tree

Showing 22 changed files with 89 additions and 104 deletions.
diff --git a/browser/net/brave_network_audit_allowed_lists.h b/browser/net/brave_network_audit_allowed_lists.h
@@ -79,13 +79,6 @@ inline constexpr const char* kAllowedUrlPrefixes[] = {
     "https://static1.brave.com/",
 };
 
-inline constexpr const char* kAllowedBraveSearchTemplates[] = {
-    // Brave search
-    // The test simulation has a pattern https://search.brave.com:<port>
-    // port is changed dynamically
-    "https://search.brave.com:%s/",
-};
-
 // Before adding to this list, get approval from the security team.
 inline constexpr const char* kAllowedUrlPatterns[] = {
     // allowed because it's url for fetching super referral's mapping table

diff --git a/browser/net/brave_network_audit_search_ad_browsertest.cc b/browser/net/brave_network_audit_search_ad_browsertest.cc
@@ -14,7 +14,7 @@
 #include "base/memory/raw_ptr.h"
 #include "base/path_service.h"
 #include "base/run_loop.h"
-#include "base/strings/stringprintf.h"
+#include "base/strings/strcat.h"
 #include "base/task/single_thread_task_runner.h"
 #include "base/test/bind.h"
 #include "base/test/scoped_feature_list.h"
@@ -119,9 +119,13 @@ class BraveNetworkAuditSearchAdTest : public InProcessBrowserTest {
     std::string port =
         base::NumberToString(https_server()->host_port_pair().port());
     std::vector<std::string> allowed_prefixes;
-    for (const char* prefix : kAllowedBraveSearchTemplates) {
-      allowed_prefixes.push_back(base::StringPrintf(prefix, port.c_str()));
-    }
+
+    // Brave search
+    // The test simulation has a pattern https://search.brave.com:<port>
+    // port is changed dynamically
+    const char kAllowedBraveSearchTemplate[] = "https://search.brave.com:%s/";
+    allowed_prefixes.push_back(
+        base::StringPrintf(kAllowedBraveSearchTemplate, port.c_str()));
     VerifyNetworkAuditLog(net_log_path_, audit_results_path_, allowed_prefixes);
   }
 

diff --git a/components/brave_rewards/core/endpoints/brave/get_wallet.cc b/components/brave_rewards/core/endpoints/brave/get_wallet.cc
@@ -9,6 +9,7 @@
 #include <utility>
 
 #include "base/json/json_reader.h"
+#include "base/strings/strcat.h"
 #include "brave/components/brave_rewards/core/common/environment_config.h"
 #include "brave/components/brave_rewards/core/common/request_signer.h"
 #include "brave/components/brave_rewards/core/common/url_helpers.h"
@@ -23,6 +24,8 @@ using Result = GetWallet::Result;
 
 namespace {
 
+constexpr char kPath[] = "/v4/wallets/";
+
 Result ParseBody(RewardsEngine& engine, const std::string& body) {
   auto value = base::JSONReader::Read(body);
   if (!value || !value->is_dict()) {
@@ -92,10 +95,6 @@ GetWallet::GetWallet(RewardsEngine& engine) : RequestBuilder(engine) {}
 
 GetWallet::~GetWallet() = default;
 
-std::string GetWallet::Path() const {
-  return "/v4/wallets/";
-}
-
 std::optional<std::string> GetWallet::Url() const {
   const auto wallet = engine_->wallet()->GetWallet();
   if (!wallet) {
@@ -105,7 +104,7 @@ std::optional<std::string> GetWallet::Url() const {
 
   auto url =
       URLHelpers::Resolve(engine_->Get<EnvironmentConfig>().rewards_grant_url(),
-                          {Path(), wallet->payment_id});
+                          {kPath, wallet->payment_id});
 
   return url.spec();
 }
@@ -131,8 +130,8 @@ std::optional<std::vector<std::string>> GetWallet::Headers(
     return std::nullopt;
   }
 
-  return signer->GetSignedHeaders("get " + Path() + wallet->payment_id,
-                                  content);
+  return signer->GetSignedHeaders(
+      base::StrCat({"get ", kPath, wallet->payment_id}), content);
 }
 
 }  // namespace brave_rewards::internal::endpoints
diff --git a/components/brave_rewards/core/endpoints/brave/get_wallet.h b/components/brave_rewards/core/endpoints/brave/get_wallet.h
@@ -79,8 +79,6 @@ class GetWallet final : public RequestBuilder,
   ~GetWallet() override;
 
  private:
-  std::string Path() const;
-
   std::optional<std::string> Url() const override;
   mojom::UrlMethod Method() const override;
   std::optional<std::vector<std::string>> Headers(

diff --git a/components/brave_rewards/core/endpoints/brave/patch_wallets.cc b/components/brave_rewards/core/endpoints/brave/patch_wallets.cc
@@ -10,7 +10,7 @@
 
 #include "base/json/json_reader.h"
 #include "base/json/json_writer.h"
-#include "base/strings/stringprintf.h"
+#include "base/strings/strcat.h"
 #include "brave/components/brave_rewards/core/common/environment_config.h"
 #include "brave/components/brave_rewards/core/common/request_signer.h"
 #include "brave/components/brave_rewards/core/rewards_engine.h"
@@ -23,6 +23,8 @@ using Result = PatchWallets::Result;
 
 namespace {
 
+constexpr char kPatchWalletsPathPrefix[] = "/v4/wallets/";
+
 Result ParseBody(RewardsEngine& engine, const std::string& body) {
   const auto value = base::JSONReader::Read(body);
   if (!value || !value->is_dict()) {
@@ -87,10 +89,6 @@ PatchWallets::PatchWallets(RewardsEngine& engine, std::string&& geo_country)
 
 PatchWallets::~PatchWallets() = default;
 
-const char* PatchWallets::Path() const {
-  return "/v4/wallets/%s";
-}
-
 std::optional<std::string> PatchWallets::Url() const {
   const auto wallet = engine_->wallet()->GetWallet();
   if (!wallet) {
@@ -102,7 +100,7 @@ std::optional<std::string> PatchWallets::Url() const {
 
   return engine_->Get<EnvironmentConfig>()
       .rewards_grant_url()
-      .Resolve(base::StringPrintf(Path(), wallet->payment_id.c_str()))
+      .Resolve(base::StrCat({kPatchWalletsPathPrefix, wallet->payment_id}))
       .spec();
 }
 
@@ -128,7 +126,7 @@ std::optional<std::vector<std::string>> PatchWallets::Headers(
   }
 
   return signer->GetSignedHeaders(
-      "patch " + base::StringPrintf(Path(), wallet->payment_id.c_str()),
+      base::StrCat({"patch ", kPatchWalletsPathPrefix, wallet->payment_id}),
       content);
 }
 

diff --git a/components/brave_rewards/core/endpoints/brave/patch_wallets.h b/components/brave_rewards/core/endpoints/brave/patch_wallets.h
@@ -48,8 +48,6 @@ class PatchWallets final : public RequestBuilder,
   ~PatchWallets() override;
 
  private:
-  const char* Path() const;
-
   std::optional<std::string> Url() const override;
   mojom::UrlMethod Method() const override;
   std::optional<std::vector<std::string>> Headers(

diff --git a/components/brave_rewards/core/endpoints/brave/post_connect_bitflyer.cc b/components/brave_rewards/core/endpoints/brave/post_connect_bitflyer.cc
@@ -9,6 +9,7 @@
 #include <utility>
 
 #include "base/json/json_writer.h"
+#include "base/strings/stringprintf.h"
 #include "brave/components/brave_rewards/core/rewards_engine.h"
 
 namespace brave_rewards::internal::endpoints {
@@ -37,8 +38,8 @@ std::optional<std::string> PostConnectBitflyer::Content() const {
   return json;
 }
 
-const char* PostConnectBitflyer::Path() const {
-  return "/v3/wallet/bitflyer/%s/claim";
+std::string PostConnectBitflyer::Path(base::cstring_view payment_id) const {
+  return base::StringPrintf("/v3/wallet/bitflyer/%s/claim", payment_id.c_str());
 }
 
 }  // namespace brave_rewards::internal::endpoints
diff --git a/components/brave_rewards/core/endpoints/brave/post_connect_bitflyer.h b/components/brave_rewards/core/endpoints/brave/post_connect_bitflyer.h
@@ -38,7 +38,7 @@ class PostConnectBitflyer final : public PostConnect {
  private:
   std::optional<std::string> Content() const override;
 
-  const char* Path() const override;
+  std::string Path(base::cstring_view payment_id) const override;
 
   std::string linking_info_;
 };

diff --git a/components/brave_rewards/core/endpoints/brave/post_connect_gemini.cc b/components/brave_rewards/core/endpoints/brave/post_connect_gemini.cc
@@ -9,6 +9,7 @@
 #include <utility>
 
 #include "base/json/json_writer.h"
+#include "base/strings/stringprintf.h"
 #include "brave/components/brave_rewards/core/rewards_engine.h"
 
 namespace brave_rewards::internal::endpoints {
@@ -46,8 +47,8 @@ std::optional<std::string> PostConnectGemini::Content() const {
   return json;
 }
 
-const char* PostConnectGemini::Path() const {
-  return "/v3/wallet/gemini/%s/claim";
+std::string PostConnectGemini::Path(base::cstring_view payment_id) const {
+  return base::StringPrintf("/v3/wallet/gemini/%s/claim", payment_id.c_str());
 }
 
 }  // namespace brave_rewards::internal::endpoints
diff --git a/components/brave_rewards/core/endpoints/brave/post_connect_gemini.h b/components/brave_rewards/core/endpoints/brave/post_connect_gemini.h
@@ -41,7 +41,7 @@ class PostConnectGemini final : public PostConnect {
  private:
   std::optional<std::string> Content() const override;
 
-  const char* Path() const override;
+  std::string Path(base::cstring_view payment_id) const override;
 
   std::string linking_info_;
   std::string recipient_id_;

diff --git a/components/brave_rewards/core/endpoints/brave/post_connect_uphold.cc b/components/brave_rewards/core/endpoints/brave/post_connect_uphold.cc
@@ -10,6 +10,7 @@
 
 #include "base/base64.h"
 #include "base/json/json_writer.h"
+#include "base/strings/stringprintf.h"
 #include "brave/components/brave_rewards/core/common/request_signer.h"
 #include "brave/components/brave_rewards/core/rewards_engine.h"
 #include "brave/components/brave_rewards/core/wallet/wallet.h"
@@ -102,8 +103,8 @@ std::optional<std::vector<std::string>> PostConnectUphold::Headers(
   return std::vector<std::string>{};
 }
 
-const char* PostConnectUphold::Path() const {
-  return "/v3/wallet/uphold/%s/claim";
+std::string PostConnectUphold::Path(base::cstring_view payment_id) const {
+  return base::StringPrintf("/v3/wallet/uphold/%s/claim", payment_id.c_str());
 }
 
 }  // namespace brave_rewards::internal::endpoints
diff --git a/components/brave_rewards/core/endpoints/brave/post_connect_uphold.h b/components/brave_rewards/core/endpoints/brave/post_connect_uphold.h
@@ -58,7 +58,7 @@ class PostConnectUphold final : public PostConnect {
   std::optional<std::vector<std::string>> Headers(
       const std::string& content) const override;
 
-  const char* Path() const override;
+  std::string Path(base::cstring_view payment_id) const override;
 
   std::string address_;
 };

diff --git a/components/brave_rewards/core/endpoints/brave/post_connect_zebpay.cc b/components/brave_rewards/core/endpoints/brave/post_connect_zebpay.cc
@@ -9,6 +9,7 @@
 #include <utility>
 
 #include "base/json/json_writer.h"
+#include "base/strings/stringprintf.h"
 #include "brave/components/brave_rewards/core/rewards_engine.h"
 
 namespace brave_rewards::internal::endpoints {
@@ -37,8 +38,8 @@ std::optional<std::string> PostConnectZebPay::Content() const {
   return json;
 }
 
-const char* PostConnectZebPay::Path() const {
-  return "/v3/wallet/zebpay/%s/claim";
+std::string PostConnectZebPay::Path(base::cstring_view payment_id) const {
+  return base::StringPrintf("/v3/wallet/zebpay/%s/claim", payment_id.c_str());
 }
 
 }  // namespace brave_rewards::internal::endpoints
diff --git a/components/brave_rewards/core/endpoints/brave/post_connect_zebpay.h b/components/brave_rewards/core/endpoints/brave/post_connect_zebpay.h
@@ -38,7 +38,7 @@ class PostConnectZebPay final : public PostConnect {
  private:
   std::optional<std::string> Content() const override;
 
-  const char* Path() const override;
+  std::string Path(base::cstring_view payment_id) const override;
 
   std::string linking_info_;
 };

diff --git a/components/brave_rewards/core/endpoints/common/post_connect.cc b/components/brave_rewards/core/endpoints/common/post_connect.cc
@@ -182,7 +182,7 @@ std::optional<std::string> PostConnect::Url() const {
 
   return engine_->Get<EnvironmentConfig>()
       .rewards_grant_url()
-      .Resolve(base::StringPrintf(Path(), wallet->payment_id.c_str()))
+      .Resolve(Path(wallet->payment_id))
       .spec();
 }
 
@@ -203,9 +203,7 @@ std::optional<std::vector<std::string>> PostConnect::Headers(
     return std::nullopt;
   }
 
-  return signer->GetSignedHeaders(
-      "post " + base::StringPrintf(Path(), wallet->payment_id.c_str()),
-      content);
+  return signer->GetSignedHeaders("post " + Path(wallet->payment_id), content);
 }
 
 std::string PostConnect::ContentType() const {

diff --git a/components/brave_rewards/core/endpoints/common/post_connect.h b/components/brave_rewards/core/endpoints/common/post_connect.h
@@ -10,6 +10,7 @@
 #include <string>
 #include <vector>
 
+#include "base/strings/cstring_view.h"
 #include "brave/components/brave_rewards/common/mojom/rewards.mojom.h"
 #include "brave/components/brave_rewards/common/mojom/rewards_core.mojom.h"
 #include "brave/components/brave_rewards/core/endpoints/request_builder.h"
@@ -40,7 +41,7 @@ class PostConnect : public RequestBuilder, public ResponseHandler<PostConnect> {
   ~PostConnect() override;
 
  protected:
-  virtual const char* Path() const = 0;
+  virtual std::string Path(base::cstring_view payment_id) const = 0;
 
  private:
   std::optional<std::string> Url() const override;

diff --git a/components/brave_rewards/core/endpoints/common/post_connect_unittest.cc b/components/brave_rewards/core/endpoints/common/post_connect_unittest.cc
@@ -9,6 +9,7 @@
 #include <tuple>
 #include <utility>
 
+#include "base/strings/stringprintf.h"
 #include "brave/components/brave_rewards/core/common/environment_config.h"
 #include "brave/components/brave_rewards/core/endpoints/request_for.h"
 #include "brave/components/brave_rewards/core/state/state_keys.h"
@@ -31,7 +32,9 @@ class PostConnectMock final : public endpoints::PostConnect {
   ~PostConnectMock() override = default;
 
  private:
-  const char* Path() const override { return "/v3/wallet/mock/%s/claim"; }
+  std::string Path(base::cstring_view payment_id) const override {
+    return base::StringPrintf("/v3/wallet/mock/%s/claim", payment_id.c_str());
+  }
 };
 
 using PostConnectParamType = std::tuple<