Closed
Bug 1384696
Opened 7 years ago
Closed 3 years ago
Crash while deleting a folder which has more than SQLITE_MAX_VARIABLE_NUMBER child folders on any of the tree levels
Categories
(Firefox for Android Graveyard :: Data Providers, enhancement, P2)
Firefox for Android Graveyard
Data Providers
Tracking
(Not tracked)
RESOLVED
INCOMPLETE
People
(Reporter: Grisha, Assigned: jwu, NeedInfo)
References
Details
Attachments
(2 files)
59 bytes,
text/x-review-board-request
|
Details | |
5.89 KB,
patch
|
Grisha
:
review+
|
Details | Diff | Splinter Review |
We don't perform "chunking" while traversing through the bookmark tree in [0], which will result in a crash (failing to prepare sql statement, "too many variables") for any structure which has more than SQLITE_max_variable_number folders on any tree level. Example: root -> folder --> bookmark --> folder-1 --> folder-2 ... --> folder-1000 'folder' has one bookmark and 1000 folders in it. If we were to delete guid='folder', we'll make a list of all folders whose parent='folder', and perform a SQL statement which looks like "SELECT ... WHERE parent IN (?, ?, ?... for each child folder..., ?, ?)". This will crash because number of child folders (1000) exceeds SQL's max_variable_number (999 by default). [0] https://dxr.mozilla.org/mozilla-central/source/mobile/android/base/java/org/mozilla/gecko/db/BrowserProvider.java#2364
Comment hidden (mozreview-request) |
Reporter | ||
Comment 2•7 years ago
|
||
I'll push a patch with a fix later this week, unless jwu can get to it faster.
Updated•7 years ago
|
Component: General → Data Providers
Summary: Crash while deleting a folder which has more than SQLITE_max_variable_number children folders on any of the tree levels → Crash while deleting a folder which has more than SQLITE_MAX_VARIABLE_NUMBER child folders on any of the tree levels
Assignee | ||
Comment 3•7 years ago
|
||
Sorry for the incorrect use about database query. A patch is attached for reviewing. Also I made a little change in the test case to meet the requirement, hope my understanding is correct.
Assignee: nobody → topwu.tw
Attachment #8890658 -
Flags: review?(gkruglov)
Reporter | ||
Comment 4•7 years ago
|
||
Comment on attachment 8890658 [details] [diff] [review] Query folders iteratively to prevent exceeding maximum variable count in a clause Review of attachment 8890658 [details] [diff] [review]: ----------------------------------------------------------------- Before landing this, can you add tests around boundary conditions of the chunking logic: triggering one under, at, and one above the variable limit cases.
Attachment #8890658 -
Flags: review?(gkruglov) → review+
Updated•6 years ago
|
Priority: -- → P2
Comment 6•5 years ago
|
||
There's a r+ patch which didn't land and no activity in this bug for 2 weeks.
:jwu, could you have a look please?
Flags: needinfo?(topwu.tw)
Comment 7•3 years ago
|
||
We have completed our launch of our new Firefox on Android. The development of the new versions use GitHub for issue tracking. If the bug report still reproduces in a current version of [Firefox on Android nightly](https://play.google.com/store/apps/details?id=org.mozilla.fenix) an issue can be reported at the [Fenix GitHub project](https://github.com/mozilla-mobile/fenix/). If you want to discuss your report please use [Mozilla's chat](https://wiki.mozilla.org/Matrix#Connect_to_Matrix) server https://chat.mozilla.org and join the [#fenix](https://chat.mozilla.org/#/room/#fenix:mozilla.org) channel.
Status: NEW → RESOLVED
Closed: 3 years ago
Resolution: --- → INCOMPLETE
Updated•3 years ago
|
Product: Firefox for Android → Firefox for Android Graveyard
You need to log in
before you can comment on or make changes to this bug.
Description
•