fix(core): 优化 File 组件处理逻辑并增强 OneBot 驱动层路径兼容性#5391
fix(core): 优化 File 组件处理逻辑并增强 OneBot 驱动层路径兼容性#5391Soulter merged 2 commits intoAstrBotDevs:masterfrom
Conversation
原因 (Necessity): 1. 内核一致性:AstrBot 内核的 Record 和 Video 组件均具备识别 `file:///` 协议头的逻辑,但 File 组件此前缺失此功能,导致行为不统一。 2. OneBot 协议合规:OneBot 11 标准要求本地文件路径必须使用 `file:///` 协议头。此前驱动层未对裸路径进行自动转换,导致发送本地文件时常触发 retcode 1200 (识别URL失败) 错误。 3. 容器环境适配:在 Docker 等路径隔离环境下,裸路径更容易因驱动或协议端的解析歧义而失效。 更改 (Changes): - [astrbot/core/message/components.py]: - 在 File.get_file() 中增加对 `file:///` 前缀的识别与剥离逻辑,使其与 Record/Video 组件行为对齐。 - [astrbot/core/platform/sources/aiocqhttp/aiocqhttp_message_event.py]: - 在发送文件前增加自动修正逻辑:若路径为绝对路径且未包含协议头,驱动层将自动补全 `file:///` 前缀。 - 对 http、base64 及已有协议头,确保不干扰原有的正常传输逻辑。 影响 (Impact): - 以完全兼容的方式增强了文件发送的鲁棒性。 - 解决了插件在发送日志等本地生成的压缩包时,因路径格式不规范导致的发送失败问题。
Summary of ChangesHello @SXP-Simon, 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! 此拉取请求旨在优化 AstrBot 框架中文件处理的逻辑和 OneBot 驱动层的路径兼容性。通过统一 Highlights
Changelog
Activity
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
|
There was a problem hiding this comment.
Hey - 我发现了 1 个问题,并给出了一些整体性的反馈:
file:URI 的生成和解析方式存在不一致:_from_segment_to_dict可能生成以file://(两个斜杠)开头的值,但File.get_file只会去掉file:///前缀(三个斜杠),这会导致在两个斜杠的情况下os.path.exists调用失败——这两者应该统一成一种规范格式,或者在处理时保持对称。_from_segment_to_dict中的 Windows 路径情况(例如C:\foo)会通过os.path.isabs检查,进入else分支,然后变成file:///C:\foo,但没有规范化分隔符,也没有使用正确的file:URI 转换方式,这可能会带来问题;建议使用pathlib.Path.as_uri(),或者在构造file:URI 时显式地把反斜杠转换为正斜杠。- 协议判断
not file_val.startswith(("http", "base64", "file://"))过于宽松(例如,它会把任何以http*或base64*开头的前缀都当作有协议);如果你只打算跳过真正的 URL scheme,建议检查是否包含"://",或者使用更具体的前缀,如"http://"、"https://"和"base64://",以避免误判。
面向 AI 代理的提示词
Please address the comments from this code review:
## Overall Comments
- There’s an inconsistency between how `file:` URIs are generated and parsed: `_from_segment_to_dict` may produce values starting with `file://` (two slashes), but `File.get_file` only strips the `file:///` prefix (three slashes), which will cause `os.path.exists` to fail for the two-slash case—these should be unified to a single canonical format or handled symmetrically.
- The Windows path case in `_from_segment_to_dict` (e.g., `C:\foo`) will pass `os.path.isabs`, fall into the `else` branch, and become `file:///C:\foo` without normalizing separators or using a proper `file:` URI conversion, which can be problematic; consider using `pathlib.Path.as_uri()` or explicitly normalizing backslashes to forward slashes when constructing `file:` URIs.
- The protocol guard `not file_val.startswith(("http", "base64", "file://"))` is quite loose (e.g., it treats any `http*` or `base64*` prefix as having a protocol); if you only intend to skip entries with real URL schemes, consider checking for `"://"` or more specific prefixes like `"http://"`, `"https://"`, and `"base64://"` to avoid misclassification.
## Individual Comments
### Comment 1
<location path="astrbot/core/platform/sources/aiocqhttp/aiocqhttp_message_event.py" line_range="49-58" />
<code_context>
if isinstance(segment, File):
# For File segments, we need to handle the file differently
d = await segment.to_dict()
+ file_val = d.get("data", {}).get("file", "")
+ if (
+ file_val
+ and os.path.isabs(file_val)
+ and not file_val.startswith(("http", "base64", "file://"))
+ ):
+ if file_val.startswith("/"):
+ d["data"]["file"] = f"file://{file_val}"
+ else:
+ d["data"]["file"] = f"file:///{file_val}"
return d
if isinstance(segment, Video):
</code_context>
<issue_to_address>
**issue (bug_risk):** The absolute-path to `file://` conversion may behave unexpectedly on Windows (drive letters, backslashes, and UNC paths).
On Windows, `os.path.isabs(file_val)` will treat `C:\path\file` and `\\server\share` as absolute, but the current logic produces invalid or nonstandard URIs (e.g. `file:///C:\path\file`, `file://///server\share`). If Windows is in scope, add explicit handling for drive-letter and UNC paths (normalizing to `file:///C:/path` and `file://server/share`). Otherwise, consider limiting this conversion to POSIX-style paths or clearly documenting that only POSIX paths are supported.
</issue_to_address>
帮我变得更有用!请对每条评论点 👍 或 👎,我会基于这些反馈改进后续的评审。
Original comment in English
Hey - I've found 1 issue, and left some high level feedback:
- There’s an inconsistency between how
file:URIs are generated and parsed:_from_segment_to_dictmay produce values starting withfile://(two slashes), butFile.get_fileonly strips thefile:///prefix (three slashes), which will causeos.path.existsto fail for the two-slash case—these should be unified to a single canonical format or handled symmetrically. - The Windows path case in
_from_segment_to_dict(e.g.,C:\foo) will passos.path.isabs, fall into theelsebranch, and becomefile:///C:\foowithout normalizing separators or using a properfile:URI conversion, which can be problematic; consider usingpathlib.Path.as_uri()or explicitly normalizing backslashes to forward slashes when constructingfile:URIs. - The protocol guard
not file_val.startswith(("http", "base64", "file://"))is quite loose (e.g., it treats anyhttp*orbase64*prefix as having a protocol); if you only intend to skip entries with real URL schemes, consider checking for"://"or more specific prefixes like"http://","https://", and"base64://"to avoid misclassification.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- There’s an inconsistency between how `file:` URIs are generated and parsed: `_from_segment_to_dict` may produce values starting with `file://` (two slashes), but `File.get_file` only strips the `file:///` prefix (three slashes), which will cause `os.path.exists` to fail for the two-slash case—these should be unified to a single canonical format or handled symmetrically.
- The Windows path case in `_from_segment_to_dict` (e.g., `C:\foo`) will pass `os.path.isabs`, fall into the `else` branch, and become `file:///C:\foo` without normalizing separators or using a proper `file:` URI conversion, which can be problematic; consider using `pathlib.Path.as_uri()` or explicitly normalizing backslashes to forward slashes when constructing `file:` URIs.
- The protocol guard `not file_val.startswith(("http", "base64", "file://"))` is quite loose (e.g., it treats any `http*` or `base64*` prefix as having a protocol); if you only intend to skip entries with real URL schemes, consider checking for `"://"` or more specific prefixes like `"http://"`, `"https://"`, and `"base64://"` to avoid misclassification.
## Individual Comments
### Comment 1
<location path="astrbot/core/platform/sources/aiocqhttp/aiocqhttp_message_event.py" line_range="49-58" />
<code_context>
if isinstance(segment, File):
# For File segments, we need to handle the file differently
d = await segment.to_dict()
+ file_val = d.get("data", {}).get("file", "")
+ if (
+ file_val
+ and os.path.isabs(file_val)
+ and not file_val.startswith(("http", "base64", "file://"))
+ ):
+ if file_val.startswith("/"):
+ d["data"]["file"] = f"file://{file_val}"
+ else:
+ d["data"]["file"] = f"file:///{file_val}"
return d
if isinstance(segment, Video):
</code_context>
<issue_to_address>
**issue (bug_risk):** The absolute-path to `file://` conversion may behave unexpectedly on Windows (drive letters, backslashes, and UNC paths).
On Windows, `os.path.isabs(file_val)` will treat `C:\path\file` and `\\server\share` as absolute, but the current logic produces invalid or nonstandard URIs (e.g. `file:///C:\path\file`, `file://///server\share`). If Windows is in scope, add explicit handling for drive-letter and UNC paths (normalizing to `file:///C:/path` and `file://server/share`). Otherwise, consider limiting this conversion to POSIX-style paths or clearly documenting that only POSIX paths are supported.
</issue_to_address>
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
astrbot/core/platform/sources/aiocqhttp/aiocqhttp_message_event.py
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Code Review
此拉取请求有效地解决了 File 组件处理 file:/// 前缀的不一致性,并增强了 OneBot 驱动层对本地文件路径的兼容性。astrbot/core/message/components.py 中的 get_file 方法现在能正确剥离 file:/// 前缀,使其行为与 Record 和 Video 组件保持一致。aiocqhttp_message_event.py 文件引入了在发送前自动为绝对本地文件路径添加 file:/// 前缀的逻辑,这对于 OneBot 11 标准合规性至关重要,并提高了容器环境中的鲁棒性。
| if file_val.startswith("/"): | ||
| d["data"]["file"] = f"file://{file_val}" | ||
| else: | ||
| d["data"]["file"] = f"file:///{file_val}" |
There was a problem hiding this comment.
aiocqhttp_message_event.py 中为绝对路径添加 file:// 前缀的逻辑需要修正,以完全符合 OneBot 11 标准。OneBot 11 标准要求本地文件路径使用 file:/// 协议头,其中路径部分应为绝对路径。
当前的实现中:
- 如果
file_val是 Unix-like 绝对路径(例如/home/user/file.txt),if file_val.startswith("/")分支会将其转换为file:////home/user/file.txt(四个斜杠)。这与file:///标准不符。 - 如果
file_val是 Windows 绝对路径(例如C:\Users\user\file.txt),else分支会将其转换为file:///C:\Users\user\file.txt(三个斜杠),这符合标准。
为了确保所有绝对本地路径都以 file:/// 格式正确生成,无论操作系统如何,都应该先移除路径中可能存在的起始斜杠,然后再统一添加 file:/// 前缀。
例如:
/home/user/file.txt应变为file:///home/user/file.txtC:\Users\user\file.txt应变为file:///C:\Users\user\file.txt
normalized_path = file_val.lstrip("/")
d["data"]["file"] = f"file:///{normalized_path}"
原因 (Necessity):
1. 修复原生路径与 URI 转换在 Windows 下的不对称问题。
2. 规范化 file: 协议头处理,确保符合 RFC 标准并能在 Linux/Windows 间稳健切换。
3. 增强协议判定准确度,防止对普通绝对路径的误处理。
更改 (Changes):
- [astrbot/core/platform/sources/aiocqhttp]:
- 弃用手动拼接,改用 `pathlib.Path.as_uri()` 生成标准 URI。
- 将协议检测逻辑从前缀匹配优化为包含性检测 ("://")。
- [astrbot/core/message/components]:
- 重构 `File.get_file` 解析逻辑,支持对称处理 2/3 斜杠格式。
- 针对 Windows 环境增加了对 `file:///C:/` 格式的自动修正,避免 `os.path` 识别失效。
- [data/plugins/astrbot_plugin_logplus]:
- 在直接 API 调用中同步应用 URI 规范化处理。
影响 (Impact):
- 解决 Docker 环境中因路径不规范导致的 "识别URL失败" 报错。
- 提升了本体框架在 Windows 系统下的文件操作鲁棒性。
|
Generated docs update PR (pending manual review): AI change summary:
Experimental bot notice:
|
原因 (Necessity):
file:///协议头的逻辑,但 File 组件此前缺失此功能,导致行为不统一。file:///协议头。此前驱动层未对裸路径进行自动转换,导致发送本地文件时常触发 retcode 1200 (识别URL失败) 错误。Modifications / 改动点
更改 (Changes):
file:///前缀的识别与剥离逻辑,使其与 Record/Video 组件行为对齐。file:///前缀。影响 (Impact):
Screenshots or Test Results / 运行截图或测试结果
Checklist / 检查清单
requirements.txt和pyproject.toml文件相应位置。/ I have ensured that no new dependencies are introduced, OR if new dependencies are introduced, they have been added to the appropriate locations inrequirements.txtandpyproject.toml.Summary by Sourcery
在核心 File 组件和 OneBot(aiocqhttp)驱动之间统一文件路径处理方式,以提升本地文件发送的可靠性和协议兼容性。
Bug 修复:
file:///URI scheme 的本地文件时无法正确定位到磁盘路径的问题。file://URI。功能增强:
File.get_file中标准化文件路径规范化逻辑,使其在处理file:///URI 时的行为与其他媒体组件保持一致。Original summary in English
Summary by Sourcery
Align file path handling across core File components and the OneBot (aiocqhttp) driver to improve local file sending robustness and protocol compatibility.
Bug Fixes:
Enhancements: