Cap search query length to prevent crash on huge paste (fixes #1147) - #1394
Open
williamruiz1 wants to merge 1 commit into
Open
Cap search query length to prevent crash on huge paste (fixes #1147)#1394williamruiz1 wants to merge 1 commit into
williamruiz1 wants to merge 1 commit into
Conversation
When a user accidentally pastes large clipboard contents into the search field (e.g. cmd+v in Maccy's search bar), regex compilation in simpleSearch and pattern construction in Fuse can hang or crash the app. The fuzzy search already caps the *target* string at 5,000 chars, but the *pattern* (search query) was unbounded. Cap the query at 1,000 chars at the entry of Search.search so all search modes are protected. Refs p0deje#1147
| // Cap query length so an accidental cmd+v of huge clipboard contents | ||
| // into the search field can't blow up regex compilation or fuzzy matching. | ||
| let string = string.count > queryLimit ? String(string.prefix(queryLimit)) : string | ||
|
|
There was a problem hiding this comment.
Instead of capping the query we should avoid searching blobs alltogether.
For example if someone copy a text then image (very large) then text, theoretically because of image fuzzy search won't be able to find text below the image.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
When a user accidentally pastes large clipboard contents into Maccy's search field (e.g. cmd+v while the popup is open), regex compilation and Fuse pattern construction can hang or crash the app. Caps the search query at 1,000 chars at the entry of
Search.searchso all search modes are protected.Repro
See #1147 (comment) —
pbcopy < /usr/share/dict/words, open Maccy, cmd+v in the search bar.Root cause
simpleSearchwith.regularExpressioncompiles the entire query as anNSRegularExpressionpattern — very slow / can OOM on multi-MB input.fuzzySearchalready caps the target string atfuzzySearchLimit = 5_000, but the pattern passed tofuse.createPatternis unbounded.Fix
One-line guard in
Search.searchthat truncates the query to 1,000 chars before dispatching to any backend. No real user is searching clipboard history with a 1,000+ char pattern, so this is safe.Test plan
Fixes #1147