fix(drivers/onedrive_sharelink): respect root_folder_path for subdirectory access - #2840
fix(drivers/onedrive_sharelink): respect root_folder_path for subdirectory access#2840xireiki wants to merge 1 commit into
Conversation
…cess - Add relativePath() to strip RootFolderPath prefix from virtual paths - Add effectiveDriveRootPath() to compute drive-relative path from RootFolderPath - Override rootFolder in getFiles() when RootFolderPath is configured - Apply relativePath() in List, MakeDir, Put, GetDirectUploadInfo - Store listURL for path computation against document library root Co-authored-by: GitHub Copilot <copilot@github.com>
There was a problem hiding this comment.
Pull request overview
This PR fixes path handling in the onedrive_sharelink driver so that when root_folder_path is configured, listing/creating/uploading targets the configured subdirectory correctly (and avoids double path-prefixing that could lead to 404s).
Changes:
- Add
relativePath()to translate OpenList virtual paths into paths relative to the share root, and use it acrossList,MakeDir,Put,GetDirectUploadInfo. - Adjust Graph API path building via
effectiveDriveRootPath()whenRootFolderPathis set. - Update
getFiles()to treatRootFolderPathas the initial root folder for the GraphQL query.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| drivers/onedrive_sharelink/driver.go | Adds root-relative path conversion and adjusts drive API base-path computation for root_folder_path. |
| drivers/onedrive_sharelink/util.go | Updates GraphQL listing logic to anchor queries at root_folder_path. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if d.RootFolderPath != "" && d.RootFolderPath != "/" { | ||
| rootFolder = d.RootFolderPath | ||
| } |
| func (d *OnedriveSharelink) effectiveDriveRootPath() string { | ||
| if d.listURL == "" || d.RootFolderPath == "" || d.RootFolderPath == "/" { | ||
| return d.driveRootPath | ||
| } | ||
| root := strings.TrimRight(d.RootFolderPath, "/") | ||
| list := strings.TrimRight(d.listURL, "/") | ||
| if root == list { | ||
| return "/" | ||
| } | ||
| prefix := list + "/" | ||
| if strings.HasPrefix(root+"/", prefix) { | ||
| return utils.FixAndCleanPath(strings.TrimPrefix(root, list)) | ||
| } | ||
| return d.driveRootPath | ||
| } |
PIKACHUIM
left a comment
There was a problem hiding this comment.
🙏 感谢贡献
感谢 @xireiki 提交此PR!我已完成代码评审,以下是评审结果。
🤖 AI 自动审核声明
本评审报告由 AI 自动生成,当前使用 Claude Opus 5 模型进行分析,部分复杂场景可能辅助使用 ChatGPT、DeepSeek 等模型进行交叉验证。
⚠️ AI 分析结果仅供参考,可能存在误判或遗漏。如您发现任何问题或有不同意见,欢迎随时提出讨论和纠正。
⚠️ 重要提醒:即使 AI 评审认为代码质量良好且建议合并,最终是否合并仍需由项目维护者进行人工判定。项目维护者会综合考虑代码质量、项目规划、技术方向、团队资源等多方面因素做出决策。
📖 PR背景与需求
PR标题:fix(drivers/onedrive_sharelink): respect root_folder_path for subdirectory access
需求说明:修复 OneDrive ShareLink 驱动在配置 root_folder_path 后无法正确访问子目录的问题。当用户配置了根文件夹路径后,挂载点应该指向该子目录,而不是分享链接的根目录。
预期目标:
- 设置「根文件夹路径」后,挂载点正确指向该子目录
- 文件夹大小统计不再因路径双重拼接返回 404
- 所有文件操作(列表、创建目录、上传、直接上传)都正确处理路径转换
📋 问题摘要
- ✅ 功能性:功能完整,解决了实际问题
- ✅ 代码质量:路径转换逻辑清晰
⚠️ 改进建议:路径解析逻辑需要增强,避免边界情况
📂 逐文件分析
drivers/onedrive_sharelink/driver.go
改动意图:新增路径转换逻辑,将 OpenList 的完整虚拟路径转换为相对于 SharePoint 根文件夹的路径。
代码修改逻辑:
- 新增字段
listURL:存储 SharePoint 文档库的 URL,用于计算有效的驱动根路径 - 新增
relativePath():将 OpenList 传入的完整路径转换为相对于RootFolderPath的路径 - 新增
effectiveDriveRootPath():从用户配置的RootFolderPath(SharePoint 服务器相对路径)中提取文档库之外的部分 - 修改
List()、MakeDir()、Put()、GetDirectUploadInfo():统一使用relativePath()转换路径 - 修改
drivePathAPIURL():在配置了RootFolderPath时使用effectiveDriveRootPath()作为基准
合理性评估:
-
✅ 优点:
- 路径转换逻辑集中在
relativePath()方法,便于维护 - 修改范围精准,仅影响需要路径转换的方法
- 向后兼容,未配置
RootFolderPath时行为不变
- 路径转换逻辑集中在
-
⚠️ 疑问:-
relativePath()的前缀匹配逻辑不够健壮:- 当前代码:
strings.HasPrefix(vpath, root+"/") - 问题:如果
vpath恰好等于root但不是以/结尾,第二个分支会返回virtualPath(未清理) - 场景:
vpath="/a/b",root="/a/b"时,第一个if返回/(正确),但如果vpath="/a/bc",root="/a/b"时,第三个return会返回/a/bc(应该不匹配)
- 当前代码:
-
effectiveDriveRootPath()的逻辑复杂:- 依赖
listURL和RootFolderPath的前缀关系 - 如果
RootFolderPath不是listURL的子路径,直接返回d.driveRootPath,这可能导致不一致
- 依赖
-
详细建议:
-
增强
relativePath()的路径匹配逻辑:func (d *OnedriveSharelink) relativePath(virtualPath string) string { if d.RootFolderPath == "" || d.RootFolderPath == "/" { return virtualPath } root := utils.FixAndCleanPath(d.RootFolderPath) vpath := utils.FixAndCleanPath(virtualPath) if vpath == root { return "/" } // 使用 utils.IsSubPath 或明确的前缀+分隔符检查 if strings.HasPrefix(vpath+"/", root+"/") { rel := strings.TrimPrefix(vpath, root) return utils.FixAndCleanPath(rel) } // 如果不匹配,记录警告并返回原始路径 log.Warnf("onedrive_sharelink: path %q is outside configured root %q", virtualPath, d.RootFolderPath) return virtualPath }
-
简化
effectiveDriveRootPath()或增加验证:func (d *OnedriveSharelink) effectiveDriveRootPath() string { if d.listURL == "" || d.RootFolderPath == "" || d.RootFolderPath == "/" { return d.driveRootPath } root := utils.FixAndCleanPath(d.RootFolderPath) list := utils.FixAndCleanPath(d.listURL) if root == list { return "/" } // 使用更健壮的路径裁剪逻辑 if strings.HasPrefix(root+"/", list+"/") { return utils.FixAndCleanPath(strings.TrimPrefix(root, list)) } // 不匹配时记录警告 log.Warnf("onedrive_sharelink: RootFolderPath %q is not under listURL %q", d.RootFolderPath, d.listURL) return d.driveRootPath }
-
在
Init()中验证RootFolderPath的合法性:func (d *OnedriveSharelink) Init(ctx context.Context) error { // ... 现有初始化逻辑 ... // 验证 RootFolderPath 格式 if d.RootFolderPath != "" && d.RootFolderPath != "/" { cleaned := utils.FixAndCleanPath(d.RootFolderPath) if !strings.HasPrefix(cleaned, "/") { return fmt.Errorf("root_folder_path must be an absolute path, got %q", d.RootFolderPath) } } return nil }
drivers/onedrive_sharelink/util.go
改动意图:在 getFiles() 中,如果用户配置了 RootFolderPath,将其覆盖到 rootFolder 变量,使 GraphQL 查询目标正确的子目录。
代码修改逻辑:
- 在计算出初始
rootFolder后,检查d.RootFolderPath是否配置 - 如果配置了且不是
/,直接覆盖rootFolder变量
合理性评估:
- ✅ 优点:改动最小化,仅在需要的地方覆盖值
⚠️ 疑问:覆盖逻辑可能与之前的解析逻辑冲突,建议在注释中说明这个覆盖是有意为之
🎯 总体评价
功能性:⭐⭐⭐⭐ - 功能完整,解决了实际问题,但路径处理逻辑需要增强
安全性:⭐⭐⭐⭐ - 无明显安全隐患,路径清理到位
代码质量:⭐⭐⭐ - 核心逻辑清晰,但路径匹配需要更健壮的实现
实现方案:⭐⭐⭐⭐ - 方案合理,改动最小化,向后兼容
建议操作:
- ✅ Approve(建议合并)
- 🔄 Request Changes(需要修改)
- ❌ Close(建议关闭)
理由:功能设计合理,解决了实际问题,但路径匹配逻辑存在边界情况漏洞。建议增强 relativePath() 的前缀匹配逻辑,使用更健壮的路径裁剪方法,并在 Init() 中验证 RootFolderPath 的合法性。修复后即可合并。
Next Steps / 后续建议:
- 增强
relativePath()的路径匹配逻辑(使用utils.IsSubPath或明确的前缀+分隔符检查) - 在
effectiveDriveRootPath()中增加路径不匹配时的警告日志 - 在
Init()中验证RootFolderPath的格式(必须是绝对路径) - 补充单元测试,覆盖边界情况(路径前缀匹配、
/avs/ab、RootFolderPath不在listURL下等)
再次感谢你的贡献!修复后这个功能将大大提升 OneDrive ShareLink 驱动的易用性。👏
Summary / 摘要
修复 onedrive_sharelink 驱动在配置
root_folder_path后无法正确访问子目录的问题。用户可感知的变化:
实现变化:
driver.go:新增relativePath()、effectiveDriveRootPath()方法;List、MakeDir、Put、GetDirectUploadInfo统一使用relativePath()转换路径;drivePathAPIURL()在配置RootFolderPath时改用effectiveDriveRootPath()作为基准util.go:getFiles()在计算出初始 rootFolder 后,若用户配置了RootFolderPath则覆盖之This PR has breaking changes.
/ 此 PR 包含破坏性变更。
This PR changes public API, config, storage format, or migration behavior.
/ 此 PR 修改了公开 API、配置、存储格式或迁移行为。
This PR requires corresponding changes in related repositories.
/ 此 PR 需要关联仓库同步修改。
Related repository PRs / 关联仓库 PR:
Related Issues / 关联 Issue
Testing / 测试
go vet ./drivers/onedrive_sharelink/...go build ./drivers/onedrive_sharelink/...Checklist / 检查清单
/ 我已阅读 CONTRIBUTING。
/ 我确认此贡献符合仓库许可证、贡献规范和行为准则。
gofmt,go fmt, orprettierwhere applicable./ 我已按适用情况使用
gofmt、go fmt或prettier格式化变更代码。/ 我已在适用情况下请求相关维护者或代码所有者审查。
AI Disclosure / AI 使用声明
/ 此 PR 包含 AI 辅助内容。
Tools used / 使用工具:
Usage scope / 使用范围:
Code generation / 代码生成
Refactoring / 重构
Documentation / 文档
Tests / 测试
Translation / 翻译
Review assistance / 审查辅助
I have reviewed and validated all AI-assisted content included in this PR.
/ 我已审核并验证此 PR 中的所有 AI 辅助内容。
I have ensured that all AI-assisted commits include
Co-Authored-Byattribution./ 我已确保所有 AI 辅助提交都包含
Co-Authored-By归属信息。I can reproduce all AI-assisted content included in this PR without any AI tools.
/ 我可以在没有任何 AI 工具的情况下重现此 PR 中包含的所有 AI 辅助内容。