Intent to Implement and Ship: Make URL parser to not decode percent-encoded ASCII character in URL's path

[Hayato Ito]

Contact emails

hay...@chromium.org

Specification

https://url.spec.whatwg.org/

Summary

Make URL parser to not decode percent-encoded ASCII characters in URL's path, such as "%41" ('A'). Before this change: > const url = new URL("http://example.com/%41"); > url.href "http://example.com/A" After this change: > const url = new URL("http://example.com/%41"); > url.href "http://example.com/%41"

Blink component

Blink>Network

TAG review

None

TAG review status

Not applicable

Risks

Interoperability and Compatibility

Gecko: Shipped/Shipping

WebKit: Shipped/Shipping

There are risks. Please see the WIP CL's description for details (https://crrev.com/c/4607744).

I'd like to collect feedback about possible risks widely through this thread.

The usage (Canary): 0.000106% (URL.Path.UnescapeEscapedChar).

This usage is not specific to any particular use case and can be considered a theoretical upper bound. The actual breakage is likely much lower than this number.

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?

Yes

Tracking bug

https://crbug.com/1252531

Link to entry on the Chrome Platform Status

https://chromestatus.com/feature/6389236793606144

--

Hayato

[TAMURA, Kent]

LGTM1.  It seems to have very low risk.

--
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/CAFpjS_2-4PAY47VbDdd%2BHS%2BchmNUc9dW3BsRtW33LDr1QOeLGw%40mail.gmail.com.

--

TAMURA, Kent
Software Engineer, Google

[Mike Taylor]

I'm having a hard time assessing the risk, despite the very low usage (it has doubled since the original email was sent - but still very low) and other browsers shipping it.

That said, LGTM2 % having a base::Feature we can use as a killswitch, in case we discover something we didn't anticipate.

To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/blink-dev/CAGH7WqHcCModrUAQ%3DGJx-oiLcEmBwi%2BjU0ONCpnNWh%3Dp_THRdg%40mail.gmail.com.

[Daniel Bratell]

Compatibility is nice.

LGTM3, but note Mike's request

/Daniel

To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/blink-dev/b674e07c-7804-41d5-b8f5-7ada6295651b%40chromium.org.

[Hayato Ito]

Thanks for the LGTMs. I appreciate it.

Let me implement and ship with a feature flag so that we can have a kill switch.

--

Hayato

[Charles Harrison]

crbug.com/452084005 is one holdover that was accidentally not updated by this work. We still decode "%2E" (dot) into ".". There is an external contributor (+Sashmitha) who has CL crrev.com/c/7100669 fixing this.

Question for Blink owners: Does this bug fix require a kill switch?

To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/blink-dev/CAFpjS_3_a5ZoZbPFw4V-CHH2%2BmCdqmQfD8yjQOuC8tJRSt%2B7Gw%40mail.gmail.com.

[Daniel Bratell]

That is hard to judge without deep knowledge about how much is affected. Do you have any numbers or a gut feeling, or experience from the earlier changes?

/Daniel

[Mike Taylor]

I think putting this behind a kill switch is a good idea - like Daniel said, it's hard to judge the risk with just the info in the CL.

[Charles Harrison]

I think the previous launch did not require using the kill switch, but I'm fine being cautious here. I don't have much a gut feeling or data to suggest otherwise. I'll let Hayato chime in though.

[Mike Taylor]

FWIW, my LGTM for the original launch was contingent on a kill switch (see down thread). So hopefully we had one. :)