[tor-commits] [Git][tpo/applications/tor-browser][tor-browser-128.3.0esr-14.0-1] 3 commits: Bug 1906024 - Format download file names better a=diannaS

ma1 (@ma1) git at gitlab.torproject.org
Tue Oct 1 20:23:57 UTC 2024



ma1 pushed to branch tor-browser-128.3.0esr-14.0-1 at The Tor Project / Applications / Tor Browser


Commits:
42b4d421 by rahulsainani at 2024-10-01T17:58:48+02:00
Bug 1906024 - Format download file names better  a=diannaS

Original Revision: https://phabricator.services.mozilla.com/D220559

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

- - - - -
5928c112 by rahulsainani at 2024-10-01T17:58:55+02:00
Bug 1906024 - Format download file names  a=diannaS

Original Revision: https://phabricator.services.mozilla.com/D221771

Differential Revision: https://phabricator.services.mozilla.com/D222259
- - - - -
e139ad27 by Nika Layzell at 2024-10-01T18:01:23+02:00
Bug 1911745 - Unify BrowsingContext flag coherency checks, r=mccr8

Previously these checks were largely diagnostic tools for finding bugs
in other code as it evolves. This unifies the checks a bit more and
makes them stronger for BrowsingContexts created over IPC, providing a
place for more coherency checks to be added in the future.

Differential Revision: https://phabricator.services.mozilla.com/D218860
- - - - -


4 changed files:

- docshell/base/BrowsingContext.cpp
- docshell/base/BrowsingContext.h
- mobile/android/android-components/components/support/ktx/src/main/java/mozilla/components/support/ktx/kotlin/String.kt
- mobile/android/android-components/components/support/ktx/src/test/java/mozilla/components/support/ktx/kotlin/StringTest.kt


Changes:

=====================================
docshell/base/BrowsingContext.cpp
=====================================
@@ -578,9 +578,20 @@ mozilla::ipc::IPCResult BrowsingContext::CreateFromIPC(
   context->mRequestContextId = aInit.mRequestContextId;
   // NOTE: Private browsing ID is set by `SetOriginAttributes`.
 
+  if (const char* failure =
+          context->BrowsingContextCoherencyChecks(aOriginProcess)) {
+    mozilla::ipc::IProtocol* actor = aOriginProcess;
+    if (!actor) {
+      actor = ContentChild::GetSingleton();
+    }
+    return IPC_FAIL_UNSAFE_PRINTF(actor, "Incoherent BrowsingContext: %s",
+                                  failure);
+  }
+
   Register(context);
 
-  return context->Attach(/* aFromIPC */ true, aOriginProcess);
+  context->Attach(/* aFromIPC */ true, aOriginProcess);
+  return IPC_OK();
 }
 
 BrowsingContext::BrowsingContext(WindowContext* aParentWindow,
@@ -795,8 +806,64 @@ void BrowsingContext::Embed() {
   }
 }
 
