LGTM3

First of all, thank you for working on this!

I think adding a use counter is probably too much overhead for this change,
but I took a look [1] in httparchive for the strings 'http://[::'
and 'https://[::' and got 123 and 2 matches respectively. Among the top
ranked sites are https://www.cheapcaribbean.com/ and https://redketchup.io/
with code like this:

var aa = document.createElement("a");
aa.href = "http://[::1]";;
var ah = "[::1]" === aa.hostname;

I "randomly" checked a few other sites too and they were all this pattern.

In all of Chrome, Firefox and Safari `ah` will be true, so I don't think
this change is a risk for this pattern. It looks like "http://[::1]";
and "http://[::1.2]"; aren't tested in WPT, only the same with a trailing
dot. If you think it would make sense to test those to ensure the same
behavior, that would be great.

[1] The query was:

SELECT
  rank,
  page,
  url
FROM
  `httparchive.all.requests`
JOIN
  `httparchive.all.pages`
USING
  (date, client, page)
WHERE
  date = '2023-01-01' AND
  client = 'desktop' AND
  is_main_document AND
  STRPOS(response_body, 'http://[::') > 0
ORDER BY
  rank,
  page

On Wed, Feb 15, 2023 at 5:51 PM Daniel Bratell <bratel...@gmail.com> wrote:

