LGTM1 to optimistically add a usecounter and enable (with a base feature that'd enable to disable this if needed)
On Wed, Feb 22, 2023 at 5:30 PM Yoav Weiss <[email protected]> wrote: > Adding the usecounter and optimistically defaulting to enable it (while > keeping an eye on the numbers) would also work for me. > > On Wed, Feb 22, 2023 at 5:27 PM Rick Byers <[email protected]> wrote: > >> If this only ever caused connections that previously failed to now >> succeed as they do in other browsers, then the risk of it causing a compat >> issue is exceedingly low right? Perhaps this is more of a bugfix than a >> breaking API change? >> >> I see the implementation is already behind a base::Feature. One option >> would be to add a UseCounter and turn the feature on to 100% via finch. >> Then we can check the UseCounter data in beta and turn the finch flag off >> if it's surprisingly high. Thoughts? >> >> Rick >> >> On Wed, Feb 22, 2023 at 11:21 AM Yoav Weiss <[email protected]> >> wrote: >> >>> >>> >>> On Wed, Feb 22, 2023 at 3:32 PM Jiacheng Guo <[email protected]> wrote: >>> >>>> We don't currently have a use counter for it. >>>> Does it make sense to add the port overflow check under a flag and a >>>> usecounter as well to record the frequency of setting URL ports with an >>>> overflow value. >>>> >>> >>> I think it would make sense. If there's time pressure, we may not even >>> have to wait till the usecounter hits stable and use the internal UMA >>> equivalent to get a read of what current impacted usage is like. With any >>> luck, it'd be close to 0, and we'd be able to go ahead. At the same time, >>> having those usage numbers would help reassure us that breakage is not >>> significant. >>> >>> >>>> We can collect data first and then gradually enable the feature based >>>> on the data we collected. >>>> >>>> On Wed, Feb 22, 2023 at 11:01 PM Yoav Weiss <[email protected]> >>>> wrote: >>>> >>>>> >>>>> >>>>> On Wed, Feb 22, 2023 at 2:23 PM 'Jiacheng Guo' via blink-dev < >>>>> [email protected]> wrote: >>>>> >>>>>> The implementation can be found at >>>>>> https://chromium-review.googlesource.com/c/chromium/src/+/4252309 >>>>>> >>>>>> On Wed, Feb 22, 2023 at 9:39 PM Jiacheng Guo <[email protected]> wrote: >>>>>> >>>>>>> Contact [email protected] >>>>>>> >>>>>>> ExplainerThis is an implementation of an established standard. >>>>>>> >>>>>> >>>>> An explainer (even inline) helps to understand what change you're >>>>> trying to ship, regardless of its spec status. >>>>> At the same time, the explanation you included in your summary does >>>>> that. >>>>> >>>>> >>>>>> >>>>>>> >>>>>>> Specificationhttps://url.spec.whatwg.org/#dom-url-port >>>>>>> >>>>>>> Summary >>>>>>> >>>>>>> The port value will be checked when setting url.port. All the values >>>>>>> that overflows the 16-bit numeric limit will be no longer valid. For >>>>>>> instance the following script behave differently after the change: ``` >>>>>>> u = >>>>>>> new URL("http://test.com";); u.port = 65536; console.log(u.port); >>>>>>> ``` Before the change the output is 65536. After the change the output >>>>>>> will >>>>>>> be 80. >>>>>>> >>>>>> >>>>> Do we have a usecounter for this? >>>>> >>>>> Do I understand correctly and the current behavior would cause >>>>> requests that are based on such URL values to fail, given that the port >>>>> number exceeds what's permitted on the network protocol? >>>>> If that's the case, this change could cause such requests to "succeed" >>>>> even though they are sent to a different origin than what the developer >>>>> had >>>>> in mind. >>>>> >>>>> >>>>>>> >>>>>>> Blink componentBlink>JavaScript>API >>>>>>> <https://bugs.chromium.org/p/chromium/issues/list?q=component:Blink%3EJavaScript%3EAPI> >>>>>>> >>>>>>> TAG review >>>>>>> >>>>>>> TAG review statusNot applicable >>>>>>> >>>>>>> Risks >>>>>>> >>>>>>> >>>>>>> Interoperability and Compatibility >>>>>>> >>>>>>> >>>>>>> >>>>>>> *Gecko*: Shipped/Shipping >>>>>>> >>>>>>> *WebKit*: Shipped/Shipping >>>>>>> >>>>>>> *Web developers*: No signals >>>>>>> >>>>>>> *Other signals*: >>>>>>> >>>>>>> WebView application risks >>>>>>> >>>>>>> No signals >>>>>>> >>>>>>> >>>>>>> >>>>>>> Debuggability >>>>>>> >>>>>>> >>>>>>> >>>>>>> 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 >>>>>>> >>>>>>> Flag nameURLSetPortCheckOverflow >>>>>>> >>>>>>> Requires code in //chrome?False >>>>>>> >>>>>>> Tracking bughttps://crbug.com/1416017 >>>>>>> >>>>>>> Estimated milestones >>>>>>> >>>>>>> No milestones specified >>>>>>> >>>>>>> >>>>>>> Anticipated spec changes >>>>>>> >>>>>>> Open questions about a feature may be a source of future web compat >>>>>>> or interop issues. Please list open issues (e.g. links to known github >>>>>>> issues in the project for the feature specification) whose resolution >>>>>>> may >>>>>>> introduce web compat/interop risk (e.g., changing to naming or >>>>>>> structure of >>>>>>> the API in a non-backward-compatible way). >>>>>>> >>>>>>> >>>>>>> Link to entry on the Chrome Platform Status >>>>>>> https://chromestatus.com/feature/5097311074516992 >>>>>>> >>>>>>> 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 [email protected]. >>>>>> To view this discussion on the web visit >>>>>> https://groups.google.com/a/chromium.org/d/msgid/blink-dev/CAJQw1NyQV7zuDODbvobJK%2BK_WeSX%2B-Bq5S80RAQqQZmy2NZxqw%40mail.gmail.com >>>>>> <https://groups.google.com/a/chromium.org/d/msgid/blink-dev/CAJQw1NyQV7zuDODbvobJK%2BK_WeSX%2B-Bq5S80RAQqQZmy2NZxqw%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 [email protected]. >>> To view this discussion on the web visit >>> https://groups.google.com/a/chromium.org/d/msgid/blink-dev/CAL5BFfXNj3LDqQ0H%3DUE_p3zML%3DTRksMzb%3Dtc25EmtkaaCPNa3w%40mail.gmail.com >>> <https://groups.google.com/a/chromium.org/d/msgid/blink-dev/CAL5BFfXNj3LDqQ0H%3DUE_p3zML%3DTRksMzb%3Dtc25EmtkaaCPNa3w%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 [email protected]. To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/blink-dev/CAL5BFfUcjZ_9nLbvUX4Dg12d6RYaWd05FjsqokKtBZfe%3DJohsw%40mail.gmail.com.
