Firefox freezes / hangs when clicking on key with having to parse large data in Storage Inspector

RESOLVED FIXED in Firefox 48

Status

defect
RESOLVED FIXED
4 years ago
11 months ago

People

(Reporter: human.peng, Assigned: jsnajdr)

Tracking

Trunk
Firefox 48
Dependency tree / graph

Firefox Tracking Flags

(firefox48 fixed)

Details

Attachments

(1 attachment)

Reporter

Description

4 years ago
User Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:38.0) Gecko/20100101 Firefox/38.0
Build ID: 20150416143048

Steps to reproduce:

Install Firebug, because Firefox's built-in dev tools can't modiefy value of localstorage.
Open a website (for example, https://google.com ).
Open Firebug's DOM tool, search for "LocalStorage".
Modify any key's value to http://pastebin.com/NC4Y02BF (this particular one is what I encounter on a website)
Save.
Open developer tools. Check "Storage" in options.
Open "Storage", locate to Local Storage.
Find a key you just modified. Click on it.


Actual results:

Firefox freezes. 

I wait for minutes, it doesn't recover. I forcefully closed it, with this crash report:

https://crash-stats.mozilla.com/report/index/01da30fb-c650-41e1-8a54-4e02f2150421


Expected results:

It shouldn't freeze. I tried some other text for value like 15K of Lorem ipsums, but it doesn't trigger the bug. I tried only use half of the text I provided above, it doesn't tirgger the bug either. I don't know why. But what I provided can reproduce the bug on newest Nightly.
Reporter

Updated

4 years ago
Component: Untriaged → Developer Tools: Storage Inspector
Reporter

Updated

4 years ago
Version: 38 Branch → 40 Branch

Comment 1

4 years ago
I feel like we should just use JSON.parse to parse stuff here.
Status: UNCONFIRMED → NEW
Ever confirmed: true
Summary: Firefox freezes / hangs when clicking on key with certain large data in LocalStorage → Firefox freezes / hangs when clicking on key with having to parse large data in Storage Inspector
Version: 40 Branch → Trunk

Updated

4 years ago
Blocks: 1194190
Assignee

Comment 2

3 years ago
Firefox freezes when trying to parse the value as a key-value list with key-value delimiter "=" and pair delimiter "&", when matching the following regexp at [1]:

/^([^=&]*=+[^=&]*&*)+$/

The value reported by the reporter is a 18383 bytes long string with following structure:

6209bytes = 1498bytes = 1100bytes = 2798bytes = 1792bytes & 1882bytes = 3019bytes & 78bytes

The regexp matches very well until near the end, where it fails to find a match with the "=" character after the last "&". This leads to runaway exponential backtracking as described in [2].

The regexp is badly designed: it contains nested repetition operators, and one string can be matched in several ways:
1. Because "&*" matches empty string, "a=bcd=e" can be matched as ["a=", "&*", "bcd=e"], ["a=b", "&*", "cd=e"], etc.
2. Because of the "=+" clause, "a==b" can be matched as ["a=", "&*", "=b"], ["a==b", "&*"], etc.

The solution is to redesign the regexp so that everything can be matched only in one way. The attached fix does exactly that.

The fix changes the behavior of the parsing in two ways:
1. "a==b&c==d" is no longer parsed as object - only one "=" delimiter is allowed. The previous behavior had a bug where this string would be parsed as object with empty values: {a:"", c:""}, because of the array deconstruction at [3]. Repeated pair delimiters "a=b&&&c=d" are not allowed, too.
2. Arrays with empty field at the start are allowed. "#a#b#c" wasn't detected as array before, now it's detected as ["", "a", "b", "c"].

[1] https://dxr.mozilla.org/mozilla-central/source/devtools/client/storage/ui.js#612
[2] http://www.regular-expressions.info/catastrophic.html
[3] https://dxr.mozilla.org/mozilla-central/source/devtools/client/storage/ui.js#594
Attachment #8743908 - Flags: review?(mratcliffe)
Assignee

Updated

3 years ago
Assignee: nobody → jsnajdr
Comment on attachment 8743908 [details] [diff] [review]
Firefox freezes when parsing large data in Storage Inspector.

Review of attachment 8743908 [details] [diff] [review]:
-----------------------------------------------------------------

r+ assuming a green try.
Attachment #8743908 - Flags: review?(mratcliffe) → review+
Assignee

Updated

3 years ago
Keywords: checkin-needed

Comment 6

3 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/80598be5d80e
Status: NEW → RESOLVED
Last Resolved: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 48

Updated

11 months ago
Product: Firefox → DevTools
You need to log in before you can comment on or make changes to this bug.