> LGTM2
>
> /Daniel
> On 2023-02-15 17:51, Chris Harrelson wrote:
>
> LGTM1
>
> On Wed, Feb 15, 2023 at 6:38 AM Yoav Weiss <yoavwe...@chromium.org> wrote:
>
>> Any update on the other questions I asked above?
>>
>> On Mon, Feb 13, 2023 at 3:31 PM Jiacheng Guo <g...@google.com> wrote:
>>
>>> The implementation and the feature has been updated with the feature
>>> flag StrictIPv4EmbeddedIPv6AddressParsing.
>>>
>>> Thanks for the advice.
>>>
>>> On Thu, Feb 9, 2023 at 12:15 AM Yoav Weiss <yoavwe...@chromium.org>
>>> wrote:
>>>
>>>>
>>>>
>>>> On Tue, Feb 7, 2023 at 6:56 AM 'Jiacheng Guo' via blink-dev <
>>>> blink-dev@chromium.org> wrote:
>>>>
>>>>> Contact emails g...@google.com
>>>>>
>>>>> Explainer This is an implementation of an established standard.
>>>>>
>>>>> Specification https://url.spec.whatwg.org/#concept-ipv6-parser
>>>>>
>>>>> Summary
>>>>>
>>>>> The behavior of parsing IPv4 embedded IPv6 host parser will be updated
>>>>> to strictly follow the web URL standard:
>>>>> https://url.spec.whatwg.org/#concept-ipv6-parser The introduced
>>>>> restrictions on the IPv6 address are: * The embedded IPv4 address shall
>>>>> always consist of 4 parts. Addresses with less than 4 parts like 
>>>>> http://[::1.2]
>>>>> will be no longer valid. * Embedded IPv4 addresses with trailing dots like
>>>>> http://[::1.2.3.4.] will be no longer valid. The feature is a part of
>>>>> the URL interop 2023.
>>>>>
>>>>>
>>>>> Blink component Blink>Network
>>>>> <https://bugs.chromium.org/p/chromium/issues/list?q=component:Blink%3ENetwork>
>>>>>
>>>>> TAG review Not required for the URL standard.
>>>>>
>>>>> TAG review status Not applicable
>>>>>
>>>>> Risks
>>>>>
>>>>>
>>>>> Interoperability and Compatibility
>>>>>
>>>>> The URL standard is a well-established standard and the fix is a part
>>>>> of the Interop. No interoperability risk is expected. Shortened IPv4
>>>>> addresses embedded in IPv6 are rarely used. Compatibility risk shall be
>>>>> minimal.
>>>>>
>>>>
>>>> How many such URLs do we see? Any use counters? (or another form of
>>>> risk analysis)
>>>>
>>>>
>>>>>
>>>>>
>>>>> *Gecko*: Shipped/Shipping (
>>>>> https://wpt.fyi/results/url/url-constructor.any.html%3Fexclude%3D)
>>>>> The IPv6 parser in Safari has already forced the check.
>>>>>
>>>>
>>>> You mean Gecko?
>>>>
>>>>
>>>>>
>>>>> *WebKit*: Shipped/Shipping (
>>>>> https://wpt.fyi/results/url/url-constructor.any.html%3Fexclude%3D)
>>>>> The IPv6 parser in Safari has already forced the check.
>>>>>
>>>>> *Web developers*: No signals
>>>>>
>>>>> *Other signals*:
>>>>>
>>>>> WebView application risks
>>>>>
>>>>> Does this intent deprecate or change behavior of existing APIs, such
>>>>> that it has potentially high risk for Android WebView-based applications?
>>>>>
>>>>>
>>>>> Debuggability
>>>>>
>>>>> Invalid URLs will be reported in devtools.
>>>>>
>>>>> Will this feature be supported on all six Blink platforms (Windows,
>>>>> Mac, Linux, Chrome OS, Android, and Android WebView)? Yes
>>>>>
>>>>> Is this feature fully tested by web-platform-tests
>>>>> <https://chromium.googlesource.com/chromium/src/+/main/docs/testing/web_platform_tests.md>
>>>>> ? Yes
>>>>> Under
>>>>> https://wpt.fyi/results/url/url-constructor.any.html%3Fexclude%3D
>>>>>
>>>>> Flag name
>>>>> Not under a flag.
>>>>>
>>>>
>>>> You probably want to put such a change behind a base feature flag, to
>>>> enable turning it off in case of unanticipated breakage.
>>>>
>>>>
>>>>>
>>>>> Requires code in //chrome? False
>>>>>
>>>>> Tracking bug https://crbug.com/1411619
>>>>>
>>>>> Sample links
>>>>> https://chromium-review.googlesource.com/c/chromium/src/+/4206417
>>>>>
>>>>> Estimated milestones
>>>>>
>>>>> M113
>>>>>
>>>>> Anticipated spec changes
>>>>>
>>>>> No spec change
>>>>>
>>>>>
>>>>> Link to entry on the Chrome Platform Status
>>>>> https://chromestatus.com/feature/5184515301965824
>>>>>
>>>>> Links to previous Intent discussions
>>>>>
>>>>> This intent message was generated by Chrome Platform Status
>>>>> <https://chromestatus.com/>.
>>>>> --
>>>>> You received this message because you are subscribed to the Google
>>>>> Groups "blink-dev" group.
>>>>> To unsubscribe from this group and stop receiving emails from it, send
>>>>> an email to blink-dev+unsubscr...@chromium.org.
>>>>> To view this discussion on the web visit
>>>>> https://groups.google.com/a/chromium.org/d/msgid/blink-dev/CAJQw1NxUtU8Wns3TYrEQZGQbWQNhNzKm41xYsfv0CKxSO_AngA%40mail.gmail.com
>>>>> <https://groups.google.com/a/chromium.org/d/msgid/blink-dev/CAJQw1NxUtU8Wns3TYrEQZGQbWQNhNzKm41xYsfv0CKxSO_AngA%40mail.gmail.com?utm_medium=email&utm_source=footer>
>>>>> .
>>>>>
>>>> --
>> You received this message because you are subscribed to the Google Groups
>> "blink-dev" group.
>> To unsubscribe from this group and stop receiving emails from it, send an
>> email to blink-dev+unsubscr...@chromium.org.
>> To view this discussion on the web visit
>> https://groups.google.com/a/chromium.org/d/msgid/blink-dev/CAL5BFfWezQAhPhZ3sGjpndjbOGZhaAavV6mo5-boWspQRh3L9g%40mail.gmail.com
>> <https://groups.google.com/a/chromium.org/d/msgid/blink-dev/CAL5BFfWezQAhPhZ3sGjpndjbOGZhaAavV6mo5-boWspQRh3L9g%40mail.gmail.com?utm_medium=email&utm_source=footer>
>> .
>>
> --
> You received this message because you are subscribed to the Google Groups
> "blink-dev" group.
> To unsubscribe from this group and stop receiving emails from it, send an
> email to blink-dev+unsubscr...@chromium.org.
> To view this discussion on the web visit
> https://groups.google.com/a/chromium.org/d/msgid/blink-dev/CAOMQ%2Bw93hg1jF-mQFixOWfSc%2BOXbGkQkEBA4RNrdKKgoGoMziQ%40mail.gmail.com
> <https://groups.google.com/a/chromium.org/d/msgid/blink-dev/CAOMQ%2Bw93hg1jF-mQFixOWfSc%2BOXbGkQkEBA4RNrdKKgoGoMziQ%40mail.gmail.com?utm_medium=email&utm_source=footer>
> .
>
> --
> You received this message because you are subscribed to the Google Groups
> "blink-dev" group.
> To unsubscribe from this group and stop receiving emails from it, send an
> email to blink-dev+unsubscr...@chromium.org.
> To view this discussion on the web visit
> https://groups.google.com/a/chromium.org/d/msgid/blink-dev/88fd4a00-3fac-a315-7756-dd767bd8b294%40gmail.com
> <https://groups.google.com/a/chromium.org/d/msgid/blink-dev/88fd4a00-3fac-a315-7756-dd767bd8b294%40gmail.com?utm_medium=email&utm_source=footer>
> .
>

-- 
You received this message because you are subscribed to the Google Groups 
"blink-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to blink-dev+unsubscr...@chromium.org.
To view this discussion on the web visit 
https://groups.google.com/a/chromium.org/d/msgid/blink-dev/CAARdPYdv5xr2kK0AVQL-JL4WSKZ%2B5ciPfVtT6mD5rujKmv5qXw%40mail.gmail.com.

Reply via email to