Summary: | Improve IPv6 detection when setting host/hostname | ||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Rob Buis <rbuis> | ||||||||||||
Component: | New Bugs | Assignee: | Rob Buis <rbuis> | ||||||||||||
Status: | RESOLVED FIXED | ||||||||||||||
Severity: | Normal | CC: | benjamin, cdumez, changseok, cmarcelo, darin, esprehn+autocc, ews-watchlist, gyuyoung.kim, webkit-bug-importer | ||||||||||||
Priority: | P2 | Keywords: | InRadar | ||||||||||||
Version: | Safari Technology Preview | ||||||||||||||
Hardware: | Unspecified | ||||||||||||||
OS: | Unspecified | ||||||||||||||
Attachments: |
|
Description
Rob Buis
2020-07-11 02:43:26 PDT
Created attachment 404049 [details]
Patch
Created attachment 404054 [details]
Patch
Comment on attachment 404054 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=404054&action=review Looks good, but I am not sure it’s perfect. > Source/WTF/wtf/URL.cpp:464 > + if (hostName.find(':') != notFound && !hostName.startsWith('[')) find != notFound is the same thing as contains; contains is preferred. Unless you meant to say != colonIndex here? If you did, then do we have enough test coverage, because somehow we missed that error? > Source/WebCore/html/URLDecomposition.cpp:110 > + size_t ipv6separator = value.reverseFind(']'); ipv6separator should not be all lowercase. I think the "s" should be capitalized. Created attachment 404093 [details]
Patch
Created attachment 404094 [details]
Patch
Comment on attachment 404054 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=404054&action=review >> Source/WTF/wtf/URL.cpp:464 >> + if (hostName.find(':') != notFound && !hostName.startsWith('[')) > > find != notFound is the same thing as contains; contains is preferred. > > Unless you meant to say != colonIndex here? If you did, then do we have enough test coverage, because somehow we missed that error? Yeah, contains is better. But I do not want != colonIndex here, since the hostName has been adjusted to not include the ':' separator for the port in the line above. >> Source/WebCore/html/URLDecomposition.cpp:110 >> + size_t ipv6separator = value.reverseFind(']'); > > ipv6separator should not be all lowercase. I think the "s" should be capitalized. Done. Created attachment 404105 [details]
Patch
Committed r264282: <https://trac.webkit.org/changeset/264282> All reviewed patches have been landed. Closing bug and clearing flags on attachment 404105 [details]. |