From ed6c1802cc955a49b4d71cffb00526dd24d95bea Mon Sep 17 00:00:00 2001 From: Terry Mancey Date: Mon, 17 Oct 2022 18:31:44 -0500 Subject: [PATCH 1/6] Refactor raw loops to use base::ranges --- .../deprecated/client/client_state_manager.cc | 29 ++++++++++++------- 1 file changed, 19 insertions(+), 10 deletions(-) diff --git a/vendor/bat-native-ads/src/bat/ads/internal/deprecated/client/client_state_manager.cc b/vendor/bat-native-ads/src/bat/ads/internal/deprecated/client/client_state_manager.cc index ca56251cc07..f70abaf732c 100644 --- a/vendor/bat-native-ads/src/bat/ads/internal/deprecated/client/client_state_manager.cc +++ b/vendor/bat-native-ads/src/bat/ads/internal/deprecated/client/client_state_manager.cc @@ -15,6 +15,7 @@ #include "base/time/time.h" #include "bat/ads/ad_info.h" #include "bat/ads/ad_type.h" +#include "bat/ads/history_item_info.h" #include "bat/ads/internal/ads_client_helper.h" #include "bat/ads/internal/base/logging_util.h" #include "bat/ads/internal/deprecated/client/client_info.h" @@ -256,13 +257,17 @@ AdContentLikeActionType ClientStateManager::ToggleAdThumbDown( AdContentLikeActionType ClientStateManager::GetAdContentLikeActionTypeForAdvertiser( const std::string& advertiser_id) { - for (const auto& item : client_->history_items) { - if (item.ad_content.advertiser_id == advertiser_id) { - return item.ad_content.like_action_type; - } + const auto iter = base::ranges::find_if( + client_->history_items, + [&advertiser_id](const HistoryItemInfo& history_item) -> bool { + return history_item.ad_content.advertiser_id == advertiser_id; + }); + + if (iter == client_->history_items.cend()) { + return AdContentLikeActionType::kNeutral; } - return AdContentLikeActionType::kNeutral; + return iter->ad_content.like_action_type; } CategoryContentOptActionType ClientStateManager::ToggleAdOptIn( @@ -327,13 +332,17 @@ CategoryContentOptActionType ClientStateManager::ToggleAdOptOut( CategoryContentOptActionType ClientStateManager::GetCategoryContentOptActionTypeForSegment( const std::string& segment) { - for (const auto& item : client_->history_items) { - if (item.category_content.category == segment) { - return item.category_content.opt_action_type; - } + const auto iter = base::ranges::find_if( + client_->history_items, + [&segment](const HistoryItemInfo& history_item) -> bool { + return history_item.category_content.category == segment; + }); + + if (iter == client_->history_items.cend()) { + return CategoryContentOptActionType::kNone; } - return CategoryContentOptActionType::kNone; + return iter->category_content.opt_action_type; } bool ClientStateManager::ToggleSavedAd(const AdContentInfo& ad_content) { From 1f8eaad473973bc937187861ce6cc1661f9d6456 Mon Sep 17 00:00:00 2001 From: Terry Mancey Date: Mon, 17 Oct 2022 18:35:26 -0500 Subject: [PATCH 2/6] Fix comparison 'kMaximumNotificationAds > 0' is always false for non Android builds --- .../creatives/notification_ads/notification_ad_manager.cc | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/vendor/bat-native-ads/src/bat/ads/internal/creatives/notification_ads/notification_ad_manager.cc b/vendor/bat-native-ads/src/bat/ads/internal/creatives/notification_ads/notification_ad_manager.cc index 8ab9048c041..8ede5f6ea63 100644 --- a/vendor/bat-native-ads/src/bat/ads/internal/creatives/notification_ads/notification_ad_manager.cc +++ b/vendor/bat-native-ads/src/bat/ads/internal/creatives/notification_ads/notification_ad_manager.cc @@ -26,8 +26,6 @@ NotificationAdManager* g_notification_ad_manager_instance = nullptr; #if BUILDFLAG(IS_ANDROID) constexpr int kMaximumNotificationAds = 3; -#else // !BUILDFLAG(IS_ANDROID) -constexpr int kMaximumNotificationAds = 0; // Unlimited #endif // BUILDFLAG(IS_ANDROID) } // namespace @@ -76,12 +74,14 @@ void NotificationAdManager::Add(const NotificationAdInfo& ad) { ads_.push_back(ad); - if (kMaximumNotificationAds > 0 && ads_.size() > kMaximumNotificationAds) { +#if BUILDFLAG(IS_ANDROID) + if (ads_.size() > kMaximumNotificationAds) { AdsClientHelper::GetInstance()->CloseNotificationAd( ads_.front().placement_id); ads_.pop_front(); } +#endif // BUILDFLAG(IS_ANDROID) AdsClientHelper::GetInstance()->SetListPref(prefs::kNotificationAds, NotificationAdsToValue(ads_)); From 7dc7dd93d36bc5f49ae71b51b14bad9f59dc7a46 Mon Sep 17 00:00:00 2001 From: Terry Mancey Date: Mon, 17 Oct 2022 18:39:43 -0500 Subject: [PATCH 3/6] Fix exit path from function with non-void return type that has missing return statement --- .../src/bat/ads/internal/conversions/conversions.cc | 4 ++++ .../internal/history/filters/confirmation_history_filter.cc | 4 ++++ 2 files changed, 8 insertions(+) diff --git a/vendor/bat-native-ads/src/bat/ads/internal/conversions/conversions.cc b/vendor/bat-native-ads/src/bat/ads/internal/conversions/conversions.cc index 80209964142..b6b794539f0 100644 --- a/vendor/bat-native-ads/src/bat/ads/internal/conversions/conversions.cc +++ b/vendor/bat-native-ads/src/bat/ads/internal/conversions/conversions.cc @@ -89,6 +89,10 @@ bool DoesConfirmationTypeMatchConversionType( return false; } } + + NOTREACHED() << "Unexpected value for ConfirmationType: " + << static_cast(confirmation_type.value()); + return false; } std::string ExtractConversionIdFromText( diff --git a/vendor/bat-native-ads/src/bat/ads/internal/history/filters/confirmation_history_filter.cc b/vendor/bat-native-ads/src/bat/ads/internal/history/filters/confirmation_history_filter.cc index c2c6d9f993e..1b61f00c040 100644 --- a/vendor/bat-native-ads/src/bat/ads/internal/history/filters/confirmation_history_filter.cc +++ b/vendor/bat-native-ads/src/bat/ads/internal/history/filters/confirmation_history_filter.cc @@ -37,6 +37,10 @@ bool ShouldFilterConfirmationType(const ConfirmationType& confirmation_type) { return true; } } + + NOTREACHED() << "Unexpected value for ConfirmationType: " + << static_cast(confirmation_type.value()); + return true; } std::map BuildBuckets( From 9f015a9eac7888d78a6facd2decdc395096cba60 Mon Sep 17 00:00:00 2001 From: Terry Mancey Date: Mon, 17 Oct 2022 18:42:18 -0500 Subject: [PATCH 4/6] Fix condition 'date_range_filter' is always true --- .../src/bat/ads/internal/history/history_manager.cc | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/vendor/bat-native-ads/src/bat/ads/internal/history/history_manager.cc b/vendor/bat-native-ads/src/bat/ads/internal/history/history_manager.cc index 219a03771db..b6341b7abad 100644 --- a/vendor/bat-native-ads/src/bat/ads/internal/history/history_manager.cc +++ b/vendor/bat-native-ads/src/bat/ads/internal/history/history_manager.cc @@ -70,9 +70,7 @@ HistoryItemList HistoryManager::Get(const HistoryFilterType filter_type, const auto date_range_filter = std::make_unique(from_time, to_time); - if (date_range_filter) { - history_items = date_range_filter->Apply(history_items); - } + history_items = date_range_filter->Apply(history_items); const auto filter = HistoryFilterFactory::Build(filter_type); if (filter) { From 7b9f20569238b6521fde2e02076a96f4f0f3f48b Mon Sep 17 00:00:00 2001 From: Terry Mancey Date: Mon, 17 Oct 2022 18:56:25 -0500 Subject: [PATCH 5/6] Fix local variable 'dict' shadows outer variable --- .../confirmation_state_manager.cc | 24 +++++++++++-------- 1 file changed, 14 insertions(+), 10 deletions(-) diff --git a/vendor/bat-native-ads/src/bat/ads/internal/deprecated/confirmations/confirmation_state_manager.cc b/vendor/bat-native-ads/src/bat/ads/internal/deprecated/confirmations/confirmation_state_manager.cc index 7ac13eae415..0ba82591831 100644 --- a/vendor/bat-native-ads/src/bat/ads/internal/deprecated/confirmations/confirmation_state_manager.cc +++ b/vendor/bat-native-ads/src/bat/ads/internal/deprecated/confirmations/confirmation_state_manager.cc @@ -201,9 +201,9 @@ bool GetFailedConfirmationsFromDictionary(const base::Value::Dict& dict, ConfirmationList new_failed_confirmations; - for (const auto& item : *failed_confirmations) { - const base::Value::Dict* const dict = item.GetIfDict(); - if (!dict) { + for (const auto& value : *failed_confirmations) { + const base::Value::Dict* const failed_confirmation_dict = value.GetIfDict(); + if (!failed_confirmation_dict) { BLOG(0, "Confirmation should be a dictionary"); continue; } @@ -211,7 +211,8 @@ bool GetFailedConfirmationsFromDictionary(const base::Value::Dict& dict, ConfirmationInfo confirmation; // Transaction id - if (const std::string* const value = dict->FindString("transaction_id")) { + if (const std::string* const value = + failed_confirmation_dict->FindString("transaction_id")) { confirmation.transaction_id = *value; } else { // Migrate legacy confirmations @@ -221,7 +222,7 @@ bool GetFailedConfirmationsFromDictionary(const base::Value::Dict& dict, // Creative instance id if (const std::string* const value = - dict->FindString("creative_instance_id")) { + failed_confirmation_dict->FindString("creative_instance_id")) { confirmation.creative_instance_id = *value; } else { BLOG(0, "Missing confirmation creative instance id"); @@ -229,7 +230,8 @@ bool GetFailedConfirmationsFromDictionary(const base::Value::Dict& dict, } // Type - if (const std::string* const value = dict->FindString("type")) { + if (const std::string* const value = + failed_confirmation_dict->FindString("type")) { confirmation.type = ConfirmationType(*value); } else { BLOG(0, "Missing confirmation type"); @@ -237,7 +239,8 @@ bool GetFailedConfirmationsFromDictionary(const base::Value::Dict& dict, } // Ad type - if (const std::string* const value = dict->FindString("ad_type")) { + if (const std::string* const value = + failed_confirmation_dict->FindString("ad_type")) { confirmation.ad_type = AdType(*value); } else { // Migrate legacy confirmations, this value is not used right now so safe @@ -247,7 +250,7 @@ bool GetFailedConfirmationsFromDictionary(const base::Value::Dict& dict, // Created at if (const std::string* const value = - dict->FindString("timestamp_in_seconds")) { + failed_confirmation_dict->FindString("timestamp_in_seconds")) { double timestamp_as_double; if (!base::StringToDouble(*value, ×tamp_as_double)) { continue; @@ -257,11 +260,12 @@ bool GetFailedConfirmationsFromDictionary(const base::Value::Dict& dict, } // Was created - const absl::optional was_created = dict->FindBool("created"); + const absl::optional was_created = + failed_confirmation_dict->FindBool("created"); confirmation.was_created = was_created.value_or(true); // Opted-in - confirmation.opted_in = GetOptedIn(*dict); + confirmation.opted_in = GetOptedIn(*failed_confirmation_dict); if (!IsValid(confirmation)) { BLOG(0, "Invalid confirmation"); From 5365f33e659163056848c0e1272922eb3d7295c0 Mon Sep 17 00:00:00 2001 From: Terry Mancey Date: Wed, 19 Oct 2022 15:54:43 -0500 Subject: [PATCH 6/6] Fix false positive when returning object that points to local variable 'padded_plaintext' that will be invalid when returning --- .../bat/ads/internal/base/crypto/crypto_unittest_util.cc | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/vendor/bat-native-ads/src/bat/ads/internal/base/crypto/crypto_unittest_util.cc b/vendor/bat-native-ads/src/bat/ads/internal/base/crypto/crypto_unittest_util.cc index 1a5dbbd8efd..88b7c7fad6a 100644 --- a/vendor/bat-native-ads/src/bat/ads/internal/base/crypto/crypto_unittest_util.cc +++ b/vendor/bat-native-ads/src/bat/ads/internal/base/crypto/crypto_unittest_util.cc @@ -5,6 +5,9 @@ #include "bat/ads/internal/base/crypto/crypto_unittest_util.h" +#include +#include + #include "tweetnacl.h" // NOLINT namespace ads::security { @@ -18,9 +21,9 @@ std::vector Decrypt(const std::vector& ciphertext, ciphertext.size(), &nonce.front(), &ephemeral_public_key.front(), &secret_key.front()); - std::vector plaintext( - padded_plaintext.cbegin() + crypto_box_ZEROBYTES, - padded_plaintext.cend()); + std::vector plaintext; + std::copy(padded_plaintext.cbegin() + crypto_box_ZEROBYTES, + padded_plaintext.cend(), std::back_inserter(plaintext)); return plaintext; }