Bookmark search does not work, cannot type in letters in the search field.

[mhouse777]

Description

You cant type in the bookmark search field.

Steps to Reproduce / Test plan

(thanks to @jasonrsadler!)

1. Launch Brave via CLI
2. Create a bookmark (it can be fake). Put a single quote in the url (example: [http://www.example.org/quote'test].
3. export the bookmarks to a file.
4. reimport the file.
   (Alternatively, you can create the bookmark in a different browser with a ' and import from that browser into brave and the issue appears)
5. Open bookmarks manager
6. Try to search

Actual result:
Search tries to work but clears immediately. console displays sql lock errors. state becomes empty with title = undefined and .match is not a function

Expected result:
Bookmark search should work

Reproduces how often:

%100

Brave Version

0.20.46

about:brave info:

Reproducible on current live release:

Additional Information

[Tomatoshadow2]

Also post what OS you are using.

[srirambv]

You need to have these two enabled for bookmark suggestions to show up.

Also please provide more info so that we can figure out what issue you are facing. Its not very clear from the issue

[mhouse777]

windows 10 , 64 bit. go to bookmarks tab in brave browser. select bookmarks. You are now in book mark manager, On the right you will see a field that says, "search Bookmarks". Try to type a search term in it., You can't , it doesn't work.

[jasonrsadler]

Reproduced under 0.23 after importing a large bookmark test file.

Win10 x64, working on STR

[jasonrsadler]

STR:

Create a bookmark (it can be fake). Put a single quote in the url (example: [http://www.example.org/quote'test].

export the bookmarks to a file.

reimport the file.

Alternatively, you can create the bookmark in a different browser with a ' and import from that browser into brave and the issue appears

Search tries to work but clears immediately. console displays sql lock errors. state becomes empty with title = undefined and .match is not a function

suspecting importing HTML bookmark files not escaping/urlencoding single quotes properly.

RECOVERY: Go to the application data folder (on windows it's /appdata/roaming/brave (or brave-development). Open session-store-1 in a text editor. Find the bookmarks object near the end of the file. The 'broken' entry will look like this:

"http://www.brandonporter.com/mozilla_bug/test'2.html|0|0": {
            "width": 239.11572265625,
            "parentFolderId": 0,
            "partitionNumber": 0,
            "location": "http://www.brandonporter.com/mozilla_bug/test'2.html",
            "title": "www.brandonporter.com/mozilla_bug/test'2.html",
            "skipSync": null,
            "objectId": null,
            "type": "bookmark",
            "key": "http://www.brandonporter.com/mozilla_bug/test'2.html|0|0"
        },
http://www.brandonporter.com/mozilla_bug/test!2.html|0|0": {
            "width": 239.11572265625
}

The second entry with the 'width' parameter is the corrupt entry. Remove it to get bookmark search working again.

This bug does not occur if you add a bookmark through the browser. Only on importing.

@diracdeltas could this potentially be a sqlite injection vulnerability?

[diracdeltas]

could this potentially be a sqlite injection vulnerability?

brave uses JSON, not sqlite, for its internal bookmarks store. it's possible that you would see errors like this if you created the bookmarks profile in a browser that does use sqlite for its store and then imported into Brave. i would not expect any sqlite errors if you made the profile in Brave without any importing.

either way, the risk is low if the user has to manually edit the bookmark URL for it to work.

[jasonrsadler]

Thanks for the clarification. The only reason I was asking was after importing a 'bad' bookmarks import and trying to run a bookmark search, the console spammed a bunch of sqlite lock errors at me. May be something else indirectly related but knowing that bookmarks is JSON based I would thing eliminates that as a possible vector.

[diracdeltas]

i've seen some sqlite lock errors when trying to run two instances of Brave at once from the same profile

[jasonrsadler]

That didn't seem to be the case on this, but I did notice I had to kill the residuals after closing the browser. Can you point me to where are the bookmarks JSON are kept?

Thanks.

Looking in session-store to see what's hosing up bookmarks

[jasonrsadler]

This will not fix those already affected by the issue. ~~~To resolve for end users, recommend deleting session-store-1 file from appdata~~~ Recommend backup up session-store-1. Will need to be manually edited to remove the corrupt entry

Edited based on @bsclifton's comment below

[LaurenWags]

multiple +1s from community: https://community.brave.com/t/bookmark-search-field-does-not-work/16154

[bsclifton]

@jasonrsadler deleting the session-store-1 file is pretty extreme and IMO shouldn't be done unless there is no other choice (and even then, we'd want to back up the file).

Please keep in mind, users may be reading this issue and may have a lot of data associated in their session store (ledger, bookmarks, history, etc)

Would the user be able to work-around this issue by exporting / importing their bookmarks?

[jasonrsadler]

@bsclifton I agree deleting sessionStore is a last resort. I don't think exporting and reimporting the bookmarks fixes it as the bug is introduced upon first import. In fact, if you manually create a bookmark with a single-quote in it, export the bookmarks and reimport them, the bug is introduced.

But yes. Definitely a backup of the file should be made if the user goes that route. Otherwise, they would need to open the sessionStore, prettify it, and remove the corrupt entry by hand.

[jasonrsadler]

Resolved in 122aa70

[bsclifton]

@jasonrsadler I updated the original post with your test steps and assigned this to the 0.22.x release 3 milestone. Also tagged with release notes include/test plan specified (QA uses these tags to generate release notes and know which issues have manual test steps)

Do you mind looking over the steps to see if they look correct? Thanks 😄 👍

[jasonrsadler]

@bsclifton Yes. Those steps are correct. Obviously to STR now, you'll need a commit before 122aa70. Thank you sir!

[srirambv]

Verified on Windows x64

• 0.22.702 e4a853d
• libchromiumcontent 66.0.3359.139
• muon: 6.0.7

Verified with macOS 10.12.6 using

• 0.22.702 e4a853d
• muon 6.0.7
• libchromiumcontent 66.0.3359.139

Verified on Ubuntu 17.10 x64

• 0.22.703 903b8d0
• libchromiumcontent 66.0.3359.139
• muon: 6.0.8

[mhouse777]

....The search bookmark field STILL does not work, even after recent update v0.23.105

[mhouse777]

windows 10 64 bit