Summary: | Make WebCore::topPrivatelyControlledDomain() return "localhost" for localhost | ||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | John Wilander <wilander> | ||||||||
Component: | WebCore Misc. | Assignee: | John Wilander <wilander> | ||||||||
Status: | RESOLVED FIXED | ||||||||||
Severity: | Normal | CC: | achristensen, commit-queue, webkit-bug-importer, wilander | ||||||||
Priority: | P2 | Keywords: | InRadar | ||||||||
Version: | WebKit Nightly Build | ||||||||||
Hardware: | Unspecified | ||||||||||
OS: | Unspecified | ||||||||||
See Also: |
https://bugs.webkit.org/show_bug.cgi?id=170763 https://bugs.webkit.org/show_bug.cgi?id=169399 |
||||||||||
Attachments: |
|
Description
John Wilander
2017-04-12 18:00:23 PDT
Created attachment 306959 [details]
Patch
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. Created attachment 307309 [details]
Patch
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 Created attachment 307433 [details]
Patch for landing
(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! Comment on attachment 307433 [details] Patch for landing Clearing flags on attachment: 307433 Committed r215489: <http://trac.webkit.org/changeset/215489> All reviewed patches have been landed. Closing bug. |