[webkit-reviews] review denied: [Bug 158121] Implement W3C Secure Contexts Draft Specification : [Attachment 312095] Part 2: Implement Secure Contexts

[bugzilla-daemon at webkit.org]

Alex Christensen <achristensen at apple.com> has denied Daniel Bates
<dbates at webkit.org>'s request for review:
Bug 158121: Implement W3C Secure Contexts Draft Specification
https://bugs.webkit.org/show_bug.cgi?id=158121

Attachment 312095: Part 2: Implement Secure Contexts

https://bugs.webkit.org/attachment.cgi?id=312095&action=review




--- Comment #27 from Alex Christensen <achristensen at apple.com> ---
Comment on attachment 312095
  --> https://bugs.webkit.org/attachment.cgi?id=312095
Part 2: Implement Secure Contexts

View in context: https://bugs.webkit.org/attachment.cgi?id=312095&action=review

> Source/WebCore/page/SecurityOrigin.cpp:113
> +    if (url.hostType() == URL::HostType::IPv6Address && url.host() == "::1")

I think this is supposed to be "[::1]" which leads me to believe that this is
not tested.

> Source/WebCore/platform/URL.h:228
> +    HostType m_hostType { HostType::Domain };

Let's not add more member variables to the URL.  We are trying to work towards
reducing the number of member variables in the URL object.  Instead we should
make a function that checks URL.host() for a valid 127.*.*.* IPv4 address or an
IPv6 address that is [::1].

static bool isPotentiallyTrustworthy(const URL& url)
{
    if (!url.isValid())
	return false;
    auto host = url.host();
    if (host == "[::1]")
	return true;

    // Check to see if it's a valid IPv4 address in 127.*.*.*
    if (!host.startsWith("127."))
	return false;
    size_t dotsFound = 0;
    for (size_t i = 0; i < host.length(); ++i) {
	if (host[i] == '.') {
	    dotsFound++;
	    continue;
	}
	if (!isASCIIDigit(host[i]))
	    return false;
    }
    return dotsFound == 3;
}