Bug 170798 – Make WebCore::topPrivatelyControlledDomain() return "localhost" for localhost

• Status: RESOLVED
• Resolution: FIXED
• Component: WebCore Misc.

[John Wilander]

WebCore::topPrivatelyControlledDomain() is used for partitioning and should return "localhost" if the host domain is "localhost".

[Radar WebKit Bug Importer]

<rdar://problem/31595108>

[John Wilander]

Created attachment 306959 [details]
Patch

[Alex Christensen]

Comment on attachment 306959 [details]
Patch

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

> Source/WebCore/platform/mac/PublicSuffixMac.mm:48
> +    if ([domain _web_looksLikeIPAddress] || [domain isEqualToString:@"localhost"])

Let's do a case-insensitive comparison and add a test for LocalHost.

[John Wilander]

Created attachment 307309 [details]
Patch

[Alex Christensen]

Comment on attachment 307309 [details]
Patch

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

> Source/WebCore/platform/mac/PublicSuffixMac.mm:54
> +    const auto& lowercaseDomain = domain.convertToASCIILowercase();

I guess punycode encoding should not happen here.  This seems correct.

> Tools/TestWebKitAPI/Tests/mac/PublicSuffix.mm:64
> +    EXPECT_FALSE(isPublicSuffix(""));

Is this the UTF-8 encoding if an uppercase non-ASCII character?  Careful when committing, because sometimes our tools like webkit-patch UTF-8 encode non-ASCII characters.  svn  commit doesn't change the characters.
We should probably include a test with an uppercase non-ASCII character and a test with a lowercase non-ASCII character just for fun.

> Tools/TestWebKitAPI/Tests/mac/PublicSuffix.mm:90
> +    EXPECT_EQ(String("example.com"), topPrivatelyControlledDomain("ExamPle.com"));

And let's add a test with a.b.subdomain.Example.com

[John Wilander]

Created attachment 307433 [details]
Patch for landing

[John Wilander]

(In reply to Alex Christensen from comment #5)
> Comment on attachment 307309 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=307309&action=review
> 
> > Source/WebCore/platform/mac/PublicSuffixMac.mm:54
> > +    const auto& lowercaseDomain = domain.convertToASCIILowercase();
> 
> I guess punycode encoding should not happen here.  This seems correct.
> 
> > Tools/TestWebKitAPI/Tests/mac/PublicSuffix.mm:64
> > +    EXPECT_FALSE(isPublicSuffix(""));
> 
> Is this the UTF-8 encoding if an uppercase non-ASCII character?  Careful
> when committing, because sometimes our tools like webkit-patch UTF-8 encode
> non-ASCII characters.  svn  commit doesn't change the characters.
> We should probably include a test with an uppercase non-ASCII character and
> a test with a lowercase non-ASCII character just for fun.
> 
> > Tools/TestWebKitAPI/Tests/mac/PublicSuffix.mm:90
> > +    EXPECT_EQ(String("example.com"), topPrivatelyControlledDomain("ExamPle.com"));
> 
> And let's add a test with a.b.subdomain.Example.com

All done. Thanks, Alex!

[WebKit Commit Bot]

Comment on attachment 307433 [details]
Patch for landing

Clearing flags on attachment: 307433

Committed r215489: <http://trac.webkit.org/changeset/215489>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.