✨ feat: 本地添加SMB\NFS\WebDav\FTP #759
Conversation
Summary of ChangesHello @kazukokawagawa, 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! 此拉取请求为应用程序引入了强大的远程文件夹管理功能，使用户能够添加和管理来自SMB、NFS、WebDAV和FTP服务器的音乐目录。这极大地扩展了应用程序的音乐源，不再局限于本地存储。通过新的UI组件和后端服务，用户可以方便地配置、测试和集成远程音乐库，从而提升了用户体验和应用程序的灵活性。 Highlights
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
本次 PR 添加了对 SMB、NFS、WebDAV 和 FTP 远程文件夹的支持，这是一个很棒的功能。代码结构清晰，包含了后端连接测试服务和前端配置界面。我的审查意见主要集中在以下几个方面：
- 协议实现的健壮性：建议使用成熟的第三方库来处理 WebDAV 和 FTP 协议，以增强稳定性和安全性。
- 跨平台兼容性：当前的 SMB/NFS 路径处理是 Windows 特定的，这在跨平台的 Electron 应用中可能引发问题。
- 代码质量：修复了一个 SMB 路径构建的 bug，并提出了一些关于代码重用、异步代码模式和唯一 ID 生成安全性的改进建议。
| * 测试 SMB/NFS 网络路径 | ||
| * Windows 上 SMB 和 NFS 可以通过 UNC 路径直接访问 | ||
| */ | ||
| private async testNetworkPath(path: string): Promise<TestConnectionResult> { | ||
| try { | ||
| // 验证路径格式 | ||
| if (!path.startsWith("\\\\")) { | ||
| return { success: false, message: "路径格式错误，应为 \\\\server\\share 格式" }; | ||
| } |
There was a problem hiding this comment.
此处的 SMB/NFS 路径验证（path.startsWith("\\\\")）是 Windows 特定的 UNC 路径格式。由于 Electron 是一个跨平台框架，这种实现将导致在 macOS 和 Linux 系统上无法使用该功能，因为它们使用不同的网络路径挂载方式（例如 /Volumes/share）。
为了保证跨平台兼容性，建议：
- 为不同操作系统提供不同的路径处理逻辑。
- 或者，在文档和界面中明确指出此功能目前仅支持 Windows。
| /** | ||
| * 测试 WebDAV 连接 | ||
| * 使用 HTTP OPTIONS 请求检测 WebDAV 服务 | ||
| */ | ||
| private async testWebDav(params: TestConnectionParams): Promise<TestConnectionResult> { | ||
| return new Promise((resolve) => { | ||
| try { | ||
| const url = new URL(params.path); | ||
| const isHttps = url.protocol === "https:"; | ||
| const httpModule = isHttps ? require("https") : require("http"); | ||
|
|
||
| const options = { | ||
| hostname: url.hostname, | ||
| port: url.port || (isHttps ? 443 : 80), | ||
| path: url.pathname || "/", | ||
| method: "OPTIONS", | ||
| timeout: 10000, | ||
| headers: {} as Record<string, string>, | ||
| }; | ||
|
|
||
| // 添加认证 | ||
| if (params.username && params.password) { | ||
| const auth = Buffer.from(`${params.username}:${params.password}`).toString("base64"); | ||
| options.headers["Authorization"] = `Basic ${auth}`; | ||
| } | ||
|
|
||
| const req = httpModule.request(options, (res: { statusCode: number; headers: Record<string, string> }) => { | ||
| // WebDAV 服务通常会在 DAV header 中返回支持的版本 | ||
| const davHeader = res.headers["dav"]; | ||
| if (res.statusCode >= 200 && res.statusCode < 400) { | ||
| if (davHeader) { | ||
| resolve({ success: true, message: `WebDAV 连接成功 (DAV: ${davHeader})` }); | ||
| } else { | ||
| resolve({ success: true, message: "HTTP 服务可访问（可能支持 WebDAV）" }); | ||
| } | ||
| } else if (res.statusCode === 401) { | ||
| resolve({ success: false, message: "认证失败，请检查用户名和密码" }); | ||
| } else { | ||
| resolve({ success: false, message: `服务器返回错误: ${res.statusCode}` }); | ||
| } | ||
| }); | ||
|
|
||
| req.on("error", (err: Error) => { | ||
| resolve({ success: false, message: `连接失败: ${err.message}` }); | ||
| }); | ||
|
|
||
| req.on("timeout", () => { | ||
| req.destroy(); | ||
| resolve({ success: false, message: "连接超时" }); | ||
| }); | ||
|
|
||
| req.end(); | ||
| } catch (error) { | ||
| resolve({ success: false, message: `URL 格式错误: ${(error as Error).message}` }); | ||
| } | ||
| }); | ||
| } | ||
|
|
||
| /** | ||
| * 测试 FTP 连接 | ||
| * 使用 TCP Socket 检测 FTP 服务器 | ||
| */ | ||
| private async testFtp(params: TestConnectionParams): Promise<TestConnectionResult> { | ||
| return new Promise((resolve) => { | ||
| try { | ||
| const url = new URL(params.path); | ||
| const net = require("net"); | ||
| const port = parseInt(url.port) || 21; | ||
|
|
||
| const socket = net.createConnection({ host: url.hostname, port, timeout: 10000 }, () => { | ||
| // FTP 服务器会发送欢迎消息 | ||
| }); | ||
|
|
||
| let welcomed = false; | ||
|
|
||
| socket.on("data", (data: Buffer) => { | ||
| const response = data.toString(); | ||
| if (response.startsWith("220")) { | ||
| welcomed = true; | ||
|
|
||
| // 如果有认证信息，尝试登录 | ||
| if (params.username) { | ||
| socket.write(`USER ${params.username}\r\n`); | ||
| } else { | ||
| socket.end(); | ||
| resolve({ success: true, message: "FTP 服务器可访问" }); | ||
| } | ||
| } else if (response.startsWith("331") && params.password) { | ||
| // 需要密码 | ||
| socket.write(`PASS ${params.password}\r\n`); | ||
| } else if (response.startsWith("230")) { | ||
| // 登录成功 | ||
| socket.write("QUIT\r\n"); | ||
| resolve({ success: true, message: "FTP 登录成功" }); | ||
| } else if (response.startsWith("530") || response.startsWith("430")) { | ||
| // 登录失败 | ||
| socket.end(); | ||
| resolve({ success: false, message: "FTP 认证失败" }); | ||
| } | ||
| }); | ||
|
|
||
| socket.on("error", (err: Error) => { | ||
| resolve({ success: false, message: `连接失败: ${err.message}` }); | ||
| }); | ||
|
|
||
| socket.on("timeout", () => { | ||
| socket.destroy(); | ||
| resolve({ success: false, message: "连接超时" }); | ||
| }); | ||
|
|
||
| socket.on("close", () => { | ||
| if (!welcomed) { | ||
| resolve({ success: false, message: "无法连接到 FTP 服务器" }); | ||
| } | ||
| }); | ||
| } catch (error) { | ||
| resolve({ success: false, message: `URL 格式错误: ${(error as Error).message}` }); | ||
| } | ||
| }); | ||
| } |
There was a problem hiding this comment.
您手动实现了 WebDAV 和 FTP 的连接测试逻辑，虽然这展示了您对协议的理解，但通常存在风险。这些协议有很多边缘情况和服务器实现差异，手动处理容易出错且难以维护。
pnpm-lock.yaml 文件显示已经添加了 webdav 和 basic-ftp 这两个库。建议直接使用这些成熟的第三方库来代替手动实现。这样做可以：
- 提高健壮性：库通常能更好地处理各种服务器响应和网络问题（例如 FTP 的被动模式）。
- 增强安全性：库的维护者会持续跟进安全漏洞。
- 简化代码：减少大量底层协议处理的代码，使
RemoteFolderService更专注于业务逻辑。
| case "smb": | ||
| // SMB 使用 UNC 路径: \\host\share\path | ||
| return `\\\\${host}\\${cleanPath}`; |
There was a problem hiding this comment.
这里构建 SMB 的 UNC 路径时存在一个 bug。cleanPath 只是移除了路径开头的斜杠，但没有将路径中所有的 / 替换为 \。这会导致在 Windows 上生成一个无效的路径，例如 \\server\share/music。
建议使用 replace 并配合正则表达式全局替换斜杠。
case "smb":
// SMB 使用 UNC 路径: \\host\share\path
return `\\\\${host}\\${cleanPath.replace(/\//g, "\\")}`;
| private async testWebDav(params: TestConnectionParams): Promise<TestConnectionResult> { | ||
| return new Promise((resolve) => { | ||
| try { | ||
| const url = new URL(params.path); | ||
| const isHttps = url.protocol === "https:"; | ||
| const httpModule = isHttps ? require("https") : require("http"); | ||
|
|
||
| const options = { | ||
| hostname: url.hostname, | ||
| port: url.port || (isHttps ? 443 : 80), | ||
| path: url.pathname || "/", | ||
| method: "OPTIONS", | ||
| timeout: 10000, | ||
| headers: {} as Record<string, string>, | ||
| }; | ||
|
|
||
| // 添加认证 | ||
| if (params.username && params.password) { | ||
| const auth = Buffer.from(`${params.username}:${params.password}`).toString("base64"); | ||
| options.headers["Authorization"] = `Basic ${auth}`; | ||
| } | ||
|
|
||
| const req = httpModule.request(options, (res: { statusCode: number; headers: Record<string, string> }) => { | ||
| // WebDAV 服务通常会在 DAV header 中返回支持的版本 | ||
| const davHeader = res.headers["dav"]; | ||
| if (res.statusCode >= 200 && res.statusCode < 400) { | ||
| if (davHeader) { | ||
| resolve({ success: true, message: `WebDAV 连接成功 (DAV: ${davHeader})` }); | ||
| } else { | ||
| resolve({ success: true, message: "HTTP 服务可访问（可能支持 WebDAV）" }); | ||
| } | ||
| } else if (res.statusCode === 401) { | ||
| resolve({ success: false, message: "认证失败，请检查用户名和密码" }); | ||
| } else { | ||
| resolve({ success: false, message: `服务器返回错误: ${res.statusCode}` }); | ||
| } | ||
| }); | ||
|
|
||
| req.on("error", (err: Error) => { | ||
| resolve({ success: false, message: `连接失败: ${err.message}` }); | ||
| }); | ||
|
|
||
| req.on("timeout", () => { | ||
| req.destroy(); | ||
| resolve({ success: false, message: "连接超时" }); | ||
| }); | ||
|
|
||
| req.end(); | ||
| } catch (error) { | ||
| resolve({ success: false, message: `URL 格式错误: ${(error as Error).message}` }); | ||
| } | ||
| }); | ||
| } |
There was a problem hiding this comment.
在 async 函数中，testWebDav 方法使用了 new Promise 构造函数来包装异步操作，这是一种反模式（Promise constructor anti-pattern）。它可以被重构为使用纯 async/await 语法，使代码更简洁、更易于理解和维护。
此外，将 require("https") 和 require("http") 放在方法内部会使得每次调用都执行一次 require。建议将它们移至文件顶部，以便模块只被加载一次。
private async testWebDav(params: TestConnectionParams): Promise<TestConnectionResult> {
try {
const url = new URL(params.path);
const isHttps = url.protocol === "https:";
const httpModule = isHttps ? require("https") : require("http");
const options = {
hostname: url.hostname,
port: url.port || (isHttps ? 443 : 80),
path: url.pathname || "/",
method: "OPTIONS",
timeout: 10000,
headers: {} as Record<string, string>,
};
if (params.username && params.password) {
const auth = Buffer.from(`${params.username}:${params.password}`).toString("base64");
options.headers["Authorization"] = `Basic ${auth}`;
}
return await new Promise<TestConnectionResult>((resolve) => {
const req = httpModule.request(options, (res: { statusCode: number; headers: Record<string, string> }) => {
const davHeader = res.headers["dav"];
if (res.statusCode >= 200 && res.statusCode < 400) {
if (davHeader) {
resolve({ success: true, message: `WebDAV 连接成功 (DAV: ${davHeader})` });
} else {
resolve({ success: true, message: "HTTP 服务可访问（可能支持 WebDAV）" });
}
} else if (res.statusCode === 401) {
resolve({ success: false, message: "认证失败，请检查用户名和密码" });
} else {
resolve({ success: false, message: `服务器返回错误: ${res.statusCode}` });
}
});
req.on("error", (err: Error) => {
resolve({ success: false, message: `连接失败: ${err.message}` });
});
req.on("timeout", () => {
req.destroy();
resolve({ success: false, message: "连接超时" });
});
req.end();
});
} catch (error) {
return { success: false, message: `URL 格式错误: ${(error as Error).message}` };
}
}| case "webdav": | ||
| // WebDAV URL | ||
| const webdavPort = port || defaultPorts.webdav; | ||
| const webdavAuth = username ? `${username}:${password}@` : ""; | ||
| return `https://${webdavAuth}${host}:${webdavPort}${path.startsWith("/") ? path : "/" + path}`; |
There was a problem hiding this comment.
为 WebDAV URL 硬编码了 https:// 协议，这会限制用户连接到仅支持 http 的 WebDAV 服务器。后端服务 RemoteFolderService 实际上是同时支持 http 和 https 的。
建议允许用户选择或指定协议。一个简单的改进是，如果用户在“服务器地址”字段中手动输入了 http://，则尊重用户的输入。
| export const nanoid = (size: number = 16): string => { | ||
| const chars = "ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789"; | ||
| let id = ""; | ||
| for (let i = 0; i < size; i++) { | ||
| id += chars.charAt(Math.floor(Math.random() * chars.length)); | ||
| } | ||
| return id; | ||
| }; |
There was a problem hiding this comment.
当前的 nanoid 实现使用了 Math.random()，它不是一个密码学安全的随机数生成器。对于需要保证高度唯一性的 ID，这可能会增加冲突的风险（尽管在当前场景下风险较低）。
为了提高健壮性，建议使用 window.crypto.getRandomValues()，这是一个浏览器提供的密码学安全随机数生成器接口。这样可以在不引入新依赖的情况下，生成更高质量的随机 ID。
| export const nanoid = (size: number = 16): string => { | |
| const chars = "ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789"; | |
| let id = ""; | |
| for (let i = 0; i < size; i++) { | |
| id += chars.charAt(Math.floor(Math.random() * chars.length)); | |
| } | |
| return id; | |
| }; | |
| export const nanoid = (size: number = 16): string => { | |
| const chars = "ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789"; | |
| const bytes = window.crypto.getRandomValues(new Uint8Array(size)); | |
| let id = ""; | |
| for (let i = 0; i < size; i++) { | |
| id += chars[bytes[i] % chars.length]; | |
| } | |
| return id; | |
| }; |
src/views/Local/layout.vue
Outdated
| // 添加文件夹下拉选项 | ||
| const addFolderOptions: DropdownOption[] = [ | ||
| { label: "选择本地文件夹", key: "local", icon: renderIcon("Folder") }, | ||
| { label: "添加SMB", key: "smb", icon: renderIcon("Link") }, | ||
| { label: "添加FTP", key: "ftp", icon: renderIcon("Link") }, | ||
| { label: "添加NFS", key: "nfs", icon: renderIcon("Link") }, | ||
| { label: "添加WebDAV", key: "webdav", icon: renderIcon("Cloud") }, | ||
| ]; | ||
|
|
||
| // 处理添加文件夹选择 | ||
| const handleAddFolderSelect = (key: string) => { | ||
| switch (key) { | ||
| case "local": | ||
| changeLocalMusicPath(); | ||
| break; | ||
| case "smb": | ||
| openAddRemoteFolder("smb"); | ||
| break; | ||
| case "ftp": | ||
| openAddRemoteFolder("ftp"); | ||
| break; | ||
| case "nfs": | ||
| openAddRemoteFolder("nfs"); | ||
| break; | ||
| case "webdav": | ||
| openAddRemoteFolder("webdav"); | ||
| break; | ||
| } | ||
| }; |
There was a problem hiding this comment.
此处的 addFolderOptions 和 handleAddFolderSelect 逻辑与 src/components/Setting/LocalSetting.vue 文件中的代码完全重复。这违反了 DRY (Don't Repeat Yourself) 原则，并可能导致未来维护困难（需要同步修改两处）。
建议将这部分共享逻辑提取到一个可复用的 Vue Composable 文件中（例如 useFolderActions.ts），然后在两个组件中导入和使用。
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
No description provided.