mirror of
https://github.com/kilyabin/psysonic.git
synced 2026-07-21 22:15:40 +00:00
fix(search): local index rejects FTS syntax in live search queries (#983)
* fix(search): reject FTS syntax chars in local index live search queries FTS5 treats `=` and similar characters as query syntax, so tokens like 1=2 produced unrelated prefix hits. Skip FTS for unsafe tokens instead. * docs: CHANGELOG for local index FTS syntax query fix (PR #983) * fix(search): reject syntax junk in server search3 and live search race Share FTS-safe token guard with search3; skip network invoke for ** and similar queries; show empty dropdown without misleading source badge. * fix(search): allow censorship stars in queries, reject wildcard-only tokens Block ** and **** but keep ***Flawless-style title searches working in local FTS and search3.
This commit is contained in:
@@ -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<SearchResults> {
|
||||
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<SubsonicSong[]> {
|
||||
if (!searchQueryIsFtsSafe(query.trim())) return [];
|
||||
const data = await api<{ searchResult3: { song?: SubsonicSong[] } }>('search3.view', {
|
||||
query,
|
||||
artistCount: 0,
|
||||
|
||||
@@ -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 && (
|
||||
<div className="search-empty">{t('search.noResults', { query })}</div>
|
||||
<div className="search-empty">{t('search.noResults', { query: query.trim() })}</div>
|
||||
)}
|
||||
|
||||
{share.shareMatch && (
|
||||
|
||||
@@ -311,7 +311,7 @@ export default function MobileSearchOverlay({ onClose }: { onClose: () => void }
|
||||
{/* ── No results ── */}
|
||||
{!loading && query && !hasResults && !isLiveSearchDropdownBlocked(scope) && (
|
||||
<div className="mobile-search-noresults">
|
||||
{t('search.noResults', { query })}
|
||||
{t('search.noResults', { query: query.trim() })}
|
||||
</div>
|
||||
)}
|
||||
|
||||
|
||||
@@ -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);
|
||||
|
||||
@@ -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<SearchResults | null> {
|
||||
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<SearchResults | null> {
|
||||
const q = query.trim();
|
||||
if (liveSearchQueryTooShort(q)) return null;
|
||||
if (liveSearchQueryRejected(q)) return null;
|
||||
try {
|
||||
return await search(q, {
|
||||
signal,
|
||||
|
||||
@@ -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);
|
||||
});
|
||||
});
|
||||
@@ -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);
|
||||
}
|
||||
Reference in New Issue
Block a user