feat: add CountEntries to skip decryption for list --detail#562
feat: add CountEntries to skip decryption for list --detail#562
Conversation
list --detail previously called Extract() which performed full decryption just to count entries. Add CountEntries() that uses SELECT COUNT(*) queries, JSON element counting, and LevelDB key iteration — no master key needed. This fixes misleading zero counts when the master key is unavailable and significantly improves performance for the detail listing.
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #562 +/- ##
==========================================
+ Coverage 72.47% 73.28% +0.80%
==========================================
Files 48 48
Lines 1751 1950 +199
==========================================
+ Hits 1269 1429 +160
- Misses 364 384 +20
- Partials 118 137 +19
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Pull request overview
Adds a lightweight per-category counting pathway so list --detail can display entry counts without requiring master-key retrieval or decrypting browser data, addressing misleading zero counts and unnecessary work.
Changes:
- Introduce
Browser.CountEntries()and updatelist --detailto use it instead ofExtract(). - Add
sqliteutil.CountRows()plus per-categorycount*helpers for Chromium and Firefox (SQLite/JSON/LevelDB). - Add/extend unit tests to cover count behavior for supported categories across both browser implementations.
Reviewed changes
Copilot reviewed 35 out of 35 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| utils/sqliteutil/query.go | Adds CountRows helper for scalar COUNT(*) queries. |
| cmd/hack-browser-data/list.go | Switches detail listing to use CountEntries counts per category. |
| browser/browser.go | Extends Browser interface with CountEntries. |
| browser/chromium/chromium.go | Implements CountEntries and category routing for Chromium-based browsers. |
| browser/chromium/extract_storage.go | Adds LevelDB-based counting for local/session storage. |
| browser/chromium/extract_storage_test.go | Adds unit tests for storage counting; refactors fixtures. |
| browser/chromium/extract_password.go | Adds SQLite password count query + countPasswords. |
| browser/chromium/extract_password_test.go | Adds count tests; refactors DB setup helper. |
| browser/chromium/extract_history.go | Adds history count query + countHistories. |
| browser/chromium/extract_history_test.go | Adds count tests; refactors DB setup helper. |
| browser/chromium/extract_extension.go | Adds JSON-based extension counting (incl. Opera key path). |
| browser/chromium/extract_extension_test.go | Adds count tests; refactors JSON setup helper. |
| browser/chromium/extract_download.go | Adds download count query + countDownloads. |
| browser/chromium/extract_download_test.go | Adds count tests; refactors DB setup helper. |
| browser/chromium/extract_creditcard.go | Adds credit-card count query + countCreditCards. |
| browser/chromium/extract_creditcard_test.go | Adds count tests; refactors DB setup helper. |
| browser/chromium/extract_cookie.go | Adds cookie count query + countCookies. |
| browser/chromium/extract_cookie_test.go | Adds count tests; refactors DB setup helper. |
| browser/chromium/extract_bookmark.go | Adds JSON bookmark counting via recursive traversal. |
| browser/chromium/extract_bookmark_test.go | Adds count tests; refactors JSON setup helper. |
| browser/firefox/firefox.go | Implements CountEntries and category routing for Firefox. |
| browser/firefox/extract_storage.go | Adds SQLite localStorage count query + countLocalStorage. |
| browser/firefox/extract_storage_test.go | Adds localStorage count tests; refactors DB setup helper. |
| browser/firefox/extract_password.go | Adds JSON-based password counting from logins array. |
| browser/firefox/extract_password_test.go | Adds count tests. |
| browser/firefox/extract_history.go | Adds history count query + countHistories. |
| browser/firefox/extract_history_test.go | Adds count tests; refactors DB setup helper. |
| browser/firefox/extract_extension.go | Adds extension counting filtered to location == app-profile. |
| browser/firefox/extract_extension_test.go | Adds count tests; refactors JSON setup helper. |
| browser/firefox/extract_download.go | Adds download count query + countDownloads. |
| browser/firefox/extract_download_test.go | Adds count tests; refactors DB setup helper. |
| browser/firefox/extract_cookie.go | Adds cookie count query + countCookies. |
| browser/firefox/extract_cookie_test.go | Adds count tests; refactors DB setup helper. |
| browser/firefox/extract_bookmark.go | Adds bookmark count query + countBookmarks. |
| browser/firefox/extract_bookmark_test.go | Adds count tests; refactors DB setup helper. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| // CountRows runs a scalar count query (e.g. SELECT COUNT(*) FROM ...) and | ||
| // returns the integer result. It reuses the same database-open logic as | ||
| // QuerySQLite (file existence check, optional journal_mode=off). | ||
| func CountRows(dbPath string, journalOff bool, query string) (int, error) { | ||
| var count int | ||
| err := QuerySQLite(dbPath, journalOff, query, func(rows *sql.Rows) error { | ||
| return rows.Scan(&count) | ||
| }) | ||
| if err != nil { | ||
| return 0, fmt.Errorf("count rows: %w", err) | ||
| } | ||
| return count, nil |
There was a problem hiding this comment.
CountRows uses QuerySQLite, but QuerySQLite intentionally swallows scanFn errors (logs at debug and continues). That means rows.Scan errors (e.g., type/overflow issues on 32-bit when COUNT(*) exceeds int) will be silently ignored and CountRows may return 0 with nil error, producing misleading counts. Consider implementing CountRows with a fail-fast QueryRow/Scan path (reusing the same file existence + optional PRAGMA logic), and scan into int64 with explicit range checking before converting to int.
browser/chromium/extract_storage.go
Outdated
| if bytes.Equal(key, []byte(localStorageVersionKey)) { | ||
| continue | ||
| } | ||
| count++ |
There was a problem hiding this comment.
countLocalStorage currently counts every LevelDB key except VERSION. extractLocalStorage/parseLocalStorageEntry only includes META/METAACCESS and '_' data keys, and skips any unrecognized keys; with the current logic, CountEntries can overcount if the DB contains additional internal keys. To keep list --detail consistent with Extract(), filter using the same key classification (e.g., call parseLocalStorageEntry on iter.Key()/iter.Value() and count only when ok==true).
| count++ | |
| if _, ok := parseLocalStorageEntry(key, iter.Value()); ok { | |
| count++ | |
| } |
Add integration tests for CountEntries() method and unit tests for countCategory() dispatch in both chromium_test.go and firefox_test.go, matching the existing Extract/extractCategory test patterns.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 39 out of 39 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| func countLocalStorage(path string) (int, error) { | ||
| if _, err := os.Stat(path); err != nil { | ||
| return 0, err | ||
| } | ||
| db, err := leveldb.OpenFile(path, nil) | ||
| if err != nil { | ||
| return 0, err | ||
| } | ||
| defer db.Close() | ||
|
|
||
| var count int | ||
| iter := db.NewIterator(nil, nil) | ||
| defer iter.Release() | ||
|
|
||
| for iter.Next() { | ||
| key := iter.Key() | ||
| if bytes.Equal(key, []byte(localStorageVersionKey)) { | ||
| continue | ||
| } | ||
| count++ | ||
| } | ||
| return count, iter.Error() |
There was a problem hiding this comment.
countLocalStorage currently counts all LevelDB keys except VERSION. This can diverge from extractLocalStorage, which ignores unrecognized keys via parseLocalStorageEntry (and only treats METAACCESS:, META:, and '_' data keys as entries). To keep list --detail counts consistent with extracted entry semantics and avoid over-counting future/unknown metadata keys, consider counting only recognized key patterns (METAACCESS:/META:/data prefix) instead of every non-VERSION key.
- CountRows: use db.QueryRow instead of QuerySQLite to fail fast on scan errors instead of silently returning 0. - countLocalStorage: use parseLocalStorageEntry for key classification to match extractLocalStorage filtering (skip unrecognized keys).
Cover normal count, empty table, file not found, and journalOff mode.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 40 out of 40 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| var count int | ||
| if err := db.QueryRow(query).Scan(&count); err != nil { | ||
| return 0, fmt.Errorf("count rows: %w", err) | ||
| } |
There was a problem hiding this comment.
CountRows scans COUNT(*) into an int, but SQLite returns an integer type that can exceed 32-bit limits. On 32-bit builds (or very large profiles) this can overflow silently. Consider scanning into int64 (or uint64) and then converting to int with a bounds check (or return int64 from CountRows).
Summary
Closes #549
list --detailpreviously calledExtract()for each browser, which performed full extraction (copy files → retrieve master key → decrypt all data) just to count entries vialen(). This was wasteful and produced misleading zero counts when the master key was unavailable.This PR adds a
CountEntries()method to theBrowserinterface that counts entries without decryption:SELECT COUNT(*) FROM <table>Key changes:
CountRowsutility toutils/sqliteutilfor scalar count queriescount*functions alongside existingextract*functions in bothbrowser/chromium/andbrowser/firefox/CountEntries()+countCategory()methods to bothchromium.Browserandfirefox.BrowserprintDetail()inlist.goto useCountEntries()instead ofExtract()opsettings) and Yandex browser variantsTest plan
setup*functionsgo build ./...passesgolangci-lint runpasses (no new issues)