Fix crash if catalog has less then tree unique segments.

fix brave/brave-browser#21052
brave · Feb 15, 2022 · 0016d19 · 0016d19
1 parent b599acc
commit 0016d19
Show file tree

Hide file tree

Showing 3 changed files with 31 additions and 11 deletions.
diff --git a/...internal/ad_serving/ad_targeting/models/behavioral/bandits/epsilon_greedy_bandit_model.cc b/...internal/ad_serving/ad_targeting/models/behavioral/bandits/epsilon_greedy_bandit_model.cc
@@ -79,6 +79,9 @@ SegmentList GetEligibleSegments() {
 EpsilonGreedyBanditArmMap GetEligibleArms(
     const EpsilonGreedyBanditArmMap& arms) {
   const SegmentList eligible_segments = GetEligibleSegments();
+  if (eligible_segments.empty()) {
+    return {};
+  }
 
   EpsilonGreedyBanditArmMap eligible_arms;
 
@@ -119,7 +122,7 @@ ArmList GetTopArms(const ArmBucketList& buckets, const size_t count) {
     ArmList arms = bucket.second;
     if (arms.size() > available_arms) {
       // Sample without replacement
-      base::RandomShuffle(begin(arms), end(arms));
+      base::RandomShuffle(std::begin(arms), std::end(arms));
       arms.resize(available_arms);
     }
 
@@ -136,8 +139,10 @@ SegmentList ExploreSegments(const EpsilonGreedyBanditArmMap& arms) {
     segments.push_back(arm.first);
   }
 
-  base::RandomShuffle(begin(segments), end(segments));
-  segments.resize(kTopArmCount);
+  if (segments.size() > kTopArmCount) {
+    base::RandomShuffle(std::begin(segments), std::end(segments));
+    segments.resize(kTopArmCount);
+  }
 
   BLOG(2, "Exploring epsilon greedy bandit segments:");
   for (const auto& segment : segments) {

diff --git a/...ad_serving/ad_targeting/models/behavioral/bandits/epsilon_greedy_bandit_model_unittest.cc b/...ad_serving/ad_targeting/models/behavioral/bandits/epsilon_greedy_bandit_model_unittest.cc
@@ -43,6 +43,22 @@ TEST_F(BatAdsEpsilonGreedyBanditModelTest,
   EXPECT_TRUE(segments.empty());
 }
 
+TEST_F(BatAdsEpsilonGreedyBanditModelTest, EligableSegmentsAreEmpty) {
+  // Arrange
+  base::test::ScopedFeatureList scoped_feature_list;
+  scoped_feature_list.InitAndEnableFeatureWithParameters(
+      features::kEpsilonGreedyBandit, {{"epsilon_value", "0.5"}});
+
+  processor::EpsilonGreedyBandit processor;
+
+  // Act
+  EpsilonGreedyBandit model;
+  const SegmentList segments = model.GetSegments();
+
+  // Assert
+  EXPECT_TRUE(segments.empty());
+}
+
 TEST_F(BatAdsEpsilonGreedyBanditModelTest, GetSegmentsIfNeverProcessed) {
   // Arrange
   SaveAllSegments();

diff --git a/vendor/bat-native-ads/src/bat/ads/internal/segments/segments_json_reader.cc b/vendor/bat-native-ads/src/bat/ads/internal/segments/segments_json_reader.cc
@@ -6,34 +6,33 @@
 #include "bat/ads/internal/segments/segments_json_reader.h"
 
 #include "base/json/json_reader.h"
-#include "base/notreached.h"
 #include "base/values.h"
 #include "third_party/abseil-cpp/absl/types/optional.h"
 
 namespace ads {
 namespace JSONReader {
 
 SegmentList ReadSegments(const std::string& json) {
-  SegmentList segments;
-
   absl::optional<base::Value> value = base::JSONReader::Read(json);
   if (!value) {
-    return segments;
+    return {};
   }
 
   base::ListValue* list = nullptr;
   if (!value->GetAsList(&list)) {
-    return segments;
+    return {};
   }
 
+  SegmentList segments;
   for (const auto& element : list->GetList()) {
     if (!element.is_string()) {
-      NOTREACHED();
-      continue;
+      return {};
     }
 
     const std::string segment = element.GetString();
-    DCHECK(!segment.empty());
+    if (segment.empty()) {
+      return {};
+    }
 
     segments.push_back(segment);
   }