Bug 1978217 - Don't share sheets using relative URIs when it's not safe to do so. r=firefox-style-system-reviewers,layout-reviewers,dshin a=dsmith

Track whether an inline sheet might have uris that depend on the base, and
avoid caching them if appropriate.

Differential Revision: https://phabricator.services.mozilla.com/D258292

layout/style/Loader.cpp

@@ -1648,23 +1648,72 @@ void Loader::MarkLoadTreeFailed(SheetLoadData& aLoadData,
   } while (data);
 }
 
+static bool URIsEqual(nsIURI* aA, nsIURI* aB) {
+  if (aA == aB) {
+    return true;
+  }
+  if (!aA || !aB) {
+    return false;
+  }
+  bool equal = false;
+  return NS_SUCCEEDED(aA->Equals(aB, &equal)) && equal;
+}
+
+// The intention is that this would return true for inputs like
+//   (https://example.com/a, https://example.com/b)
+// but not for:
+//   (https://example.com/a, https://example.com/b/c)
+// where "regular" relative URIs would resolve differently.
+static bool BaseURIsArePathCompatible(nsIURI* aA, nsIURI* aB) {
+  if (!aA || !aB) {
+    return false;
+  }
+  constexpr auto kDummyPath = "foo.css"_ns;
+  nsAutoCString resultA;
+  nsAutoCString resultB;
+  aA->Resolve(kDummyPath, resultA);
+  aB->Resolve(kDummyPath, resultB);
+  return resultA == resultB;
+}
+
+static bool CanReuseInlineSheet(StyleSheet* aSheet, nsIURI* aNewBaseURI) {
+  nsIURI* oldBase = aSheet->GetBaseURI();
+  if (URIsEqual(oldBase, aNewBaseURI)) {
+    return true;
+  }
+  switch (aSheet->OriginalContentsBaseUriDependency()) {
+    case StyleLikelyBaseUriDependency::No:
+      break;
+    case StyleLikelyBaseUriDependency::Path:
+      if (BaseURIsArePathCompatible(oldBase, aNewBaseURI)) {
+        break;
+      }
+    [[fallthrough]];
+    case StyleLikelyBaseUriDependency::Full:
+      LOG(("  Can't reuse due to base URI dependency"));
+      return false;
+  }
+  return true;
+}
+
 RefPtr<StyleSheet> Loader::LookupInlineSheetInCache(
-    const nsAString& aBuffer, nsIPrincipal* aSheetPrincipal) {
+    const nsAString& aBuffer, nsIPrincipal* aSheetPrincipal, nsIURI* aBaseURI) {
   MOZ_ASSERT(mSheets, "Document associated loader should have sheet cache");
   auto result = mSheets->LookupInline(LoaderPrincipal(), aBuffer);
   if (!result) {
     return nullptr;
   }
-
   StyleSheet* sheet = result.Data();
   MOZ_ASSERT(!sheet->HasModifiedRules(),
              "How did we end up with a dirty sheet?");
-
   if (NS_WARN_IF(!sheet->Principal()->Equals(aSheetPrincipal))) {
     // If the sheet is going to have different access rights, don't return it
     // from the cache. XXX can this happen now that we eagerly clone?
     return nullptr;
   }
+  if (!CanReuseInlineSheet(sheet, aBaseURI)) {
+    return nullptr;
+  }
   return sheet->Clone(nullptr, nullptr);
 }
 
@@ -1732,7 +1781,8 @@ Result<Loader::LoadSheetResult, nsresult> Loader::LoadInlineStyle(
     return LoaderPrincipal();
   }();
 
