1603316 - repeating . and a letter (s) causes firefox to hang in browser

Status: VERIFIED FIXED

(Firefox :: Address Bar, defect, P2)

Description

[astroparty1232]

User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:71.0) Gecko/20100101 Firefox/71.0

Steps to reproduce:

open tab
alternate inputting s and .
after a while browser slows, hangs and forces to restart manually

Actual results:

browser hanged

Expected results:

nothing

Comment 1

[Pablo]

i was able to replicate in windows10 64bit and Ubuntu 18.04 64bit and Macos 10.15 using:

Firefox Nightly 73.0a1
Firefox beta 72.0b7
Firefox release 71

Comment 2

[Mark Banner (:standard8)]

Doesn't reproduce in the search bar, but it does in the address bar. I suspect this is something to do with the ip-detection code.

Comment 3

[BugBot [:suhaib / :marco/ :calixte]]

The priority flag is not set for this bug.
:adw, could you have a look please?

For more information, please visit auto_nag documentation.

Comment 4

[Drew Willcoxon :adw]

Doesn't take a very long string to make this happen. Maybe it's hitting the same regexp that bug 1587867 hit. If so, we can see if the original patch in that bug helps. Worth investigating for sure.

Comment 5

[Drew Willcoxon :adw]

Attachment: Bug 1603316 - Quantumbar: Fix slow regexp that tests for IPv4 URLs.

The problem seems to be that there is a dot in both the first square-bracket group and after the group, in this regexp: https://searchfox.org/mozilla-central/rev/c52d5f8025b5c9b2b4487159419ac9012762c40c/toolkit/components/places/UnifiedComplete.jsm#565

Removing the one inside the group fixes both bug 1603316 and bug 1605108. I'm not sure why there was a dot inside the group to begin with. Am I missing something? This regexp is testing for IPv4 addresses.

Comment 6

[Drew Willcoxon :adw]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=90e3620e5b78aaca4b9ada77099f54a54cacafc2

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/84322e2831b1
Quantumbar: Fix slow regexp that tests for IPv4 URLs. r=mak

Comment 8

[Bogdan Tara[:bogdan_tara | bogdant]]

https://hg.mozilla.org/mozilla-central/rev/84322e2831b1

Comment 9

[Drew Willcoxon :adw]

Ryan, did you mean to set 73 to wontfix? Is it too late? This would be nice, and it's only a one-line regexp change, so it's safe to uplift.

Comment 10

[Ryan VanderMeulen [:RyanVM]]

I was a bit worried about unintended consequences and we're building the Fx73 RC the week after next (with next week being an All Hands), so there won't be a lot of time to deal with any fallout. Based on that, I felt that getting more bake time with 74 seemed reasonable. How confident are we in our test coverage here uplifting into late Beta isn't scary?

Comment 11

[Drew Willcoxon :adw]

I'm pretty confident about test coverage. We don't have coverage for this regexp's performance, but we do have lots of coverage for the urlbar that would catch regressions.

But there's no rush for this and it's more of a nice to have, and given your reasoning, which makes sense, I'm OK with not uplifting. Thanks Ryan.

Comment 12

[Cornel Ionce [:noni] [Hubs QA]]

Reproduced the initial issue on Windows 10 x64 using Fx 72.0b7.

Confirming this is fixed using Fx 74.0b4, build ID 20200216164042 on:

• Windows 10 x64
• Ubuntu 18.04 x64
• macOS 10.15.3