fix: only allow HTTPS URLs to pass through directly in LINE adapter#5697
Conversation
There was a problem hiding this comment.
Hey - 我在这里给出了一些整体性的反馈:
- 针对仅支持 HTTPS 的检查逻辑,在多个
_resolve_*_url方法中有重复;可以考虑抽取一个小的辅助函数(例如_is_https_url(candidate: str) -> bool),以减少重复代码,并在未来逻辑发生变化时(比如支持更多协议或增加校验)能保持行为一致。 - 为了方便后续排查问题,当某个
http://URL 被拒绝、并且代码回退到调用register_to_file_service()时,可以考虑输出一条 debug 级别的日志,这样更容易追踪那些意外出现的非 HTTPS 输入。
供 AI 代理使用的提示词
Please address the comments from this code review:
## Overall Comments
- The HTTPS-only checks are duplicated across several `_resolve_*_url` methods; consider extracting a small helper (e.g., `_is_https_url(candidate: str) -> bool`) to reduce repetition and ensure consistent behavior if the logic ever evolves (such as supporting additional schemes or validation).
- For future debugging, it may be useful to emit a debug-level log when an `http://` URL is rejected and the code falls back to `register_to_file_service()`, so unexpected non-HTTPS inputs are easier to trace.帮我变得更有用!请在每条评论上点 👍 或 👎,我会根据你的反馈来改进之后的 Review。
Original comment in English
Hey - I've left some high level feedback:
- The HTTPS-only checks are duplicated across several
_resolve_*_urlmethods; consider extracting a small helper (e.g.,_is_https_url(candidate: str) -> bool) to reduce repetition and ensure consistent behavior if the logic ever evolves (such as supporting additional schemes or validation). - For future debugging, it may be useful to emit a debug-level log when an
http://URL is rejected and the code falls back toregister_to_file_service(), so unexpected non-HTTPS inputs are easier to trace.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The HTTPS-only checks are duplicated across several `_resolve_*_url` methods; consider extracting a small helper (e.g., `_is_https_url(candidate: str) -> bool`) to reduce repetition and ensure consistent behavior if the logic ever evolves (such as supporting additional schemes or validation).
- For future debugging, it may be useful to emit a debug-level log when an `http://` URL is rejected and the code falls back to `register_to_file_service()`, so unexpected non-HTTPS inputs are easier to trace.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
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! This pull request enforces the use of HTTPS URLs in the LINE adapter to comply with the LINE Messaging API's requirements for media URLs. It fixes a bug where HTTP URLs were being passed through, leading to errors. The changes ensure that only HTTPS URLs are directly used, while HTTP URLs are handled by the 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.
Code Review
This pull request correctly fixes a bug where http:// URLs for media were causing errors with the LINE API. The changes ensure only https:// URLs are passed through directly, with other schemes correctly falling back to the file service. The implementation is correct. I've added one comment regarding potential code duplication which could be addressed to improve maintainability.
|
No docs changes were generated in this run (docs repo had no updates). Docs repo: AstrBotDevs/AstrBot-docs AI change summary (not committed):
Experimental bot notice:
|
The LINE Messaging API strictly requires all media URLs (images, audio, video, files) to use HTTPS. The previous
_resolve_*_urlmethods in the LINE adapter treatedhttp://andhttps://URLs equally, passing both through directly without any conversion. This caused a 400 error (Must be a valid HTTPS URL) .Modifications / 改动点
astrbot/core/platform/sources/line/line_event.py:Changed all five URL resolution methods (
_resolve_image_url,_resolve_record_url,_resolve_video_url,_resolve_video_preview_url,_resolve_file_url) to only pass throughhttps://URLs directly. Anyhttp://URL now falls through toregister_to_file_service().Screenshots or Test Results / 运行截图或测试结果
Before:

After:
No Error
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
错误修复:
Original summary in English
Summary by Sourcery
Bug Fixes: