feat(lyric): enhance local matching with NCM metadata and prioritize local sources#969
feat(lyric): enhance local matching with NCM metadata and prioritize local sources#969kid141252010 wants to merge 16 commits intoimsyy:devfrom
Conversation
…g, fix SSRF vuln - Added metadata-based matching for local songs (NCM/TTML). - Implemented portable match index (.splayer-match.json) in local directories. - Refined merging logic to respect user's lyric priority based on quality levels. - Fixed SSRF vulnerability in /netease/lyric/ttml endpoint.
…g, fix SSRF vuln - Added metadata-based matching for local songs (NCM/TTML). - Implemented portable match index (.splayer-match.json) in local directories. - Refined merging logic to respect user's lyric priority based on quality levels. - Fixed SSRF vulnerability in /netease/lyric/ttml endpoint.
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! 此拉取请求的核心目标是全面提升本地音乐库的歌词匹配体验。通过引入基于网易云元数据的新匹配算法，结合高效的 TTML 扫描和多层缓存机制，极大地提高了本地歌曲歌词的获取准确性和速度。同时，优化了歌词来源的优先级处理，并增强了用户手动纠正匹配结果的能力，使得本地音乐的歌词管理更加智能和便捷。 Highlights
Changelog
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This Pull Request significantly enhances the local music experience, particularly the lyric matching function, through core matching algorithm refactoring, performance optimization via parallel scanning and caching, and a manual correction UI. However, two critical security vulnerabilities were identified: sensitive session cookies are being logged in plain text in the NetEase API proxy, and several IPC handlers lack path validation, potentially leading to arbitrary directory scans, Denial of Service, or information disclosure. Additionally, the code contains a critical bug that could lead to runtime crashes and a race condition that could result in data loss. Please refer to the detailed comments for specific fix suggestions regarding these issues.
| const artistMatch = targetArtists.some(targetArtist => | ||
| entry.artists.some(cacheArtist => this.isContainsMatch(cacheArtist, targetArtist)) | ||
| ); |
There was a problem hiding this comment.
在 getBySongAndArtist 方法中，代码尝试访问 entry.artists 属性，但是 TtmlIdEntry 接口定义中并没有 artists 字段。这将导致在运行时出现 TypeError: Cannot read properties of undefined (reading 'some') 的严重错误。
为了修复这个问题，需要：
- 在
TtmlIdEntry接口中添加artists: string[]字段。 - 修改
extractMetadataFromTTML函数，使其能够从 TTML 文件中提取歌手信息（如果存在），并将其包含在返回值中。 - 更新
TtmlIdMappingCache的set方法，以接收并存储artists数据。 - 在
scanTtmlIdMapping函数中，将提取到的artists数据传递给cache.set方法。
| ipcMain.handle( | ||
| "save-local-match-index", | ||
| async (_event, dirPath: string, fileName: string, ncmId: number | null) => { | ||
| try { | ||
| const matchIndexDir = await getMatchIndexDir(); | ||
| const dirHash = createHash("md5").update(dirPath).digest("hex"); | ||
| const indexPath = join(matchIndexDir, `${dirHash}.json`); | ||
|
|
||
| let indexData: Record<string, number | null> = {}; | ||
|
|
||
| // 先尝试读取已有索引 | ||
| const exists = await access(indexPath).then(() => true).catch(() => false); | ||
| if (exists) { | ||
| const content = await readFile(indexPath, "utf-8"); | ||
| try { | ||
| indexData = JSON.parse(content); | ||
| } catch { | ||
| // 解析失败不阻断，直接覆盖 | ||
| } | ||
| } | ||
|
|
||
| // 更新记录 | ||
| indexData[fileName] = ncmId; | ||
|
|
||
| // 写入索引文件 | ||
| // 格式化输出方便用户必要时查看，也可最小化 | ||
| await writeFile(indexPath, JSON.stringify(indexData, null, 2), "utf-8"); | ||
| return { success: true }; | ||
| } catch (e) { | ||
| ipcLog.error(`Failed to save local match index for ${dirPath}:`, String(e)); | ||
| return { success: false, error: String(e) }; | ||
| } | ||
| } | ||
| ); |
There was a problem hiding this comment.
save-local-match-index IPC 处理函数存在竞态条件。如果多个请求同时针对同一个目录下的不同文件进行操作，它们会读取同一个索引文件，各自在内存中更新，然后写回。这可能导致后写入的操作覆盖掉先前的更改，造成数据丢失。
为了解决这个问题，建议引入一个基于文件路径的锁机制，确保对同一个索引文件的读-改-写操作是原子性的。可以使用一个 Map 来管理每个索引文件的 Promise 锁，将并发的写入操作串行化。
例如：
const saveIndexLocks = new Map<string, Promise<void>>();
ipcMain.handle("save-local-match-index", async (...) => {
// ... 获取 indexPath
const indexPath = join(matchIndexDir, `${dirHash}.json`);
const previousLock = saveIndexLocks.get(indexPath) || Promise.resolve();
const newLock = previousLock.then(async () => {
// ... 原有的读、改、写逻辑
});
saveIndexLocks.set(indexPath, newLock);
try {
await newLock;
return { success: true };
} catch (e) {
// ... 错误处理
return { success: false, error: String(e) };
} finally {
// 清理锁，防止内存泄漏
if (saveIndexLocks.get(indexPath) === newLock) {
saveIndexLocks.delete(indexPath);
}
}
});| cookie: req.cookies, | ||
| }; | ||
|
|
||
| serverLog.log(`🌐 Request NcmAPI: ${routerName} | params:`, JSON.stringify(params)); |
There was a problem hiding this comment.
The dynamicHandler logs the entire params object, which includes sensitive session cookies (req.cookies). This can leak authentication tokens to the application logs. It is recommended to redact or remove the cookie field before logging.
| serverLog.log(`🌐 Request NcmAPI: ${routerName} | params:`, JSON.stringify(params)); | |
| serverLog.log(`🌐 Request NcmAPI: ${routerName} | params:`, JSON.stringify({ ...params, cookie: params.cookie ? '[REDACTED]' : undefined })); |
| ipcMain.handle("read-local-lyric", async (_, lyricDirs: string[], id: number, songName?: string, artists?: string[]) => { | ||
| return musicMetadataService.readLocalLyric(lyricDirs, id, songName, artists); | ||
| }); | ||
|
|
||
| // 手动扫描本地 TTML 歌词目录，建立 ncmMusicId 映射缓存 | ||
| ipcMain.handle("scan-ttml-lyrics", async (_, lyricDirs: string[]) => { | ||
| try { | ||
| const count = await scanTtmlIdMapping(lyricDirs); | ||
| return { success: true, count }; | ||
| } catch (error: any) { | ||
| return { success: false, message: error?.message || String(error) }; | ||
| } | ||
| }); | ||
|
|
||
| // 尝试通过歌名快速在本地缓存中寻找对应的 TTML 文件信息并提取其关联的 ncmId | ||
| ipcMain.handle("match-local-ttml-by-name", async (_, lyricDirs: string[], songName: string) => { | ||
| return matchLocalTtmlByName(lyricDirs, songName); | ||
| }); |
There was a problem hiding this comment.
The IPC handlers read-local-lyric, scan-ttml-lyrics, and match-local-ttml-by-name accept arbitrary directory paths from the renderer and pass them to recursive file scanning functions (FastGlob). This allows a compromised renderer to trigger expensive disk-wide scans, leading to Denial of Service or information disclosure. Paths should be validated against an allow-list of user-defined music directories.
| if (cachedByName) { | ||
| const filePath = cachedByName.filePath; | ||
| try { | ||
| const fileStat = await stat(filePath); | ||
| if (fileStat.mtimeMs === cachedByName.mtime) { | ||
| result.ttml = await readFile(filePath, "utf-8"); | ||
| result.matchedNcmId = cachedByName.ncmIds.length > 0 ? cachedByName.ncmIds[0] : undefined; | ||
| ipcLog.info(`[readLocalLyric] 歌名缓存命中 TTML: ${filePath}`); | ||
| } else { | ||
| await cache.delete(filePath, { autoSave: false }); | ||
| isCacheDirty = true; | ||
| // @ts-ignore | ||
| cachedByName = undefined; | ||
| } | ||
| } catch (e) { | ||
| await cache.delete(filePath, { autoSave: false }); | ||
| isCacheDirty = true; | ||
| // @ts-ignore | ||
| cachedByName = undefined; | ||
| } | ||
| } |
There was a problem hiding this comment.
在 readLocalLyricImpl 函数中，有两处使用了 @ts-ignore 来抑制类型检查错误，这通常是不推荐的做法，并且这里的逻辑可以被简化。cachedByName 变量在类型收窄后被重新赋值为 undefined，但实际上后续代码并没有再次使用它，这使得代码意图不够清晰。
建议重构这部分逻辑，移除 @ts-ignore 和不必要的赋值操作，使代码更简洁易懂。
if (cachedByName) {
const filePath = cachedByName.filePath;
try {
const fileStat = await stat(filePath);
if (fileStat.mtimeMs === cachedByName.mtime) {
result.ttml = await readFile(filePath, "utf-8");
result.matchedNcmId = cachedByName.ncmIds.length > 0 ? cachedByName.ncmIds[0] : undefined;
ipcLog.info(`[readLocalLyric] 歌名缓存命中 TTML: ${filePath}`);
} else {
// 文件已更新，删除旧缓存
await cache.delete(filePath, { autoSave: false });
isCacheDirty = true;
}
} catch (e) {
// 访问文件失败，删除旧缓存
await cache.delete(filePath, { autoSave: false });
isCacheDirty = true;
}
}1 participant
Description
核心重构本地歌曲与云端元数据的匹配流程，并纠正了本地歌词文件的优先级。
主要改动:
/search/match接口，基于本地文件元数据进行更精确的网易云歌曲匹配。ncmId。