Bug 1461590: Lower-case hostnames when adding substitutions. r?smaug draft
authorKris Maglione <maglione.k@gmail.com>
Tue, 15 May 2018 13:02:08 -0700
changeset 795490 a6954046aaf5e6bda2fb7eb829a66ad1782efaef
parent 795449 b51a237fe66d79bbfda2f9e564069831020aca48
push id109994
push usermaglione.k@gmail.com
push dateTue, 15 May 2018 21:27:04 +0000
reviewerssmaug
bugs1461590
milestone62.0a1
Bug 1461590: Lower-case hostnames when adding substitutions. r?smaug 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]