-  RefPtr<StyleSheet> sheet = LookupInlineSheetInCache(aBuffer, sheetPrincipal);
+  RefPtr<StyleSheet> sheet =
+      LookupInlineSheetInCache(aBuffer, sheetPrincipal, baseURI);
   const bool isSheetFromCache = !!sheet;
   if (!isSheetFromCache) {
     sheet = MakeRefPtr<StyleSheet>(eAuthorSheetFeatures, aInfo.mCORSMode,

layout/style/Loader.h

@@ -567,7 +567,8 @@ class Loader final {
       CORSMode aCORSMode, const nsAString& aNonce, const nsAString& aIntegrity,
       uint64_t aEarlyHintPreloaderId, dom::FetchPriority aFetchPriority);
 
-  RefPtr<StyleSheet> LookupInlineSheetInCache(const nsAString&, nsIPrincipal*);
+  RefPtr<StyleSheet> LookupInlineSheetInCache(const nsAString&, nsIPrincipal*,
+                                              nsIURI* aBaseURI);
 
   // Synchronously notify of a cached load data.
   void NotifyOfCachedLoad(RefPtr<SheetLoadData>);

layout/style/StyleSheet.cpp

@@ -1254,6 +1254,20 @@ void StyleSheet::FinishAsyncParse(
   UnblockParsePromise();
 }
 
+StyleLikelyBaseUriDependency StyleSheet::OriginalContentsBaseUriDependency()
+    const {
+  const auto* counters = UseCounters();
+  if (Servo_IsCustomUseCounterRecorded(
+          counters, StyleCustomUseCounter::MaybeHasFullBaseUriDependency)) {
+    return StyleLikelyBaseUriDependency::Full;
+  }
+  if (Servo_IsCustomUseCounterRecorded(
+          counters, StyleCustomUseCounter::MaybeHasPathBaseUriDependency)) {
+    return StyleLikelyBaseUriDependency::Path;
+  }
+  return StyleLikelyBaseUriDependency::No;
+}
+
 const StyleUseCounters* StyleSheet::UseCounters() const {
   return Servo_StyleSheet_UseCounters(RawContents());
 }

layout/style/StyleSheet.h

@@ -41,6 +41,7 @@ using StyleSheetParsePromise = MozPromise</* Dummy */ bool,
                                           /* IsExclusive = */ true>;
 
 enum class StyleRuleChangeKind : uint32_t;
+enum class StyleLikelyBaseUriDependency : uint8_t;
 
 struct StyleRuleChange {
   StyleRuleChange() = delete;
@@ -159,6 +160,9 @@ class StyleSheet final : public nsICSSLoaderObserver, public nsWrapperCache {
   const StyleUseCounters* UseCounters() const;
   void PropagateUseCountersTo(dom::Document*) const;
 
+  // Whether our original contents may be using relative URIs.
+  StyleLikelyBaseUriDependency OriginalContentsBaseUriDependency() const;
+
   URLExtraData* URLData() const { return Inner().mURLData; }
 
   // nsICSSLoaderObserver interface

servo/components/style/gecko/url.rs

@@ -59,6 +59,52 @@ impl PartialEq for CssUrlData {
     }
 }
 
+/// How an URI might depend on our base URI.
+///
+/// TODO(emilio): See if necko can provide this, but for our case local refs or empty URIs are
+/// totally fine even though they wouldn't be in general...
+#[repr(u8)]
+pub enum LikelyBaseUriDependency {
+    /// No dependency on the base URI (either absolute uri, or relative URI).
+    No,
+    /// We might depend on our path depth. E.g. `https://example.com/foo` and
+    /// `https://example.com/bar` both resolve a relative URI like `baz.css` as
+    /// `https://example.com/baz.css`.
+    Path,
+    /// We might depend on our full URI. This is the case for query strings (and, in the general
+    /// case, local refs and empty URIs, but that's not the case for CSS as described below.
+    Full,
+}
+
+fn likely_base_uri_dependency(specified: &str) -> LikelyBaseUriDependency {
+    if specified.is_empty() {
+        // In CSS the empty URL is special / invalid.
+        // https://drafts.csswg.org/css-values-4/#url-empty
+        return LikelyBaseUriDependency::No;
+    }
+    if specified.starts_with('#') || specified.starts_with('/') {
+        // Local refs and absolute paths are fair game.
+        return LikelyBaseUriDependency::No;
+    }
+    const COMMON_PROTOCOLS: [&str; 3] = [
+        "http:",
+        "https:",
+        "data:",
+    ];
+    for protocol in COMMON_PROTOCOLS {
+        if specified.starts_with(protocol) {
+            // Common absolute URIs.
+            return LikelyBaseUriDependency::No;
+        }
+    }
+    if specified.starts_with('?') {
+        // Query string resolves differently for any two different base URIs
+        return LikelyBaseUriDependency::Full;
+    }
+    // Might be a relative URI, play it safe.
+    LikelyBaseUriDependency::Path
+}
+
 impl CssUrl {
     /// Parse a URL with a particular CORS mode.
     pub fn parse_with_cors_mode<'i, 't>(
@@ -76,6 +122,21 @@ impl CssUrl {
 
     /// Parse a URL from a string value that is a valid CSS token for a URL.
     pub fn parse_from_string(url: String, context: &ParserContext, cors_mode: CorsMode) -> Self {
+        use crate::use_counters::CustomUseCounter;
+        if let Some(counters) = context.use_counters {
+            if !counters.custom.recorded(CustomUseCounter::MaybeHasFullBaseUriDependency) {
+                match likely_base_uri_dependency(&url) {
+                    LikelyBaseUriDependency::No => {},
+                    LikelyBaseUriDependency::Path => {
+                        counters.custom.record(CustomUseCounter::MaybeHasPathBaseUriDependency);
+                    },
+                    LikelyBaseUriDependency::Full => {
+                        counters.custom.record(CustomUseCounter::MaybeHasPathBaseUriDependency);
+                        counters.custom.record(CustomUseCounter::MaybeHasFullBaseUriDependency);
+                    },
+                }
+            }
+        }
         CssUrl(Arc::new(CssUrlData {
             serialization: url.into(),
             extra_data: context.url_data.clone(),

servo/components/style/use_counters/mod.rs

@@ -30,6 +30,10 @@ pub struct NonCustomPropertyUseCounters {
 #[derive(Copy, Clone, Debug)]
 #[repr(u32)]
 pub enum CustomUseCounter {
+    /// Whether we are likely to be using relative URIs that depend on our path depth.
+    MaybeHasPathBaseUriDependency = 0,
+    /// Whether we are likely to be using relative URIs that depend on our full URI.
+    MaybeHasFullBaseUriDependency,
     /// Dummy value, used for indexing purposes.
     Last,
 }

servo/ports/geckolib/cbindgen.toml

@@ -175,6 +175,7 @@ include = [
   "Overflow",
   "LengthPercentage",
   "LetterSpacing",
+  "LikelyBaseUriDependency",
   "NonNegativeLengthPercentage",
   "LengthPercentageOrAuto",
   "LineHeight",