Move maxTrustedScoringSignalsURLLength to non-shared param in auction…

… config. Transfer the maximum length limit for fetching trusted scoring signals URL to the non-shared auction configuration. Github: WICG/turtledove#767 Bug: 324416012 Change-Id: I470563353b348e4dd1766c911c93a48d1befb2f6 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5301695 Reviewed-by: Ken Buchanan <kenrb@chromium.org> Reviewed-by: mmenke <mmenke@chromium.org> Commit-Queue: Tianyang Xu <xtlsheep@google.com> Cr-Commit-Position: refs/heads/main@{#1261269}
chromium · Feb 15, 2024 · 58f74fb · 58f74fb
1 parent 217caaf
commit 58f74fb
Show file tree

Hide file tree

Showing 10 changed files with 100 additions and 27 deletions.
diff --git a/content/browser/interest_group/interest_group_browsertest.cc b/content/browser/interest_group/interest_group_browsertest.cc
@@ -5333,6 +5333,60 @@ IN_PROC_BROWSER_TEST_F(InterestGroupBrowserTest,
   WaitForAccessObserved({});
 }
 
+IN_PROC_BROWSER_TEST_F(InterestGroupBrowserTest,
+                       RunAdAuctionPositiveMaxTrustedScoringSignalsURLLength) {
+  GURL url = embedded_https_test_server().GetURL("a.test", "/echo");
+  url::Origin origin = url::Origin::Create(url);
+  ASSERT_TRUE(NavigateToURL(shell(), url));
+  AttachInterestGroupObserver();
+
+  EXPECT_EQ(nullptr, RunAuctionAndWait(JsReplace(R"({
+      seller: $1,
+      decisionLogicURL: $2,
+      maxTrustedScoringSignalsURLLength: 1000
+  })",
+                                                 origin, url)));
+  WaitForAccessObserved({});
+}
+
+IN_PROC_BROWSER_TEST_F(InterestGroupBrowserTest,
+                       RunAdAuctionZeroMaxTrustedScoringSignalsURLLength) {
+  GURL url = embedded_https_test_server().GetURL("a.test", "/echo");
+  url::Origin origin = url::Origin::Create(url);
+  ASSERT_TRUE(NavigateToURL(shell(), url));
+  AttachInterestGroupObserver();
+
+  EXPECT_EQ(nullptr, RunAuctionAndWait(JsReplace(R"({
+      seller: $1,
+      decisionLogicURL: $2,
+      maxTrustedScoringSignalsURLLength: 0
+  })",
+                                                 origin, url)));
+  WaitForAccessObserved({});
+}
+
+IN_PROC_BROWSER_TEST_F(InterestGroupBrowserTest,
+                       RunAdAuctionNegativeMaxTrustedScoringSignalsURLLength) {
+  GURL url = embedded_https_test_server().GetURL("a.test", "/echo");
+  url::Origin origin = url::Origin::Create(url);
+  ASSERT_TRUE(NavigateToURL(shell(), url));
+  AttachInterestGroupObserver();
+
+  EXPECT_EQ(
+      base::StringPrintf(
+          "TypeError: Failed to execute 'runAdAuction' on 'Navigator': "
+          "maxTrustedScoringSignalsURLLength '-1000' for AuctionAdConfig with "
+          "seller '%s' must not be negative.",
+          origin.Serialize().c_str()),
+      RunAuctionAndWait(JsReplace(R"({
+      seller: $1,
+      decisionLogicURL: $2,
+      maxTrustedScoringSignalsURLLength: -1000
+  })",
+                                  origin, url)));
+  WaitForAccessObserved({});
+}
+
 // TODO(crbug.com/1441988): Remove test when old names are no longer supported.
 IN_PROC_BROWSER_TEST_F(
     InterestGroupBrowserTest,

diff --git a/content/test/data/fuzzer_corpus/ad_auction_service_mojolpm_fuzzer/basic_auction.textproto b/content/test/data/fuzzer_corpus/ad_auction_service_mojolpm_fuzzer/basic_auction.textproto
@@ -139,10 +139,10 @@ actions {
               }
               m_component_auctions {
               }
+              m_max_trusted_scoring_signals_url_length: 0
             }
           }
 
-          m_max_trusted_scoring_signals_url_length: 0
           m_direct_from_seller_signals {
             old: 1
           }

