Fixes possible crash when parsing NTT Sponsored Images component reso…

…urce
brave · Oct 24, 2024 · 0d6b61d · 0d6b61d
1 parent 5983171
commit 0d6b61d
Show file tree

Hide file tree

Showing 3 changed files with 59 additions and 2 deletions.
diff --git a/components/ntp_background_images/browser/ntp_background_images_service.h b/components/ntp_background_images/browser/ntp_background_images_service.h
@@ -80,6 +80,8 @@ class NTPBackgroundImagesService {
   FRIEND_TEST_ALL_PREFIXES(NTPBackgroundImagesServiceTest, InternalDataTest);
   FRIEND_TEST_ALL_PREFIXES(NTPBackgroundImagesServiceTest,
                            MultipleCampaignsTest);
+  FRIEND_TEST_ALL_PREFIXES(NTPBackgroundImagesServiceTest,
+                           SponsoredImageWithMissingImageUrlTest);
   FRIEND_TEST_ALL_PREFIXES(NTPBackgroundImagesServiceTest,
                            WithDefaultReferralCodeTest1);
   FRIEND_TEST_ALL_PREFIXES(NTPBackgroundImagesServiceTest,

diff --git a/components/ntp_background_images/browser/ntp_background_images_service_unittest.cc b/components/ntp_background_images/browser/ntp_background_images_service_unittest.cc
@@ -121,6 +121,34 @@ constexpr char kTestSponsoredImagesWithMultipleCampaigns[] = R"(
         ]
     })";
 
+constexpr char kTestSponsoredImagesWithMissingImageUrl[] = R"(
+    {
+        "schemaVersion": 1,
+        "campaignId": "fb7ee174-5430-4fb9-8e97-29bf14e8d828",
+        "logo": {
+          "imageUrl":  "logo.png",
+          "alt": "Technikke: For music lovers",
+          "destinationUrl": "https://www.brave.com/",
+          "companyName": "Technikke"
+        },
+        "wallpapers": [
+            {
+              "missing_imageUrl": "background-1.jpg",
+              "focalPoint": { "x": 696, "y": 691 }
+            },
+            {
+              "missing_imageUrl": "background-2.jpg",
+              "creativeInstanceId": "c0d61af3-3b85-4af4-a3cc-cf1b3dd40e70",
+              "logo": {
+                "imageUrl": "logo-2.png",
+                "alt": "logo2",
+                "companyName": "BAT",
+                "destinationUrl": "https://www.bat.com/"
+              }
+            }
+        ]
+    })";
+
 constexpr char kTestBackgroundImages[] = R"(
     {
       "schemaVersion": 1,
@@ -460,6 +488,29 @@ TEST_F(NTPBackgroundImagesServiceTest, MultipleCampaignsTest) {
   service_->RemoveObserver(&observer);
 }
 
+TEST_F(NTPBackgroundImagesServiceTest, SponsoredImageWithMissingImageUrlTest) {
+  Init();
+  TestObserver observer;
+  service_->AddObserver(&observer);
+
+  pref_service_.SetBoolean(kReferralCheckedForPromoCodeFile, true);
+  pref_service_.SetBoolean(kReferralInitialization, true);
+
+  observer.si_data_ = nullptr;
+  service_->si_images_data_.reset();
+  observer.on_si_updated_ = false;
+  service_->OnGetSponsoredComponentJsonData(
+      false, kTestSponsoredImagesWithMissingImageUrl);
+  // Mark this is not SR to get SI data.
+  service_->MarkThisInstallIsNotSuperReferralForever();
+
+  auto* si_data = service_->GetBrandedImagesData(false);
+  EXPECT_FALSE(si_data);
+  EXPECT_TRUE(observer.on_si_updated_);
+  EXPECT_TRUE(observer.si_data_->campaigns.empty());
+  EXPECT_TRUE(service_->si_images_data_->campaigns.empty());
+}
+
 #if BUILDFLAG(IS_LINUX)
 
 // Linux doesn't support referral service now.

diff --git a/components/ntp_background_images/browser/ntp_sponsored_images_data.cc b/components/ntp_background_images/browser/ntp_sponsored_images_data.cc
@@ -228,9 +228,13 @@ Campaign NTPSponsoredImagesData::GetCampaignFromValue(
   if (auto* wallpapers = value.FindList(kWallpapersKey)) {
     for (const auto& entry : *wallpapers) {
       const auto& wallpaper = entry.GetDict();
+      const std::string* image_url = wallpaper.FindString(kImageURLKey);
+      if (!image_url) {
+        continue;
+      }
+
       SponsoredBackground background;
-      background.image_file =
-          installed_dir.AppendASCII(*wallpaper.FindString(kImageURLKey));
+      background.image_file = installed_dir.AppendASCII(*image_url);
 
       if (auto* focal_point = wallpaper.FindDict(kWallpaperFocalPointKey)) {
         background.focal_point = {focal_point->FindInt(kXKey).value_or(0),