-mozilla::ipc::IPCResult BrowsingContext::Attach(bool aFromIPC,
-                                                ContentParent* aOriginProcess) {
+const char* BrowsingContext::BrowsingContextCoherencyChecks(
+    ContentParent* aOriginProcess) {
+#define COHERENCY_ASSERT(condition) \
+  if (!(condition)) return "Assertion " #condition " failed";
+
+  if (mGroup->IsPotentiallyCrossOriginIsolated() !=
+      (Top()->GetOpenerPolicy() ==
+       nsILoadInfo::OPENER_POLICY_SAME_ORIGIN_EMBEDDER_POLICY_REQUIRE_CORP)) {
+    return "Invalid CrossOriginIsolated state";
+  }
+
+  if (aOriginProcess && !IsContent()) {
+    return "Content cannot create chrome BCs";
+  }
+
+  // LoadContext should generally match our opener or parent.
+  if (IsContent()) {
+    if (RefPtr<BrowsingContext> opener = GetOpener()) {
+      COHERENCY_ASSERT(opener->mType == mType);
+      COHERENCY_ASSERT(opener->mGroup == mGroup);
+      COHERENCY_ASSERT(opener->mUseRemoteTabs == mUseRemoteTabs);
+      COHERENCY_ASSERT(opener->mUseRemoteSubframes == mUseRemoteSubframes);
+      COHERENCY_ASSERT(opener->mPrivateBrowsingId == mPrivateBrowsingId);
+      COHERENCY_ASSERT(
+          opener->mOriginAttributes.EqualsIgnoringFPD(mOriginAttributes));
+    }
+  }
+  if (RefPtr<BrowsingContext> parent = GetParent()) {
+    COHERENCY_ASSERT(parent->mType == mType);
+    COHERENCY_ASSERT(parent->mGroup == mGroup);
+    COHERENCY_ASSERT(parent->mUseRemoteTabs == mUseRemoteTabs);
+    COHERENCY_ASSERT(parent->mUseRemoteSubframes == mUseRemoteSubframes);
+    COHERENCY_ASSERT(parent->mPrivateBrowsingId == mPrivateBrowsingId);
+    COHERENCY_ASSERT(
+        parent->mOriginAttributes.EqualsIgnoringFPD(mOriginAttributes));
+  }
+
+  // UseRemoteSubframes and UseRemoteTabs must match.
+  if (mUseRemoteSubframes && !mUseRemoteTabs) {
+    return "Cannot set useRemoteSubframes without also setting useRemoteTabs";
+  }
+
+  // Double-check OriginAttributes/Private Browsing
+  // Chrome browsing contexts must not have a private browsing OriginAttribute
+  // Content browsing contexts must maintain the equality:
+  // mOriginAttributes.mPrivateBrowsingId == mPrivateBrowsingId
+  if (IsChrome()) {
+    COHERENCY_ASSERT(mOriginAttributes.mPrivateBrowsingId == 0);
+  } else {
+    COHERENCY_ASSERT(mOriginAttributes.mPrivateBrowsingId ==
+                     mPrivateBrowsingId);
+  }
+#undef COHERENCY_ASSERT
+
+  return nullptr;
+}
+
+void BrowsingContext::Attach(bool aFromIPC, ContentParent* aOriginProcess) {
   MOZ_DIAGNOSTIC_ASSERT(!mEverAttached);
   MOZ_DIAGNOSTIC_ASSERT_IF(aFromIPC, aOriginProcess || XRE_IsContentProcess());
   mEverAttached = true;
@@ -815,25 +882,15 @@ mozilla::ipc::IPCResult BrowsingContext::Attach(bool aFromIPC,
   MOZ_DIAGNOSTIC_ASSERT(mGroup);
   MOZ_DIAGNOSTIC_ASSERT(!mIsDiscarded);
 
-  if (mGroup->IsPotentiallyCrossOriginIsolated() !=
-      (Top()->GetOpenerPolicy() ==
-       nsILoadInfo::OPENER_POLICY_SAME_ORIGIN_EMBEDDER_POLICY_REQUIRE_CORP)) {
-    MOZ_DIAGNOSTIC_ASSERT(aFromIPC);
-    if (aFromIPC) {
-      auto* actor = aOriginProcess
-                        ? static_cast<mozilla::ipc::IProtocol*>(aOriginProcess)
-                        : static_cast<mozilla::ipc::IProtocol*>(
-                              ContentChild::GetSingleton());
-      return IPC_FAIL(
-          actor,
-          "Invalid CrossOriginIsolated state in BrowsingContext::Attach call");
-    } else {
-      MOZ_CRASH(
-          "Invalid CrossOriginIsolated state in BrowsingContext::Attach call");
+#ifdef MOZ_DIAGNOSTIC_ASSERT_ENABLED
+  // We'll already have checked this if `aFromIPC` is set before calling this
+  // function.
+  if (!aFromIPC) {
+    if (const char* failure = BrowsingContextCoherencyChecks(aOriginProcess)) {
+      MOZ_CRASH_UNSAFE_PRINTF("Incoherent BrowsingContext: %s", failure);
     }
   }
-
-  AssertCoherentLoadContext();
+#endif
 
   // Add ourselves either to our parent or BrowsingContextGroup's child list.
   // Important: We shouldn't return IPC_FAIL after this point, since the
@@ -915,7 +972,6 @@ mozilla::ipc::IPCResult BrowsingContext::Attach(bool aFromIPC,
   if (XRE_IsParentProcess()) {
     Canonical()->CanonicalAttach();
   }
-  return IPC_OK();
 }
 
 void BrowsingContext::Detach(bool aFromIPC) {
@@ -1768,40 +1824,6 @@ nsresult BrowsingContext::SetOriginAttributes(const OriginAttributes& aAttrs) {
   return NS_OK;
 }
 
-void BrowsingContext::AssertCoherentLoadContext() {
-#ifdef MOZ_DIAGNOSTIC_ASSERT_ENABLED
-  // LoadContext should generally match our opener or parent.
-  if (IsContent()) {
-    if (RefPtr<BrowsingContext> opener = GetOpener()) {
-      MOZ_DIAGNOSTIC_ASSERT(opener->mType == mType);
-      MOZ_DIAGNOSTIC_ASSERT(opener->mGroup == mGroup);
-      MOZ_DIAGNOSTIC_ASSERT(opener->mUseRemoteTabs == mUseRemoteTabs);
-      MOZ_DIAGNOSTIC_ASSERT(opener->mUseRemoteSubframes == mUseRemoteSubframes);
-      MOZ_DIAGNOSTIC_ASSERT(opener->mPrivateBrowsingId == mPrivateBrowsingId);
-      MOZ_DIAGNOSTIC_ASSERT(
-          opener->mOriginAttributes.EqualsIgnoringFPD(mOriginAttributes));
-    }
-  }
-  if (RefPtr<BrowsingContext> parent = GetParent()) {
-    MOZ_DIAGNOSTIC_ASSERT(parent->mType == mType);
-    MOZ_DIAGNOSTIC_ASSERT(parent->mGroup == mGroup);
-    MOZ_DIAGNOSTIC_ASSERT(parent->mUseRemoteTabs == mUseRemoteTabs);
-    MOZ_DIAGNOSTIC_ASSERT(parent->mUseRemoteSubframes == mUseRemoteSubframes);
-    MOZ_DIAGNOSTIC_ASSERT(parent->mPrivateBrowsingId == mPrivateBrowsingId);
-    MOZ_DIAGNOSTIC_ASSERT(
-        parent->mOriginAttributes.EqualsIgnoringFPD(mOriginAttributes));
-  }
-
-  // UseRemoteSubframes and UseRemoteTabs must match.
-  MOZ_DIAGNOSTIC_ASSERT(
-      !mUseRemoteSubframes || mUseRemoteTabs,
-      "Cannot set useRemoteSubframes without also setting useRemoteTabs");
-
-  // Double-check OriginAttributes/Private Browsing
-  AssertOriginAttributesMatchPrivateBrowsing();
-#endif
-}
-
 void BrowsingContext::AssertOriginAttributesMatchPrivateBrowsing() {
   // Chrome browsing contexts must not have a private browsing OriginAttribute
   // Content browsing contexts must maintain the equality:


=====================================
docshell/base/BrowsingContext.h
=====================================
@@ -984,7 +984,18 @@ class BrowsingContext : public nsILoadContext, public nsWrapperCache {
                                        bool aHasPostData);
 
  private:
-  mozilla::ipc::IPCResult Attach(bool aFromIPC, ContentParent* aOriginProcess);
+  // Assert that this BrowsingContext is coherent relative to related
+  // BrowsingContexts. This will be run before the BrowsingContext is attached.
+  //
+  // A non-null string return value indicates that there was a coherency check
+  // failure, which will be handled with either a crash or IPC failure.
+  //
+  // If provided, `aOriginProcess` is the process which is responsible for the
+  // creation of this BrowsingContext.
+  [[nodiscard]] const char* BrowsingContextCoherencyChecks(
+      ContentParent* aOriginProcess);
+
+  void Attach(bool aFromIPC, ContentParent* aOriginProcess);
 
   // Recomputes whether we can execute scripts in this BrowsingContext based on
   // the value of AllowJavascript() and whether scripts are allowed in the
@@ -998,10 +1009,6 @@ class BrowsingContext : public nsILoadContext, public nsWrapperCache {
 
   void AssertOriginAttributesMatchPrivateBrowsing();
 
-  // Assert that the BrowsingContext's LoadContext flags appear coherent
-  // relative to related BrowsingContexts.
-  void AssertCoherentLoadContext();
-
   friend class ::nsOuterWindowProxy;
   friend class ::nsGlobalWindowOuter;
   friend class WindowContext;


=====================================
mobile/android/android-components/components/support/ktx/src/main/java/mozilla/components/support/ktx/kotlin/String.kt
=====================================
@@ -53,6 +53,8 @@ const val MAX_URI_LENGTH = 25000
 
 private const val FILE_PREFIX = "file://"
 private const val MAX_VALID_PORT = 65_535
+private const val SPACE = " "
+private const val UNDERSCORE = "_"
 
 /**
  * Shortens URLs to be more user friendly.
@@ -316,7 +318,9 @@ fun String.sanitizeFileName(): String {
         file.name.replace("\\.\\.+".toRegex(), ".")
     } else {
         file.name.replace(".", "")
-    }.replaceEscapedCharacters()
+    }.replaceContinuousSpaces()
+        .replaceEscapedCharacters()
+        .trim()
 }
 
 /**
@@ -324,8 +328,16 @@ fun String.sanitizeFileName(): String {
  * and is correctly displayed.
  */
 private fun String.replaceEscapedCharacters(): String {
-    val controlCharactersRegex = "[\\x00-\\x13/*\"?<>:|\\\\]".toRegex()
-    return replace(controlCharactersRegex, "_")
+    val escapedCharactersRegex = "[\\x00-\\x13*\"?<>:|\\\\]".toRegex()
+    return replace(escapedCharactersRegex, UNDERSCORE)
+}
+
+/**
+ * Replaces continuous spaces with a single space.
+ */
+private fun String.replaceContinuousSpaces(): String {
+    val escapedCharactersRegex = "[\\p{Z}\\s]+".toRegex()
+    return replace(escapedCharactersRegex, SPACE)
 }
 
 /**


=====================================
mobile/android/android-components/components/support/ktx/src/test/java/mozilla/components/support/ktx/kotlin/StringTest.kt
=====================================
@@ -199,11 +199,11 @@ class StringTest {
             "acknowledge\u0006signal" to "acknowledge_signal",
             "bell\u0007sound" to "bell_sound",
             "back\u0008space" to "back_space",
-            "horizontal\u0009tab" to "horizontal_tab",
-            "new\u000Aline" to "new_line",
-            "vertical\u000Btab" to "vertical_tab",
-            "form\u000Cfeed" to "form_feed",
-            "return\u000Dcarriage" to "return_carriage",
+            "horizontal\u0009tab" to "horizontal tab",
+            "new\u000Aline" to "new line",
+            "vertical\u000Btab" to "vertical tab",
+            "form\u000Cfeed" to "form feed",
+            "return\u000Dcarriage" to "return carriage",
             "shift\u000Eout" to "shift_out",
             "shift\u000Fin" to "shift_in",
             "escape\u0010data" to "escape_data",



View it on GitLab: https://gitlab.torproject.org/tpo/applications/tor-browser/-/compare/689b068febb15642e667c7f224564533b3156d96...e139ad27acfc22ad49845af4f42358c3c03aff85

-- 
View it on GitLab: https://gitlab.torproject.org/tpo/applications/tor-browser/-/compare/689b068febb15642e667c7f224564533b3156d96...e139ad27acfc22ad49845af4f42358c3c03aff85
You're receiving this email because of your account on gitlab.torproject.org.


-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.torproject.org/pipermail/tor-commits/attachments/20241001/a9dc7a5c/attachment-0001.htm>


More information about the tor-commits mailing list