Bug 1800628 - Don't hold nsIDNService::mLock while iterating label ch…

…aracters r=necko-reviewers,kershaw Differential Revision: https://phabricator.services.mozilla.com/D204106
dothq · Mar 12, 2024 · ba56b10 · ba56b10
1 parent ce9712e
commit ba56b10
Show file tree

Hide file tree

Showing 2 changed files with 30 additions and 25 deletions.
diff --git a/netwerk/dns/nsIDNService.cpp b/netwerk/dns/nsIDNService.cpp
@@ -675,16 +675,20 @@ enum ScriptCombo : int32_t {
 }  // namespace mozilla::net
 
 bool nsIDNService::isLabelSafe(const nsAString& label, const nsAString& tld) {
-  AutoReadLock lock(mLock);
+  restrictionProfile profile{eASCIIOnlyProfile};
+  {
+    AutoReadLock lock(mLock);
 
-  if (!isOnlySafeChars(PromiseFlatString(label), mIDNBlocklist)) {
-    return false;
-  }
+    if (!isOnlySafeChars(PromiseFlatString(label), mIDNBlocklist)) {
+      return false;
+    }
 
-  // We should never get here if the label is ASCII
-  NS_ASSERTION(!IsAscii(label), "ASCII label in IDN checking");
-  if (mRestrictionProfile == eASCIIOnlyProfile) {
-    return false;
+    // We should never get here if the label is ASCII
+    NS_ASSERTION(!IsAscii(label), "ASCII label in IDN checking");
+    if (mRestrictionProfile == eASCIIOnlyProfile) {
+      return false;
+    }
+    profile = mRestrictionProfile;
   }
 
   nsAString::const_iterator current, end;
@@ -719,7 +723,7 @@ bool nsIDNService::isLabelSafe(const nsAString& label, const nsAString& tld) {
     Script script = UnicodeProperties::GetScriptCode(ch);
     if (script != Script::COMMON && script != Script::INHERITED &&
         script != lastScript) {
-      if (illegalScriptCombo(script, savedScript)) {
+      if (illegalScriptCombo(profile, script, savedScript)) {
         return false;
       }
     }
@@ -884,7 +888,8 @@ static const ScriptCombo scriptComboTable[13][9] = {
     /* KORE */ {FAIL, FAIL, FAIL, KORE, KORE, FAIL, FAIL, KORE, FAIL},
     /* HNLT */ {CHNA, FAIL, FAIL, KORE, HNLT, JPAN, JPAN, HNLT, FAIL}};
 
-bool nsIDNService::illegalScriptCombo(Script script, ScriptCombo& savedScript) {
+bool nsIDNService::illegalScriptCombo(restrictionProfile profile, Script script,
+                                      ScriptCombo& savedScript) {
   if (savedScript == ScriptCombo::UNSET) {
     savedScript = findScriptIndex(script);
     return false;
@@ -899,7 +904,6 @@ bool nsIDNService::illegalScriptCombo(Script script, ScriptCombo& savedScript) {
    * In the Moderately Restrictive profile Latin mixed with any other
    *  single script is allowed.
    */
-  return ((savedScript == OTHR &&
-           mRestrictionProfile == eHighlyRestrictiveProfile) ||
+  return ((savedScript == OTHR && profile == eHighlyRestrictiveProfile) ||
           savedScript == FAIL);
 }
diff --git a/netwerk/dns/nsIDNService.h b/netwerk/dns/nsIDNService.h
@@ -138,6 +138,17 @@ class nsIDNService final : public nsIIDNService {
   bool isLabelSafe(const nsAString& label, const nsAString& tld)
       MOZ_EXCLUDES(mLock);
 
+  /**
+   * Restriction-level Detection profiles defined in UTR 39
+   * http://www.unicode.org/reports/tr39/#Restriction_Level_Detection,
+   * and selected by the pref network.IDN.restriction_profile
+   */
+  enum restrictionProfile {
+    eASCIIOnlyProfile,
+    eHighlyRestrictiveProfile,
+    eModeratelyRestrictiveProfile
+  };
+
   /**
    * Determine whether a combination of scripts in a single label is
    * permitted according to the algorithm defined in UTR 39 and the
@@ -152,9 +163,9 @@ class nsIDNService final : public nsIIDNService {
    * For the "Moderately restrictive" profile, Latin is also allowed
    *  with other scripts except Cyrillic and Greek
    */
-  bool illegalScriptCombo(mozilla::intl::Script script,
-                          mozilla::net::ScriptCombo& savedScript)
-      MOZ_REQUIRES_SHARED(mLock);
+  bool illegalScriptCombo(restrictionProfile profile,
+                          mozilla::intl::Script script,
+                          mozilla::net::ScriptCombo& savedScript);
 
   /**
    * Convert a DNS label from ASCII to Unicode using IDNA2008
@@ -177,16 +188,6 @@ class nsIDNService final : public nsIIDNService {
   // guarded by mLock
   nsTArray<mozilla::net::BlocklistRange> mIDNBlocklist MOZ_GUARDED_BY(mLock);
 
-  /**
-   * Restriction-level Detection profiles defined in UTR 39
-   * http://www.unicode.org/reports/tr39/#Restriction_Level_Detection,
-   * and selected by the pref network.IDN.restriction_profile
-   */
-  enum restrictionProfile {
-    eASCIIOnlyProfile,
-    eHighlyRestrictiveProfile,
-    eModeratelyRestrictiveProfile
-  };
   // guarded by mLock;
   restrictionProfile mRestrictionProfile MOZ_GUARDED_BY(mLock){
       eASCIIOnlyProfile};