commit ae9af25ea51e8fc842492677b81ee38ff17d1fd7
author Maks Orlovich <morlovich@chromium.org> Tue May 13 16:17:00 2025
committer Chromium LUCI CQ <chromium-scoped@luci-project-accounts.iam.gserviceaccount.com> Tue May 13 16:17:00 2025
tree bdd886c8d9e06daac901f9dd1bd4c3684d639a1b
parent 8e385342486d430e7a075e4a758c4c54c3f4b661

Add missing check in browsing_topics::EpochTopics::FromDictValue

base::ValueToTime can fail, so check its return value; OTOH it can
handle a null input just fine. (Which is covered by
EpochTopicsTest.FromEmptyDictionaryValue)

Bug: 328277485
Change-Id: I7ecfcbe4859da61e0d952e727ee9ab092737517c
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6538933
Reviewed-by: Yao Xiao <yaoxia@chromium.org>
Commit-Queue: Maks Orlovich <morlovich@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1459491}

components/browsing_topics/epoch_topics.cc

diff --git a/components/browsing_topics/epoch_topics.cc b/components/browsing_topics/epoch_topics.cc
index ee9c0f8b9..9b133b8 100644
--- a/components/browsing_topics/epoch_topics.cc
+++ b/components/browsing_topics/epoch_topics.cc
@@ -81,11 +81,13 @@
 EpochTopics EpochTopics::FromDictValue(const base::Value::Dict& dict_value) {
   const base::Value* calculation_time_value =
       dict_value.Find(kCalculationTimeNameKey);
-  if (!calculation_time_value)
+  std::optional<base::Time> maybe_calculation_time =
+      base::ValueToTime(calculation_time_value);
+  if (!maybe_calculation_time) {
     return EpochTopics(base::Time());
+  }
 
-  base::Time calculation_time =
-      base::ValueToTime(calculation_time_value).value();
+  base::Time calculation_time = maybe_calculation_time.value();
 
   std::vector<TopicAndDomains> top_topics_and_observing_domains;
   const base::Value::List* top_topics_and_observing_domains_value =

components/browsing_topics/epoch_topics_unittest.cc

diff --git a/components/browsing_topics/epoch_topics_unittest.cc b/components/browsing_topics/epoch_topics_unittest.cc
index ab98faa..4499f8af 100644
--- a/components/browsing_topics/epoch_topics_unittest.cc
+++ b/components/browsing_topics/epoch_topics_unittest.cc
@@ -313,6 +313,15 @@
   EXPECT_FALSE(candidate_topic.IsValid());
 }
 
+TEST_F(EpochTopicsTest, FromDictValueInvalidCalculationTime) {
+  EpochTopics epoch_topics(kCalculationTime);
+
+  base::Value::Dict dict_value = epoch_topics.ToDictValue();
+  dict_value.Set("calculation_time", "nonsense");
+  EpochTopics read_epoch_topics = EpochTopics::FromDictValue(dict_value);
+  EXPECT_EQ(read_epoch_topics.calculation_time(), base::Time());
+}
+
 TEST_F(EpochTopicsTest,
        FromDictionaryValueWithoutConfigVersion_UseConfigVersion1) {
   base::Value::Dict dict;