From 62f83c0a284070ee23ae5f05ff74ce7ce44559af Mon Sep 17 00:00:00 2001 From: Marco Castelluccio Date: Tue, 1 Oct 2019 02:09:50 +0000 Subject: [PATCH] Bug 1342258 - Refactor SetDomain to IsRegistrableDomainSuffixOfOrEqualTo r=smaug This commit refactors the SetDomain method in a Document to call a new function IsRegistrableDomainSuffixOfOrEqualTo(), defined in HTML [1]. This commit tries not to rename anything except input variables, so as to remain as clear as possible. It likely should have various variables renamed, but given the author's unfamiliarity with this module, review seems a good time to do that. It's also duplicating comments a little bit; let me know which one(s) you'd like to keep! Note: Commentary on the HTML change is available in the PR [2], and the rationale for this behavior in Web Auentication, where this algorithm will be used, is also recorded [3]. Updates: Refactored two new protected methods to avoid code duplication. [1] https://html.spec.whatwg.org/multipage/browsers.html#is-a-registrable-domain-suffix-of-or-is-equal-to [2] https://github.com/whatwg/html/pull/2365 [3] https://github.com/w3ctag/spec-reviews/issues/97#issuecomment-175766580 MozReview-Commit-ID: 4Dr8yOMdhez UltraBlame original commit: ffa2f50d49ce0356bbf02ea0940060283dcc007c --- dom/html/nsHTMLDocument.cpp | 134 +++++++++++++++++++++++++----------- dom/html/nsHTMLDocument.h | 6 ++ 2 files changed, 99 insertions(+), 41 deletions(-) diff --git a/dom/html/nsHTMLDocument.cpp b/dom/html/nsHTMLDocument.cpp index 41204743caf1c..1e4ff9e55499d 100644 --- a/dom/html/nsHTMLDocument.cpp +++ b/dom/html/nsHTMLDocument.cpp @@ -906,64 +906,51 @@ nsHTMLDocument::GetDomain(nsAString& aDomain) return NS_OK; } -NS_IMETHODIMP -nsHTMLDocument::SetDomain(const nsAString& aDomain) -{ - ErrorResult rv; - SetDomain(aDomain, rv); - return rv.StealNSResult(); -} - -void -nsHTMLDocument::SetDomain(const nsAString& aDomain, ErrorResult& rv) +already_AddRefed +nsHTMLDocument::CreateInheritingURIForHost(const nsACString& aHostString) { - if (mSandboxFlags & SANDBOXED_DOMAIN) { - - rv.Throw(NS_ERROR_DOM_SECURITY_ERR); - return; - } - - if (aDomain.IsEmpty()) { - rv.Throw(NS_ERROR_DOM_BAD_DOCUMENT_DOMAIN); - return; + if (aHostString.IsEmpty()) { + return nullptr; } nsCOMPtr uri = GetDomainURI(); - if (!uri) { - rv.Throw(NS_ERROR_FAILURE); - return; + return nullptr; } nsCOMPtr newURI; - nsresult rv2 = uri->Clone(getter_AddRefs(newURI)); - if (NS_FAILED(rv2)) { - rv.Throw(rv2); - return; + nsresult rv = uri->Clone(getter_AddRefs(newURI)); + if (NS_FAILED(rv)) { + return nullptr; } - rv2 = newURI->SetUserPass(EmptyCString()); - if (NS_FAILED(rv2)) { - rv.Throw(rv2); - return; + rv = newURI->SetUserPass(EmptyCString()); + if (NS_FAILED(rv)) { + return nullptr; } - rv2 = newURI->SetHostAndPort(NS_ConvertUTF16toUTF8(aDomain)); - if (NS_FAILED(rv2)) { - rv.Throw(rv2); - return; + rv = newURI->SetHostAndPort(aHostString); + if (NS_FAILED(rv)) { + return nullptr; } + return newURI.forget(); +} + +already_AddRefed +nsHTMLDocument::RegistrableDomainSuffixOfInternal(const nsAString& aNewDomain, + nsIURI* aOrigHost) +{ - nsAutoCString current, domain; - if (NS_FAILED(uri->GetAsciiHost(current))) + nsAutoCString domain = NS_ConvertUTF16toUTF8(aNewDomain); + nsAutoCString current; + if (NS_FAILED(aOrigHost->GetAsciiHost(current))) { current.Truncate(); - if (NS_FAILED(newURI->GetAsciiHost(domain))) - domain.Truncate(); + } bool ok = current.Equals(domain); if (current.Length() > domain.Length() && @@ -974,19 +961,84 @@ nsHTMLDocument::SetDomain(const nsAString& aDomain, ErrorResult& rv) nsCOMPtr tldService = do_GetService(NS_EFFECTIVETLDSERVICE_CONTRACTID); if (!tldService) { - rv.Throw(NS_ERROR_NOT_AVAILABLE); - return; + return nullptr; } nsAutoCString currentBaseDomain; - ok = NS_SUCCEEDED(tldService->GetBaseDomain(uri, 0, currentBaseDomain)); + ok = NS_SUCCEEDED(tldService->GetBaseDomain(aOrigHost, 0, currentBaseDomain)); NS_ASSERTION(StringEndsWith(domain, currentBaseDomain) == (domain.Length() >= currentBaseDomain.Length()), "uh-oh! slight optimization wasn't valid somehow!"); ok = ok && domain.Length() >= currentBaseDomain.Length(); } + if (!ok) { + return nullptr; + } + + return CreateInheritingURIForHost(domain); +} + +bool +nsHTMLDocument::IsRegistrableDomainSuffixOfOrEqualTo(const nsAString& aHostSuffixString, + const nsACString& aOrigHost) +{ + + if (aHostSuffixString.IsEmpty()) { + return false; + } + + nsCOMPtr origURI = CreateInheritingURIForHost(aOrigHost); + if (!origURI) { + + return false; + } + + nsCOMPtr newURI = RegistrableDomainSuffixOfInternal(aHostSuffixString, origURI); + if (!newURI) { + + return false; + } + return true; +} + + +NS_IMETHODIMP +nsHTMLDocument::SetDomain(const nsAString& aDomain) +{ + ErrorResult rv; + SetDomain(aDomain, rv); + return rv.StealNSResult(); +} + +void +nsHTMLDocument::SetDomain(const nsAString& aDomain, ErrorResult& rv) +{ + if (mSandboxFlags & SANDBOXED_DOMAIN) { + + rv.Throw(NS_ERROR_DOM_SECURITY_ERR); + return; + } + + if (aDomain.IsEmpty()) { + rv.Throw(NS_ERROR_DOM_BAD_DOCUMENT_DOMAIN); + return; + } + + nsCOMPtr uri = GetDomainURI(); + if (!uri) { + rv.Throw(NS_ERROR_FAILURE); + return; + } + + + + + + nsCOMPtr newURI = RegistrableDomainSuffixOfInternal(aDomain, uri); + if (!newURI) { + rv.Throw(NS_ERROR_DOM_BAD_DOCUMENT_DOMAIN); return; } diff --git a/dom/html/nsHTMLDocument.h b/dom/html/nsHTMLDocument.h index dcc04657d4f4f..2254357627bb7 100644 --- a/dom/html/nsHTMLDocument.h +++ b/dom/html/nsHTMLDocument.h @@ -166,6 +166,8 @@ class nsHTMLDocument : public nsDocument, virtual JSObject* WrapNode(JSContext* aCx, JS::Handle aGivenProto) override; void SetDomain(const nsAString& aDomain, mozilla::ErrorResult& rv); + bool IsRegistrableDomainSuffixOfOrEqualTo(const nsAString& aHostSuffixString, + const nsACString& aOrigHost); void GetCookie(nsAString& aCookie, mozilla::ErrorResult& rv); void SetCookie(const nsAString& aCookie, mozilla::ErrorResult& rv); void NamedGetter(JSContext* cx, const nsAString& aName, bool& aFound, @@ -274,6 +276,10 @@ class nsHTMLDocument : public nsDocument, static void DocumentWriteTerminationFunc(nsISupports *aRef); already_AddRefed GetDomainURI(); + already_AddRefed CreateInheritingURIForHost(const nsACString& aHostString); + already_AddRefed RegistrableDomainSuffixOfInternal(const nsAString& aHostSuffixString, + nsIURI* aOrigHost); + nsresult WriteCommon(JSContext *cx, const nsAString& aText, bool aNewlineTerminate);