diff --git a/CHANGELOG.md b/CHANGELOG.md index 901ace78..409ed194 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -524,6 +524,14 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 * Excluding all genres collapses to a single untagged-genre rule instead of hundreds of `notContains` filters that stalled Navidrome; empty smart playlists settle without a false "Playlist not found" after a long spinner. +### Local index live search — no junk hits on `=` and syntax characters + +**By [@cucadmuh](https://github.com/cucadmuh), reported by zunoz on the Psysonic Discord, PR [#983](https://github.com/Psychotoxical/psysonic/pull/983)** + +* Queries such as `1=2` or `M=c` no longer return unrelated albums and artists — FTS5 was parsing `=` and similar characters as query syntax instead of a literal token. +* Wildcard-only queries (`**`, `****`) are rejected for both local index and server search; titles that contain censorship stars (e.g. `***Flawless`) remain searchable. + + ### In-page browse — virtual scroll and cover-art priority **By [@cucadmuh](https://github.com/cucadmuh), PR [#783](https://github.com/Psychotoxical/psysonic/pull/783)** diff --git a/src-tauri/crates/psysonic-library/src/live_search.rs b/src-tauri/crates/psysonic-library/src/live_search.rs index bb20c623..f1b7ae8c 100644 --- a/src-tauri/crates/psysonic-library/src/live_search.rs +++ b/src-tauri/crates/psysonic-library/src/live_search.rs @@ -659,6 +659,55 @@ mod tests { ); } + #[test] + fn live_search_equals_query_returns_no_false_positives() { + let store = LibraryStore::open_in_memory(); + TrackRepository::new(&store) + .upsert_batch(&[ + track( + "s1", + "t1", + "Intro", + "Smith & Myers", + "Volume 1 & 2", + "al_vol", + "ar1", + ), + track("s1", "t2", "Hello", "Adele", "25", "al_25", "ar2"), + track("s1", "t3", "Track", "Y.O.M.C.", "Single", "al_yo", "ar3"), + ]) + .unwrap(); + for q in ["1=2", "1=1", "M=c"] { + let resp = run_live_search(&store, "s1", q, None, 5, 5, 10).unwrap(); + assert!( + resp.tracks.is_empty() && resp.albums.is_empty() && resp.artists.is_empty(), + "query {q:?} must not fuzzy-match unrelated library rows" + ); + } + } + + #[test] + fn live_search_censorship_stars_in_title_is_searchable() { + let store = LibraryStore::open_in_memory(); + TrackRepository::new(&store) + .upsert_batch(&[ + track( + "s1", + "t1", + "***Flawless", + "Beyoncé", + "BEYONCÉ", + "al1", + "ar1", + ), + track("s1", "t2", "Other Song", "Artist", "Album", "al2", "ar2"), + ]) + .unwrap(); + let resp = run_live_search(&store, "s1", "***Flawless", None, 5, 5, 10).unwrap(); + assert_eq!(resp.tracks.len(), 1); + assert_eq!(resp.tracks[0].title, "***Flawless"); + } + #[test] fn live_search_multiword_album_matches_any_token_not_only_first() { let store = LibraryStore::open_in_memory(); diff --git a/src-tauri/crates/psysonic-library/src/search.rs b/src-tauri/crates/psysonic-library/src/search.rs index 1075ea3c..2e02cc7f 100644 --- a/src-tauri/crates/psysonic-library/src/search.rs +++ b/src-tauri/crates/psysonic-library/src/search.rs @@ -62,6 +62,32 @@ pub fn search_tracks( /// Callers clamp their requested `limit` into `1..=PAGE_LIMIT_MAX`. pub(crate) const PAGE_LIMIT_MAX: u32 = 500; +/// Characters that break FTS5 quoted tokens — not `*` (censorship stars in titles). +const FTS_QUERY_SYNTAX_CHARS: &[char] = &['=', ':', '(', ')', '^', '<', '>', '%', '|', '\\']; + +fn is_wildcard_only_token(token: &str) -> bool { + !token.is_empty() && token.chars().all(|c| c == '*') +} + +/// True when `token` can be safely wrapped in FTS5 quotes for prefix/phrase match. +pub(crate) fn fts_token_is_safe(token: &str) -> bool { + let t = token.trim(); + !t.is_empty() + && !is_wildcard_only_token(t) + && !t.chars().any(|c| FTS_QUERY_SYNTAX_CHARS.contains(&c)) + && t.chars().any(|c| c.is_alphanumeric() || c as u32 >= 0x80) +} + +/// Whitespace-split tokens when every segment is FTS-safe; otherwise `None`. +pub(crate) fn fts_safe_whitespace_tokens(raw: &str) -> Option> { + let tokens: Vec<&str> = raw.split_whitespace().filter(|t| !t.is_empty()).collect(); + if tokens.is_empty() || !tokens.iter().all(|t| fts_token_is_safe(t)) { + None + } else { + Some(tokens) + } +} + /// Local FTS is skipped below this length — single-character queries (e.g. Cyrillic /// «а», Latin «a») match huge fractions of a large library and bm25+LIMIT can /// take tens of seconds (§5.9: no heavy work on every keystroke). @@ -92,13 +118,11 @@ pub(crate) fn fts_prefix_token_expr(raw: &str) -> Option { /// Navidrome-style any-word prefix match (`"a"* OR "b"*`). pub(crate) fn fts_prefix_token_or_expr(raw: &str) -> Option { - let tokens: Vec = raw - .split_whitespace() + let tokens: Vec = fts_safe_whitespace_tokens(raw)? + .into_iter() .map(|t| format!("\"{}\"*", t.replace('"', "\"\""))) .collect(); - if tokens.is_empty() { - None - } else if tokens.len() == 1 { + if tokens.len() == 1 { Some(tokens.into_iter().next().unwrap()) } else { Some(tokens.join(" OR ")) @@ -106,8 +130,8 @@ pub(crate) fn fts_prefix_token_or_expr(raw: &str) -> Option { } fn fts_token_expr_with(raw: &str, prefix: bool) -> Option { - let tokens: Vec = raw - .split_whitespace() + let tokens: Vec = fts_safe_whitespace_tokens(raw)? + .into_iter() .map(|t| { let quoted = format!("\"{}\"", t.replace('"', "\"\"")); if prefix { @@ -117,11 +141,7 @@ fn fts_token_expr_with(raw: &str, prefix: bool) -> Option { } }) .collect(); - if tokens.is_empty() { - None - } else { - Some(tokens.join(" ")) - } + Some(tokens.join(" ")) } /// Column-scoped prefix match (`artist : "met"*` → Metallica). @@ -470,6 +490,37 @@ mod tests { assert!(fts_query(" ").is_none()); } + #[test] + fn fts_prefix_token_or_expr_rejects_syntax_metachar_tokens() { + assert!(fts_prefix_token_or_expr("1=2").is_none()); + assert!(fts_prefix_token_or_expr("1=1").is_none()); + assert!(fts_prefix_token_or_expr("M=c").is_none()); + assert!(fts_prefix_token_or_expr("V()>P").is_none()); + assert!(fts_prefix_token_or_expr("**").is_none()); + assert!(fts_prefix_token_or_expr("****").is_none()); + } + + #[test] + fn fts_prefix_token_or_expr_allows_censorship_stars_in_titles() { + assert_eq!( + fts_prefix_token_or_expr("***Flawless").as_deref(), + Some("\"***Flawless\"*") + ); + assert_eq!( + fts_prefix_token_or_expr("B********").as_deref(), + Some("\"B********\"*") + ); + } + + #[test] + fn fts_prefix_token_or_expr_still_builds_safe_tokens() { + assert_eq!( + fts_prefix_token_or_expr("love supreme").as_deref(), + Some("\"love\"* OR \"supreme\"*") + ); + assert_eq!(fts_prefix_token_or_expr("25").as_deref(), Some("\"25\"*")); + } + #[test] fn aliased_track_columns_prefixes_every_column() { let cols = aliased_track_columns("t"); diff --git a/src/api/subsonicSearch.ts b/src/api/subsonicSearch.ts index 3f3f9950..75c506d4 100644 --- a/src/api/subsonicSearch.ts +++ b/src/api/subsonicSearch.ts @@ -1,4 +1,5 @@ import { api, libraryFilterParams } from './subsonicClient'; +import { searchQueryIsFtsSafe } from '../utils/library/searchQueryFtsSafe'; import type { SearchResults, SubsonicAlbum, @@ -26,6 +27,7 @@ export async function search( }, ): Promise { if (!query.trim()) return { artists: [], albums: [], songs: [] }; + if (!searchQueryIsFtsSafe(query)) return { artists: [], albums: [], songs: [] }; const data = await api<{ searchResult3: { artist?: SubsonicArtist[]; @@ -58,6 +60,7 @@ export async function search( * Caller handles empty results gracefully (Tracks page falls back to its random pool). */ export async function searchSongsPaged(query: string, songCount: number, songOffset: number): Promise { + if (!searchQueryIsFtsSafe(query.trim())) return []; const data = await api<{ searchResult3: { song?: SubsonicSong[] } }>('search3.view', { query, artistCount: 0, diff --git a/src/components/LiveSearch.tsx b/src/components/LiveSearch.tsx index 7ba139ca..8a6f2aa0 100644 --- a/src/components/LiveSearch.tsx +++ b/src/components/LiveSearch.tsx @@ -5,7 +5,7 @@ import { LIVE_SEARCH_DEBOUNCE_NETWORK_MS, LIVE_SEARCH_DEBOUNCE_RACE_MS, EMPTY_SEARCH_RESULTS, - liveSearchQueryTooShort, + liveSearchQueryRejected, mergeLiveSearchResults, runLocalLiveSearch, runNetworkLiveSearch, @@ -261,10 +261,10 @@ export default function LiveSearch() { setLoading(true); const searchT0 = performance.now(); try { - if (liveSearchQueryTooShort(q)) { + if (liveSearchQueryRejected(q)) { if (!isStale()) { setResults(EMPTY_SEARCH_RESULTS); - setSearchSource('local'); + setSearchSource(null); setOpen(true); } return; @@ -688,7 +688,7 @@ export default function LiveSearch() { )} {!hasResults && !loading && ( -
{t('search.noResults', { query })}
+
{t('search.noResults', { query: query.trim() })}
)} {share.shareMatch && ( diff --git a/src/components/MobileSearchOverlay.tsx b/src/components/MobileSearchOverlay.tsx index 378b5df6..757fe4af 100644 --- a/src/components/MobileSearchOverlay.tsx +++ b/src/components/MobileSearchOverlay.tsx @@ -311,7 +311,7 @@ export default function MobileSearchOverlay({ onClose }: { onClose: () => void } {/* ── No results ── */} {!loading && query && !hasResults && !isLiveSearchDropdownBlocked(scope) && (
- {t('search.noResults', { query })} + {t('search.noResults', { query: query.trim() })}
)} diff --git a/src/utils/library/liveSearchLocal.test.ts b/src/utils/library/liveSearchLocal.test.ts index 4c84130e..a848d604 100644 --- a/src/utils/library/liveSearchLocal.test.ts +++ b/src/utils/library/liveSearchLocal.test.ts @@ -3,6 +3,7 @@ import { onInvoke } from '@/test/mocks/tauri'; import type { SearchResults } from '../../api/subsonicTypes'; import { useAuthStore } from '@/store/authStore'; import { + liveSearchQueryRejected, liveSearchQueryTooShort, mergeLiveSearchResults, runLocalLiveSearch, @@ -92,6 +93,16 @@ describe('runLocalLiveSearch', () => { }); }); +describe('liveSearchQueryRejected', () => { + it('rejects syntax junk and single-character queries', () => { + expect(liveSearchQueryRejected('**')).toBe(true); + expect(liveSearchQueryRejected('1=2')).toBe(true); + expect(liveSearchQueryRejected('а')).toBe(true); + expect(liveSearchQueryRejected('ab')).toBe(false); + expect(liveSearchQueryRejected('metallica')).toBe(false); + }); +}); + describe('liveSearchQueryTooShort', () => { it('treats one grapheme as too short', () => { expect(liveSearchQueryTooShort('а')).toBe(true); diff --git a/src/utils/library/liveSearchLocal.ts b/src/utils/library/liveSearchLocal.ts index dfe65b10..9e3e845e 100644 --- a/src/utils/library/liveSearchLocal.ts +++ b/src/utils/library/liveSearchLocal.ts @@ -14,6 +14,7 @@ import { trackToSong, } from './advancedSearchLocal'; import { logLibrarySearch, timed } from './libraryDevLog'; +import { searchQueryIsFtsSafe } from './searchQueryFtsSafe'; export const LIVE_SEARCH_DEBOUNCE_LOCAL_MS = 200; export const LIVE_SEARCH_DEBOUNCE_NETWORK_MS = 300; @@ -45,6 +46,14 @@ export function liveSearchQueryTooShort(query: string): boolean { return !q || queryGraphemeCount(q) < LOCAL_FTS_MIN_QUERY_CHARS; } +/** Skip local FTS and search3 when the query would only produce junk wildcard hits. */ +export function liveSearchQueryRejected(query: string): boolean { + const q = query.trim(); + if (!q) return true; + if (liveSearchQueryTooShort(q)) return true; + return !searchQueryIsFtsSafe(q); +} + export type LiveSearchStaleCheck = () => boolean; export interface LiveSearchRunContext { @@ -61,7 +70,7 @@ export async function runLocalLiveSearch( ): Promise { if (!serverId || ctx.isStale()) return null; const q = query.trim(); - if (liveSearchQueryTooShort(q)) return null; + if (liveSearchQueryRejected(q)) return null; const t0 = performance.now(); try { const { result: resp, ms: invokeMs } = await timed(() => @@ -131,7 +140,7 @@ export async function runNetworkLiveSearch( signal?: AbortSignal, ): Promise { const q = query.trim(); - if (liveSearchQueryTooShort(q)) return null; + if (liveSearchQueryRejected(q)) return null; try { return await search(q, { signal, diff --git a/src/utils/library/searchQueryFtsSafe.test.ts b/src/utils/library/searchQueryFtsSafe.test.ts new file mode 100644 index 00000000..0c288cb9 --- /dev/null +++ b/src/utils/library/searchQueryFtsSafe.test.ts @@ -0,0 +1,35 @@ +import { describe, expect, it } from 'vitest'; +import { searchQueryIsFtsSafe, searchTokenIsFtsSafe } from './searchQueryFtsSafe'; + +describe('searchQueryIsFtsSafe', () => { + it('rejects equals and wildcard-only junk queries', () => { + expect(searchQueryIsFtsSafe('1=2')).toBe(false); + expect(searchQueryIsFtsSafe('**')).toBe(false); + expect(searchQueryIsFtsSafe('***')).toBe(false); + expect(searchQueryIsFtsSafe('****')).toBe(false); + expect(searchQueryIsFtsSafe('M=c')).toBe(false); + expect(searchQueryIsFtsSafe('V()>P')).toBe(false); + }); + + it('accepts normal search terms and censorship stars in titles', () => { + expect(searchQueryIsFtsSafe('metallica')).toBe(true); + expect(searchQueryIsFtsSafe('love supreme')).toBe(true); + expect(searchQueryIsFtsSafe('25')).toBe(true); + expect(searchQueryIsFtsSafe('AC/DC')).toBe(true); + expect(searchQueryIsFtsSafe('***Flawless')).toBe(true); + expect(searchQueryIsFtsSafe('B********')).toBe(true); + expect(searchQueryIsFtsSafe('F**k This Industry')).toBe(true); + }); + + it('rejects when any token is unsafe', () => { + expect(searchQueryIsFtsSafe('dark side')).toBe(true); + expect(searchQueryIsFtsSafe('dark = side')).toBe(false); + }); +}); + +describe('searchTokenIsFtsSafe', () => { + it('requires at least one letter or digit', () => { + expect(searchTokenIsFtsSafe('***')).toBe(false); + expect(searchTokenIsFtsSafe('!!!')).toBe(false); + }); +}); diff --git a/src/utils/library/searchQueryFtsSafe.ts b/src/utils/library/searchQueryFtsSafe.ts new file mode 100644 index 00000000..df9764c3 --- /dev/null +++ b/src/utils/library/searchQueryFtsSafe.ts @@ -0,0 +1,26 @@ +/** + * Shared guard for local FTS and Subsonic search3. + * - Wildcard-only tokens (`**`, `****`) match everything on search3 / FTS5. + * - `=` and other query syntax break quoted FTS tokens (`1=2` → junk hits). + * - Asterisks in real tags (`***Flawless`, `B********`) stay searchable. + */ + +/** FTS5 / search3 syntax — not `*` (censorship stars in titles are valid). */ +const FTS_QUERY_SYNTAX_CHARS = new Set(['=', ':', '(', ')', '^', '<', '>', '%', '|', '\\']); + +function isWildcardOnlyToken(token: string): boolean { + return token.length > 0 && [...token].every(ch => ch === '*'); +} + +export function searchTokenIsFtsSafe(token: string): boolean { + const t = token.trim(); + if (!t || isWildcardOnlyToken(t)) return false; + if ([...t].some(ch => FTS_QUERY_SYNTAX_CHARS.has(ch))) return false; + return [...t].some(ch => /\p{L}|\p{N}/u.test(ch) || ch.charCodeAt(0) >= 0x80); +} + +/** Every whitespace token must be safe — mirrors `fts_safe_whitespace_tokens` in Rust. */ +export function searchQueryIsFtsSafe(query: string): boolean { + const tokens = query.trim().split(/\s+/).filter(Boolean); + return tokens.length > 0 && tokens.every(searchTokenIsFtsSafe); +}