diff --git a/third_party/blink/common/interest_group/auction_config.cc b/third_party/blink/common/interest_group/auction_config.cc
@@ -471,8 +471,6 @@ base::Value::Dict AuctionConfig::SerializeForDevtools() const {
   SerializeIntoDict("decisionLogicURL", decision_logic_url, result);
   SerializeIntoDict("trustedScoringSignalsURL", trusted_scoring_signals_url,
                     result);
-  SerializeIntoDict("maxTrustedScoringSignalsURLLength",
-                    max_trusted_scoring_signals_url_length, result);
   SerializeIntoDict("deprecatedRenderURLReplacements",
                     deprecated_render_url_replacements, result);
   SerializeIntoDict("interestGroupBuyers",
@@ -530,6 +528,10 @@ base::Value::Dict AuctionConfig::SerializeForDevtools() const {
     result.Set("componentAuctions", std::move(component_auctions));
   }
 
+  SerializeIntoDict("maxTrustedScoringSignalsURLLength",
+                    non_shared_params.max_trusted_scoring_signals_url_length,
+                    result);
+
   // direct_from_seller_signals --- skipped.
   SerializeIntoDict("expectsDirectFromSellerSignalsHeaderAdSlot",
                     expects_direct_from_seller_signals_header_ad_slot, result);

diff --git a/third_party/blink/common/interest_group/auction_config_mojom_traits.cc b/third_party/blink/common/interest_group/auction_config_mojom_traits.cc
@@ -219,6 +219,13 @@ bool StructTraits<blink::mojom::AuctionAdConfigNonSharedParamsDataView,
     return false;
   }
 
+  // Negative length limit is invalid.
+  if (data.max_trusted_scoring_signals_url_length() < 0) {
+    return false;
+  }
+  out->max_trusted_scoring_signals_url_length =
+      data.max_trusted_scoring_signals_url_length();
+
   out->all_buyers_group_limit = data.all_buyers_group_limit();
 
   // Only one of `interest_group_buyers` or `component_auctions` may be
@@ -303,13 +310,6 @@ bool StructTraits<blink::mojom::AuctionAdConfigDataView, blink::AuctionConfig>::
     return false;
   }
 
-  // Negative length limit is invalid.
-  if (data.max_trusted_scoring_signals_url_length() < 0) {
-    return false;
-  }
-  out->max_trusted_scoring_signals_url_length =
-      data.max_trusted_scoring_signals_url_length();
-
   out->expects_additional_bids = data.expects_additional_bids();
   // An auction that expects additional bids must have an auction nonce provided
   // on the config.

diff --git a/third_party/blink/common/interest_group/auction_config_mojom_traits_test.cc b/third_party/blink/common/interest_group/auction_config_mojom_traits_test.cc
@@ -324,6 +324,19 @@ TEST(AuctionConfigMojomTraitsTest, DuplicateAllSlotsRequestedSizes) {
   EXPECT_FALSE(SerializeAndDeserialize(auction_config));
 }
 
