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
--- 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]