Bug 945240 - Add a pref to be able to restore previous behaviour where nsIURI.host/.spec returned unicode instead of punycode r=mcmanus draft
authorValentin Gosu <valentin.gosu@gmail.com>
Tue, 11 Jul 2017 19:10:10 +0200
changeset 607140 15f1baf164e7f732b6b55c04290533e1c8266b6c
parent 607139 2461e4a1b33bd1a24e75f174b55db30ce033cfc7
child 607141 18cd23a849830ed715573b544d086a1306dbe53c
push id67902
push uservalentin.gosu@gmail.com
push dateTue, 11 Jul 2017 21:59:36 +0000
reviewersmcmanus
bugs945240
milestone56.0a1
Bug 945240 - Add a pref to be able to restore previous behaviour where nsIURI.host/.spec returned unicode instead of punycode r=mcmanus This is to deal with possible bugs or web-compat issues that may arrise. Also fixes GetDisplayHostPort which would not return the correct brackets for IPv6 addresses MozReview-Commit-ID: 3OfiBDND5Cs * * * [mq]: fix_ipv6_hostport.patch MozReview-Commit-ID: 3VYCwlt7IGT
modules/libpref/init/all.js
netwerk/base/nsStandardURL.cpp
netwerk/base/nsStandardURL.h
--- a/modules/libpref/init/all.js
+++ b/modules/libpref/init/all.js
@@ -1987,16 +1987,20 @@ pref("network.dns.offline-localhost", tr
 
 // The maximum allowed length for a URL - 1MB default
 pref("network.standard-url.max-length", 1048576);
 
 // The preference controls if the rust URL parser is run in parallel with the
 // C++ implementation. Requires restart for changes to take effect.
 pref("network.standard-url.enable-rust", false);
 
+// Whether nsIURI.host/.hostname/.spec should return a punycode string
+// If set to false we will revert to previous behaviour and return a unicode string.
+pref("network.standard-url.punycode-host", true);
+
 // Idle timeout for ftp control connections - 5 minute default
 pref("network.ftp.idleConnectionTimeout", 300);
 
 // directory listing format
 // 2: HTML
 // 3: XUL directory viewer
 // all other values are treated like 2
 pref("network.dir.format", 2);
--- a/netwerk/base/nsStandardURL.cpp
+++ b/netwerk/base/nsStandardURL.cpp
@@ -135,16 +135,17 @@ namespace mozilla {
 namespace net {
 
 static NS_DEFINE_CID(kThisImplCID, NS_THIS_STANDARDURL_IMPL_CID);
 static NS_DEFINE_CID(kStandardURLCID, NS_STANDARDURL_CID);
 
 nsIIDNService *nsStandardURL::gIDN = nullptr;
 bool nsStandardURL::gInitialized = false;
 char nsStandardURL::gHostLimitDigits[] = { '/', '\\', '?', '#', 0 };
+bool nsStandardURL::gPunycodeHost = true;
 
 // Invalid host characters
 // We still allow % because it is in the ID of addons.
 // Any percent encoded ASCII characters that are not allowed in the
 // hostname are not percent decoded, and will be parsed just fine.
 //
 // Note that the array below will be initialized at compile time,
 // so we do not need to "optimize" TestForInvalidHostCharacters.
@@ -354,16 +355,18 @@ nsStandardURL::InitGlobalObjects()
     nsCOMPtr<nsIPrefBranch> prefBranch( do_GetService(NS_PREFSERVICE_CONTRACTID) );
     if (prefBranch) {
         nsCOMPtr<nsIObserver> obs( new nsPrefObserver() );
 #ifdef MOZ_RUST_URLPARSE
         prefBranch->AddObserver(NS_NET_PREF_ENABLE_RUST, obs.get(), false);
 #endif
         PrefsChanged(prefBranch, nullptr);
     }
+
+    Preferences::AddBoolVarCache(&gPunycodeHost, "network.standard-url.punycode-host", true);
 }
 
 void
 nsStandardURL::ShutdownGlobalObjects()
 {
     NS_IF_RELEASE(gIDN);
 
 #ifdef DEBUG_DUMP_URLS_AT_SHUTDOWN
@@ -1336,19 +1339,24 @@ NS_INTERFACE_MAP_END
 //----------------------------------------------------------------------------
 
 // result may contain unescaped UTF-8 characters
 NS_IMETHODIMP
 nsStandardURL::GetSpec(nsACString &result)
 {
     MOZ_ASSERT(mSpec.Length() <= (uint32_t) net_GetURLMaxLength(),
                "The spec should never be this long, we missed a check.");
-    result = mSpec;
+    nsresult rv = NS_OK;
+    if (gPunycodeHost) {
+        result = mSpec;
+    } else { // XXX: This code path may be slow
+        rv = GetDisplaySpec(result);
+    }
     CALL_RUST_GETTER_STR(result, GetSpec, result);
-    return NS_OK;
+    return rv;
 }
 
 // result may contain unescaped UTF-8 characters
 NS_IMETHODIMP
 nsStandardURL::GetSensitiveInfoHiddenSpec(nsACString &result)
 {
     nsresult rv = GetSpec(result);
     if (NS_FAILED(rv)) {
@@ -1390,20 +1398,31 @@ nsStandardURL::GetDisplaySpec(nsACString
     }
 
     return NS_OK;
 }
 
 NS_IMETHODIMP
 nsStandardURL::GetDisplayHostPort(nsACString &aUnicodeHostPort)
 {
-    nsresult rv = GetDisplayHost(aUnicodeHostPort);
+    nsAutoCString unicodeHostPort;
+
+    nsresult rv = GetDisplayHost(unicodeHostPort);
     if (NS_FAILED(rv)) {
         return rv;
     }
+
+    if (StringBeginsWith(Hostport(), NS_LITERAL_CSTRING("["))) {
+        aUnicodeHostPort.AssignLiteral("[");
+        aUnicodeHostPort.Append(unicodeHostPort);
+        aUnicodeHostPort.AppendLiteral("]");
+    } else {
+        aUnicodeHostPort.Assign(unicodeHostPort);
+    }
+
     uint32_t pos = mHost.mPos + mHost.mLen;
     if (pos < mPath.mPos)
         aUnicodeHostPort += Substring(mSpec, pos, mPath.mPos - pos);
 
     return NS_OK;
 }
 
 NS_IMETHODIMP
@@ -1482,25 +1501,35 @@ nsStandardURL::GetPassword(nsACString &r
     result = Password();
     CALL_RUST_GETTER_STR(result, GetPassword, result);
     return NS_OK;
 }
 
 NS_IMETHODIMP
 nsStandardURL::GetHostPort(nsACString &result)
 {
-    nsresult rv = GetAsciiHostPort(result);
+    nsresult rv;
+    if (gPunycodeHost) {
+        rv = GetAsciiHostPort(result);
+    } else {
+        rv = GetDisplayHostPort(result);
+    }
     CALL_RUST_GETTER_STR(result, GetHostPort, result);
     return rv;
 }
 
 NS_IMETHODIMP
 nsStandardURL::GetHost(nsACString &result)
 {
-    nsresult rv = GetAsciiHost(result);
+    nsresult rv;
+    if (gPunycodeHost) {
+        rv = GetAsciiHost(result);
+    } else {
+        rv = GetDisplayHost(result);
+    }
     CALL_RUST_GETTER_STR(result, GetHost, result);
     return rv;
 }
 
 NS_IMETHODIMP
 nsStandardURL::GetPort(int32_t *result)
 {
     // should never be more than 16 bit
--- a/netwerk/base/nsStandardURL.h
+++ b/netwerk/base/nsStandardURL.h
@@ -298,16 +298,17 @@ private:
                                    // mDisplayHost has a been initialized, or
                                    // that the hostname is not punycode
 
     // global objects.  don't use COMPtr as its destructor will cause a
     // coredump if we leak it.
     static nsIIDNService               *gIDN;
     static char                         gHostLimitDigits[];
     static bool                         gInitialized;
+    static bool                         gPunycodeHost;
 
 public:
 #ifdef DEBUG_DUMP_URLS_AT_SHUTDOWN
     void PrintSpec() const { printf("  %s\n", mSpec.get()); }
 #endif
 
 #ifdef MOZ_RUST_URLPARSE
     static bool                        gRustEnabled;