Bug 1461590: Lower-case hostnames when adding substitutions. r=smaug f=dveditz
authorKris Maglione <maglione.k@gmail.com>
Tue, 15 May 2018 13:02:08 -0700
changeset 418449 3c014aea6022
parent 418448 6ddc3b915c34
child 418450 2404129f5672
push id34001
push userebalazs@mozilla.com
push dateWed, 16 May 2018 10:01:23 +0000
treeherdermozilla-central@3c9d69736f4a [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerssmaug
bugs1461590
milestone62.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1461590: Lower-case hostnames when adding substitutions. r=smaug f=dveditz Since URI hostnames are defined to be case-insensitive, we only ever see lower-case hostnames when looking up substitutions. That means that substitutions containing capital letters are inaccessible, which is a footgun that has hit many people. The handler should lower-case substitutions when they're added so that look-ups are always case-insensitive. MozReview-Commit-ID: C936hS2cSyY
netwerk/protocol/res/SubstitutingProtocolHandler.cpp
netwerk/protocol/res/nsISubstitutingProtocolHandler.idl
netwerk/test/unit/test_substituting_protocol_handler.js
netwerk/test/unit/xpcshell.ini
--- a/netwerk/protocol/res/SubstitutingProtocolHandler.cpp
+++ b/netwerk/protocol/res/SubstitutingProtocolHandler.cpp
@@ -9,16 +9,17 @@
 #include "mozilla/Unused.h"
 
 #include "SubstitutingProtocolHandler.h"
 #include "nsIChannel.h"
 #include "nsIIOService.h"
 #include "nsIFile.h"
 #include "nsNetCID.h"
 #include "nsNetUtil.h"
+#include "nsReadableUtils.h"
 #include "nsURLHelper.h"
 #include "nsEscape.h"
 
 using mozilla::dom::ContentParent;
 
 namespace mozilla {
 namespace net {
 
@@ -301,18 +302,21 @@ nsresult
 SubstitutingProtocolHandler::SetSubstitution(const nsACString& root, nsIURI *baseURI)
 {
   // Add-ons use this API but they should not be able to make anything
   // content-accessible.
   return SetSubstitutionWithFlags(root, baseURI, 0);
 }
 
 nsresult
-SubstitutingProtocolHandler::SetSubstitutionWithFlags(const nsACString& root, nsIURI *baseURI, uint32_t flags)
+SubstitutingProtocolHandler::SetSubstitutionWithFlags(const nsACString& origRoot, nsIURI *baseURI, uint32_t flags)
 {
+  nsAutoCString root;
+  ToLowerCase(origRoot, root);
+
   if (!baseURI) {
     mSubstitutions.Remove(root);
     NotifyObservers(root, baseURI);
     return SendSubstitution(root, baseURI, flags);
   }
 
   // If baseURI isn't a same-scheme URI, we can set the substitution immediately.
   nsAutoCString scheme;
@@ -344,49 +348,62 @@ SubstitutingProtocolHandler::SetSubstitu
   SubstitutionEntry& entry = mSubstitutions.GetOrInsert(root);
   entry.baseURI = newBaseURI;
   entry.flags = flags;
   NotifyObservers(root, baseURI);
   return SendSubstitution(root, newBaseURI, flags);
 }
 
 nsresult
-SubstitutingProtocolHandler::GetSubstitution(const nsACString& root, nsIURI **result)
+SubstitutingProtocolHandler::GetSubstitution(const nsACString& origRoot, nsIURI **result)
 {
   NS_ENSURE_ARG_POINTER(result);
 
+  nsAutoCString root;
+  ToLowerCase(origRoot, root);
+
   SubstitutionEntry entry;
   if (mSubstitutions.Get(root, &entry)) {
     nsCOMPtr<nsIURI> baseURI = entry.baseURI;
     baseURI.forget(result);
     return NS_OK;
   }
 
   uint32_t flags;
   return GetSubstitutionInternal(root, result, &flags);
 }
 
 nsresult
 SubstitutingProtocolHandler::GetSubstitutionFlags(const nsACString& root, uint32_t* flags)
 {
+#ifdef DEBUG
+  nsAutoCString lcRoot;
+  ToLowerCase(root, lcRoot);
+  MOZ_ASSERT(root.Equals(lcRoot), "GetSubstitutionFlags should never receive mixed-case root name");
+#endif
+
   *flags = 0;
   SubstitutionEntry entry;
   if (mSubstitutions.Get(root, &entry)) {
     *flags = entry.flags;
     return NS_OK;
   }
 
   nsCOMPtr<nsIURI> baseURI;
   return GetSubstitutionInternal(root, getter_AddRefs(baseURI), flags);
 }
 
 nsresult
-SubstitutingProtocolHandler::HasSubstitution(const nsACString& root, bool *result)
+SubstitutingProtocolHandler::HasSubstitution(const nsACString& origRoot, bool *result)
 {
   NS_ENSURE_ARG_POINTER(result);
+
+  nsAutoCString root;
+  ToLowerCase(origRoot, root);
+
   *result = HasSubstitution(root);
   return NS_OK;
 }
 
 nsresult
 SubstitutingProtocolHandler::ResolveURI(nsIURI *uri, nsACString &result)
 {
   nsresult rv;
--- a/netwerk/protocol/res/nsISubstitutingProtocolHandler.idl
+++ b/netwerk/protocol/res/nsISubstitutingProtocolHandler.idl
@@ -20,18 +20,18 @@ interface nsISubstitutingProtocolHandler
   const short ALLOW_CONTENT_ACCESS = 1;
 
   /**
    * Sets the substitution for the root key:
    *   resource://root/path ==> baseURI.resolve(path)
    *
    * A null baseURI removes the specified substitution.
    *
-   * A root key should always be lowercase; however, this may not be
-   * enforced.
+   * The root key will be converted to lower-case to conform to
+   * case-insensitive URI hostname matching behavior.
    */
   [must_use] void setSubstitution(in ACString root, in nsIURI baseURI);
 
   /**
    * Same as setSubstitution, but with specific flags.
    */
   [must_use] void setSubstitutionWithFlags(in ACString root, in nsIURI baseURI, in uint32_t flags);
 
new file mode 100644
--- /dev/null
+++ b/netwerk/test/unit/test_substituting_protocol_handler.js
@@ -0,0 +1,41 @@
+"use strict";
+
+ChromeUtils.import("resource://gre/modules/Services.jsm");
+
+add_task(async function test_case_insensitive_substitutions() {
+  let resProto = Services.io.getProtocolHandler("resource")
+    .QueryInterface(Ci.nsISubstitutingProtocolHandler);
+
+  let uri = Services.io.newFileURI(do_get_file("data"));
+
+  resProto.setSubstitution("FooBar", uri);
+  resProto.setSubstitutionWithFlags("BarBaz", uri, 0);
+
+  equal(resProto.resolveURI(Services.io.newURI("resource://foobar/")),
+        uri.spec, "Got correct resolved URI for setSubstitution");
+
+  equal(resProto.resolveURI(Services.io.newURI("resource://foobar/")),
+        uri.spec, "Got correct resolved URI for setSubstitutionWithFlags");
+
+  ok(resProto.hasSubstitution("foobar"), "hasSubstitution works with all-lower-case root");
+  ok(resProto.hasSubstitution("FooBar"), "hasSubstitution works with mixed-case root");
+
+  equal(resProto.getSubstitution("foobar").spec, uri.spec,
+        "getSubstitution works with all-lower-case root");
+  equal(resProto.getSubstitution("FooBar").spec, uri.spec,
+        "getSubstitution works with mixed-case root");
+
+  resProto.setSubstitution("foobar", null);
+  resProto.setSubstitution("barbaz", null);
+
+  Assert.throws(() => resProto.resolveURI(Services.io.newURI("resource://foobar/")),
+                e => e.result == Cr.NS_ERROR_NOT_AVAILABLE,
+                "Correctly unregistered case-insensitive substitution in setSubstitution");
+  Assert.throws(() => resProto.resolveURI(Services.io.newURI("resource://barbaz/")),
+                e => e.result == Cr.NS_ERROR_NOT_AVAILABLE,
+                "Correctly unregistered case-insensitive substitution in setSubstitutionWithFlags");
+
+  Assert.throws(() => resProto.getSubstitution("foobar"),
+                e => e.result == Cr.NS_ERROR_NOT_AVAILABLE,
+                "foobar substitution has been removed");
+});
--- a/netwerk/test/unit/xpcshell.ini
+++ b/netwerk/test/unit/xpcshell.ini
@@ -416,8 +416,9 @@ skip-if = os == "android"
 [test_header_Server_Timing.js]
 # Test requires http/2, and http/2 server doesn't run on android.
 skip-if = os == "android"
 run-sequentially = node server exceptions dont replay well
 [test_trr.js]
 # http2-using tests require node available
 skip-if = os == "android"
 [test_ioservice.js]
+[test_substituting_protocol_handler.js]