+TEST(AuctionConfigMojomTraitsTest, MaxTrustedScoringSignalsUrlLength) {
+  AuctionConfig auction_config = CreateBasicAuctionConfig();
+  auction_config.non_shared_params.max_trusted_scoring_signals_url_length =
+      8000;
+  EXPECT_TRUE(SerializeAndDeserialize(auction_config));
+
+  auction_config.non_shared_params.max_trusted_scoring_signals_url_length = 0;
+  EXPECT_TRUE(SerializeAndDeserialize(auction_config));
+
+  auction_config.non_shared_params.max_trusted_scoring_signals_url_length = -1;
+  EXPECT_FALSE(SerializeAndDeserialize(auction_config));
+}
+
 TEST(AuctionConfigMojomTraitsTest,
      DirectFromSellerSignalsPrefixWithQueryString) {
   AuctionConfig auction_config = CreateFullAuctionConfig();

diff --git a/third_party/blink/common/interest_group/auction_config_test_util.cc b/third_party/blink/common/interest_group/auction_config_test_util.cc
@@ -26,7 +26,6 @@ AuctionConfig CreateFullAuctionConfig() {
   AuctionConfig auction_config = CreateBasicAuctionConfig();
 
   auction_config.trusted_scoring_signals_url = GURL("https://seller.test/bar");
-  auction_config.max_trusted_scoring_signals_url_length = 2560;
   auction_config.seller_experiment_group_id = 1;
   auction_config.all_buyer_experiment_group_id = 2;
 
@@ -113,6 +112,7 @@ AuctionConfig CreateFullAuctionConfig() {
       SellerCapabilities::kLatencyStats};
 
   non_shared_params.auction_nonce = base::Uuid::GenerateRandomV4();
+  non_shared_params.max_trusted_scoring_signals_url_length = 2560;
 
   DirectFromSellerSignalsSubresource
       direct_from_seller_signals_per_buyer_signals_buyer;

diff --git a/third_party/blink/public/common/interest_group/auction_config.h b/third_party/blink/public/common/interest_group/auction_config.h
@@ -368,6 +368,11 @@ struct BLINK_COMMON_EXPORT AuctionConfig {
     // Nested auctions whose results will also be fed to `seller`. Only the top
     // level auction config can have component auctions.
     std::vector<AuctionConfig> component_auctions;
+
+    // The maximum length limit for the trusted scoring signal fetch URL. Can
+    // only be set as either 0 or a positive number. A value of 0 indicates that
+    // there is no limit.
+    int32_t max_trusted_scoring_signals_url_length = 0;
   };
 
   AuctionConfig();
@@ -409,11 +414,6 @@ struct BLINK_COMMON_EXPORT AuctionConfig {
   MaybePromiseDeprecatedRenderURLReplacements
       deprecated_render_url_replacements;
 
-  // The maximum length limit for the trusted scoring signal fetch URL. Can
-  // only be set as either 0 or a positive number. A value of 0 indicates that
-  // there is no limit.
-  int32_t max_trusted_scoring_signals_url_length = 0;
-
   // Other parameters are grouped in a struct that is passed to SellerWorklets.
   NonSharedParams non_shared_params;
 
@@ -454,6 +454,8 @@ If modifying AuctionConfig fields, please make sure to also modify:
 * Mojo serialization in:
     third_party/blink/public/common/interest_group/auction_config_mojom_traits.h
     third_party/blink/common/interest_group/auction_config_mojom_traits.cc
+* Fuzzer test in:
+    content/test/data/fuzzer_corpus/ad_auction_service_mojolpm_fuzzer/basic_auction.textproto
 * NumPromises() if it's a Promise.
 * SerializeForDevtools()
 * Add some non-trivial values for the type into CreateFullAuctionConfig() in

diff --git a/third_party/blink/public/common/interest_group/auction_config_mojom_traits.h b/third_party/blink/public/common/interest_group/auction_config_mojom_traits.h
@@ -410,6 +410,11 @@ struct BLINK_COMMON_EXPORT
     return params.component_auctions;
   }
 
+  static int32_t max_trusted_scoring_signals_url_length(
+      const blink::AuctionConfig::NonSharedParams& params) {
+    return params.max_trusted_scoring_signals_url_length;
+  }
+
   static bool Read(blink::mojom::AuctionAdConfigNonSharedParamsDataView data,
                    blink::AuctionConfig::NonSharedParams* out);
 };
@@ -436,11 +441,6 @@ struct BLINK_COMMON_EXPORT
     return config.trusted_scoring_signals_url;
   }
 
-  static int32_t max_trusted_scoring_signals_url_length(
-      const blink::AuctionConfig& config) {
-    return config.max_trusted_scoring_signals_url_length;
-  }
-
   static const blink::AuctionConfig::NonSharedParams&
   auction_ad_config_non_shared_params(const blink::AuctionConfig& config) {
     return config.non_shared_params;

diff --git a/third_party/blink/public/mojom/interest_group/interest_group_types.mojom b/third_party/blink/public/mojom/interest_group/interest_group_types.mojom
@@ -492,6 +492,11 @@ struct AuctionAdConfigNonSharedParams {
   // Nested auctions whose results will also be fed to `seller`. Only the top
   // level auction config can have component auctions.
   array<AuctionAdConfig> component_auctions;
+
+  // The maximum length limit for the trusted scoring signal fetch URL. Can
+  // only be set as either 0 or a positive number. A value of 0 indicates
+  // that there is no limit.
+  int32 max_trusted_scoring_signals_url_length = 0;
 };
 
 // Configuration to pass to RunAdAuction().
@@ -521,11 +526,6 @@ struct AuctionAdConfig {
   // origin with `seller`.
   url.mojom.Url? trusted_scoring_signals_url;
 
-  // The maximum length limit for the trusted scoring signal fetch URL. Can
-  // only be set as either 0 or a positive number. A value of 0 indicates
-  // that there is no limit.
-  int32 max_trusted_scoring_signals_url_length = 0;
-
   // Other parameters are grouped in a struct that is passed to SellerWorklets.
   AuctionAdConfigNonSharedParams auction_ad_config_non_shared_params;
 

diff --git a/third_party/blink/renderer/modules/ad_auction/navigator_auction.cc b/third_party/blink/renderer/modules/ad_auction/navigator_auction.cc
@@ -1355,9 +1355,11 @@ bool CopyMaxTrustedScoringSignalsURLLengthFromIdlToMojo(
         input, "maxTrustedScoringSignalsURLLength",
         String::Number(input.maxTrustedScoringSignalsURLLength()),
         "must not be negative."));
+    return false;
   }
 
-  output.max_trusted_scoring_signals_url_length =
+  output.auction_ad_config_non_shared_params
+      ->max_trusted_scoring_signals_url_length =
       input.maxTrustedScoringSignalsURLLength();
   return true;
 }