LGTM1 to ship this change with a feature flag which we can use as a kill
switch. Adding use counters so that we can get examples of breakage *if* it
happens would be great too, if it's not too much overhead.
On Tue, Mar 14, 2023 at 11:04 AM Philip Jägenstedt <foo...@chromium.org>
wrote:
> Hi Eli,
>
> Adding to what Jiacheng said, I've tested isValidHost('example!.com') with
> your new code, and it doesn't give the same result in Chrome and Safari.
>
> However, the url.host setter doesn't throw for invalid hosts, instead it
> does nothing. You could start your helper like this:
>
> let url;
> try {
> url = new globalThis.URL('https://' + host);
> } catch (e) {
> return false;
> }
>
> But isValidHost('example!.com') is still not going to get the same result
> in Chrome and Safari, because new URL('https://example!.com') doesn't
> throw in either, but Chrome currently will escape the host as 'example%
> 21.com' while Safari will leave it as 'example!.com'.
>
> If you want something that is guaranteed to work exactly the same in all
> browsers before and after these changes, I think your best bet is to avoid
> the URL constructor/API entirely,
>
> On Tue, Mar 14, 2023 at 7:27 AM Jiacheng Guo <g...@google.com> wrote:
>
>> Hi Eli,
>>
>> The implementation is not fully in line with the spec.
>> Though hosts are not percent encoded by the browser. The browser will
>> percent decode host strings. Thus 'test%22.com' is a valid host string.
>> If all the browsers are spec compliant when parsing the hosts. I believe
>> simply setting url.host and checking for errors will work. (This is not the
>> case now)
>>
>> Jiacheng Guo
>>
>> On Tue, Mar 14, 2023 at 3:05 PM Eli Grey <m...@eligrey.com> wrote:
>>
>>> I've updated my isValidHost() util to support this change. Could someone
>>> please have another look and let me know if my implementation now aligns
>>> well with the spec?
>>>
>>>
>>>
>>> On Monday, March 13, 2023 at 6:26:18 PM UTC-7 dom...@chromium.org wrote:
>>>
>>>> On Mon, Mar 13, 2023 at 6:46 PM Philip Jägenstedt <foo...@chromium.org>
>>>> wrote:
>>>>
>>>>> To simplify and keep this moving, I've filed
>>>>> https://github.com/mozilla/standards-positions/issues/759 as an
>>>>> umbrella issue for anything URL in Interop 2023.
>>>>>
>>>>> My view is that we can't improve our risk assessment of this by much
>>>>> with metrics, because we can't distinguish between harmless and serious
>>>>> breakage. Instead what we should do is take some comfort in the fact that
>>>>> the behavior is already shipping in Safari, try to ship it and revert at
>>>>> the first sign of trouble to evaluate. In other words, I'll happily LGTM
>>>>> this, but I'll send this round of feedback first, in case Yoav disagrees
>>>>> with that.
>>>>>
>>>>> Some additional replies inline:
>>>>>
>>>>> On Mon, Mar 13, 2023 at 5:30 AM Yoav Weiss <yoav...@chromium.org>
>>>>> wrote:
>>>>>
>>>>>> Thanks for working on interop! :)
>>>>>>
>>>>>
>>>>> Indeed, I'm very grateful and happy to see this work!
>>>>>
>>>>> Can you please explain what would be the impact of this change and
>>>>>> provide examples of cases that are currently working and would stop
>>>>>> working
>>>>>> after this change is landed?
>>>>>> Web developers are asking questions on this thread, and it'd be good
>>>>>> to have an explainer that answers such questions.
>>>>>>
>>>>>
>>>>> I've replied to Eli.
>>>>>
>>>>> More generally, since this is a change in the nitty gritty details, my
>>>>> concrete advice for web developers would be to test what Safari currently
>>>>> does and assume that's what Chrome is going to start doing. If one doesn't
>>>>> have access to Safari, then https://www.browserstack.com/screenshots
>>>>> can be used for one-off tests, as long as the test result appears on the
>>>>> page.
>>>>>
>>>>> The other difference to Safari that's easy to test for is when
>>>>> exceptions are thrown. `new URL('https://example|.com')` returns a
>>>>> URL escaped as "https://example%7C.com"; in Chrome, but throws
>>>>> TypeError in Safari.
>>>>>
>>>>
>>>> Developers can also use the whatwg-url Node.js package
>>>> <https://github.com/jsdom/whatwg-url>, including the live URL viewer
>>>> <https://jsdom.github.io/whatwg-url/>. It is kept 1:1 with the URL
>>>> Standard and so reflects the behavior that all browsers will be aiming
>>>> toward as part of Interop 2023 (and that Safari is already compliant with).
>>>> Examples:
>>>>
>>>> - Parsing https://example|.com
>>>>
>>>> <https://jsdom.github.io/whatwg-url/#url=aHR0cHM6Ly9leGFtcGxlfC5jb20=&base=YWJvdXQ6Ymxhbms=>
>>>> - Testing Eli's isValidHost usage of the host setter
>>>> <https://runkit.com/embed/lwn8l7w4yyj5>
>>>>
>>>>
>>>>
>>>>>
>>>>> Do we have use counters for content that would start throwing once
>>>>>> this change lands?
>>>>>>
>>>>>
>>>>> I'll let Jiacheng answer, but if the answer is no, I'm skeptical that
>>>>> adding use counters will meaningfully help us judge the risk of this.
>>>>> Breaking it down:
>>>>>
>>>>> - Many invalid URLs already throw exceptions, which may be caught.
>>>>> Knowing that new exceptions will be thrown in X% of page views will not
>>>>> help know how often those are caught and the web app still behaves
>>>>> correctly.
>>>>> - Changes in serialization are akin to changing a return value. We
>>>>> can't instrument the downstream effects of that and determine if the
>>>>> difference led to a different outcome.
>>>>>
>>>>> Can you provide a link to the tests?
>>>>>>
>>>>>
>>>>>
>>>>> https://wpt.fyi/results/url?label=experimental&label=master&product=chrome&product=firefox&product=safari&aligned&view=interop&q=label%3Ainterop-2023-url
>>>>>
>>>>> There's no way to link to exactly the subtests that will be affected,
>>>>> but "Parsing: <http://example example.com> against <http://other.com/>"
>>>>> in url-constructor.any.html is one of them.
>>>>>
>>>>> Best regards,
>>>>> Philip
>>>>>
>>>>> --
>>>>>
>>>> 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+...@chromium.org.
>>>>>
>>>> To view this discussion on the web visit
>>>>> https://groups.google.com/a/chromium.org/d/msgid/blink-dev/CAARdPYdKrtMS10VJxvzKntoXMBOEaA1doTYqpQ1W4%2BX1q40-bg%40mail.gmail.com
>>>>> <https://groups.google.com/a/chromium.org/d/msgid/blink-dev/CAARdPYdKrtMS10VJxvzKntoXMBOEaA1doTYqpQ1W4%2BX1q40-bg%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/CAARdPYeJaH8%3DFwG7dj9E9RogqKF5AEr5OJA5EaDY%3DMXBVmqgVQ%40mail.gmail.com.
