fix: qqofficial_markdown_message#5476
Conversation
dosubot
bot
added
the
size:M
Summary of ChangesHello @chawu691, 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! 此拉取请求旨在增强与QQ官方平台的集成，通过引入智能消息类型检测机制，解决了自定义模板消息发送失败和普通文本消息处理不当的问题。这一改进确保了不同类型的消息都能被正确构造和发送，从而提升了系统的稳定性和用户体验。 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
|
|
Related Documentation Checked 1 published document(s) in 1 knowledge base(s). No updates required. How did I do? Any feedback? |
dosubot
bot
added
the
area:platform
![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 modifies the message sending logic for the QQ Official Platform, adding detection for custom template messages and sending them in markdown format, otherwise as plain text messages. However, it introduces a security vulnerability where untrusted bot output (e.g., from an LLM or echoed user input) can be used to trigger unintended template messages via prompt injection, which could be exploited for phishing or social engineering. My review also highlights opportunities to improve the robustness and clarity of this new logic, suggesting moving import statements to the top and refactoring the template detection logic for safer handling of exceptions and edge cases.
| try: | ||
| # 尝试解析plain_text为JSON，检查是否包含custom_template_id | ||
| if plain_text: | ||
| template_data = json.loads(plain_text) | ||
| if isinstance(template_data, dict) and "markdown" in template_data and "custom_template_id" in template_data["markdown"]: | ||
| is_template_message = True | ||
| except: | ||
| pass | ||
|
|
||
| if is_template_message and template_data: | ||
| # 使用模板消息模式 | ||
| payload = { | ||
| "markdown": template_data["markdown"], | ||
| "msg_type": 2, | ||
| "msg_id": self.message_obj.message_id, | ||
| } |
There was a problem hiding this comment.
The current implementation attempts to parse the bot's plain text output as JSON to detect markdown templates. This introduces a security vulnerability where untrusted bot output (e.g., from an LLM or echoed user input) can be used to trigger unintended template messages via prompt injection, potentially for phishing or social engineering. Additionally, the template detection logic has robustness and clarity issues:
- The empty
except:block is too broad and can catch unintended exceptions likeSystemExitorKeyboardInterrupt. It should specifically catchjson.JSONDecodeError. - The check
"custom_template_id" in template_data["markdown"]is not safe iftemplate_data["markdown"]is not a dictionary, which could lead to aTypeError. - The logic could be simplified by refactoring the
try-exceptblock and removing theis_template_messageflag.
A more secure approach would be to use a dedicated message component for templates rather than overloading the plain text output.
template_data = None
try:
# 尝试解析plain_text为JSON，检查是否为模板消息
if plain_text:
data = json.loads(plain_text)
if (isinstance(data, dict) and isinstance(data.get("markdown"), dict) and "custom_template_id" in data["markdown"]):
template_data = data
except (json.JSONDecodeError, TypeError):
pass
if template_data:
# 使用模板消息模式
payload = {
"markdown": template_data["markdown"],
"msg_type": 2,
"msg_id": self.message_obj.message_id,
}
else:
# 使用普通文本消息模式
payload = {
"content": plain_text,
"msg_type": 0,
"msg_id": self.message_obj.message_id,
}| "msg_id": self.message_obj.message_id, | ||
| } | ||
| # 检查消息是否包含模板信息 | ||
| import json |
There was a problem hiding this comment.
为了遵循 PEP 8 规范并提高代码的可读性和性能，import json 语句应置于文件顶部，与其他标准库导入一起，而不是在方法内部。在方法中导入模块会在每次调用该方法时都执行导入操作，这可能会带来不必要的开销。请将此行移至文件顶部，并在此处删除。
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - 我发现了 3 个问题，并提供了一些高层面的反馈：
- 在 JSON 解析代码块中使用裸
except捕获所有异常会掩盖真实错误；建议改为捕获json.JSONDecodeError（以及其他必要的特定异常）。 - 现在在
_post_send内部导入json模块，从可读性和效率来说，把这个导入移动到模块级别会更好。 - 在当前逻辑下，非模板的 markdown 消息现在会作为普通
content携带msg_type: 0发送，而不是使用MarkdownPayload和msg_type: 2；请再次确认你是否希望对非模板消息保留 markdown 渲染，并据此调整分支逻辑。
面向 AI Agent 的提示词
Please address the comments from this code review:
## Overall Comments
- Catching all exceptions in the JSON parsing block with a bare `except` can hide real errors; consider catching `json.JSONDecodeError` (and other specific exceptions if needed) instead.
- The `json` module is imported inside `_post_send`; it would be cleaner and more efficient to move this import to the module level.
- With the new logic, non-template markdown messages are now sent as plain `content` with `msg_type: 0` instead of using `MarkdownPayload` and `msg_type: 2`; please double-check whether you want to preserve markdown rendering for non-template messages and adjust the branching accordingly.
## Individual Comments
### Comment 1
<location path="astrbot/core/platform/sources/qqofficial/qqofficial_message_event.py" line_range="150-151" />
<code_context>
+ template_data = json.loads(plain_text)
+ if isinstance(template_data, dict) and "markdown" in template_data and "custom_template_id" in template_data["markdown"]:
+ is_template_message = True
+ except:
+ pass
+
+ if is_template_message and template_data:
</code_context>
<issue_to_address>
**issue (bug_risk):** Avoid bare `except` and silently swallowing all exceptions during JSON parsing.
Swallowing all exceptions here makes real bugs harder to detect. Prefer catching `json.JSONDecodeError` (and possibly `TypeError`) to treat only malformed JSON as “not a template”, and let other exceptions either propagate or be logged so unexpected issues remain visible.
</issue_to_address>
### Comment 2
<location path="astrbot/core/platform/sources/qqofficial/qqofficial_message_event.py" line_range="144-151" />
<code_context>
+ # 尝试解析plain_text为JSON，检查是否包含custom_template_id
+ if plain_text:
+ template_data = json.loads(plain_text)
+ if isinstance(template_data, dict) and "markdown" in template_data and "custom_template_id" in template_data["markdown"]:
+ is_template_message = True
+ except:
+ pass
</code_context>
<issue_to_address>
**suggestion (bug_risk):** Guard against unexpected `markdown` structure when checking for `custom_template_id`.
This condition relies on `template_data["markdown"]` supporting `in` for membership and having the same structure you later send in the payload. To avoid unexpected behavior (e.g., if `markdown` is a list or string) and potential API rejections, ensure `template_data["markdown"]` is a dict before checking for `"custom_template_id"` so the check matches the expected payload shape.
```suggestion
try:
# 尝试解析plain_text为JSON，检查是否包含custom_template_id
if plain_text:
template_data = json.loads(plain_text)
if isinstance(template_data, dict):
markdown = template_data.get("markdown")
# 仅当 markdown 为字典时才检查 custom_template_id，确保结构与后续 payload 一致
if isinstance(markdown, dict) and "custom_template_id" in markdown:
is_template_message = True
except Exception:
# JSON 解析失败或结构异常时，回退为普通文本消息模式
pass
```
</issue_to_address>
### Comment 3
<location path="astrbot/core/platform/sources/qqofficial/qqofficial_message_event.py" line_range="139" />
<code_context>
- "msg_type": 2,
- "msg_id": self.message_obj.message_id,
- }
+ # 检查消息是否包含模板信息
+ import json
+ is_template_message = False
</code_context>
<issue_to_address>
**issue (complexity):** Consider extracting the JSON template parsing into a helper and using it to drive a simpler payload-selection branch in `_post_send`.
You can keep the new behavior but isolate the complexity and make the flow clearer with two small helpers and a narrower exception.
### 1. Extract template parsing into a helper
This keeps `_post_send` from doing ad‑hoc JSON parsing and reduces nesting:
```python
# at module level
import json
def _parse_template_markdown(plain_text: str) -> dict | None:
if not plain_text:
return None
try:
data = json.loads(plain_text)
except json.JSONDecodeError:
return None
if not isinstance(data, dict):
return None
markdown = data.get("markdown")
if isinstance(markdown, dict) and "custom_template_id" in markdown:
return markdown
return None
```
### 2. Simplify payload construction in `_post_send`
Use the helper to drive a shallow, explicit branch instead of inline parsing and `bare except`:
```python
template_markdown = _parse_template_markdown(plain_text)
if template_markdown is not None:
# 模板消息模式
payload = {
"markdown": template_markdown,
"msg_type": 2,
"msg_id": self.message_obj.message_id,
}
else:
# 普通文本消息模式
payload = {
"content": plain_text,
"msg_type": 0,
"msg_id": self.message_obj.message_id,
}
```
This keeps all current behavior (auto-detection via JSON, `msg_type` 2 vs 0, same payload shapes), but:
- Removes the inline `import json`.
- Replaces `except:` with a targeted `json.JSONDecodeError`.
- Keeps `_post_send` focused on “choose payload shape” rather than parsing and validating JSON.
</issue_to_address>帮我变得更有用！请在每条评论上点 👍 或 👎，我会根据这些反馈改进后续的审查建议。
Original comment in English
Hey - I've found 3 issues, and left some high level feedback:
- Catching all exceptions in the JSON parsing block with a bare
exceptcan hide real errors; consider catchingjson.JSONDecodeError(and other specific exceptions if needed) instead. - The
jsonmodule is imported inside_post_send; it would be cleaner and more efficient to move this import to the module level. - With the new logic, non-template markdown messages are now sent as plain
contentwithmsg_type: 0instead of usingMarkdownPayloadandmsg_type: 2; please double-check whether you want to preserve markdown rendering for non-template messages and adjust the branching accordingly.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Catching all exceptions in the JSON parsing block with a bare `except` can hide real errors; consider catching `json.JSONDecodeError` (and other specific exceptions if needed) instead.
- The `json` module is imported inside `_post_send`; it would be cleaner and more efficient to move this import to the module level.
- With the new logic, non-template markdown messages are now sent as plain `content` with `msg_type: 0` instead of using `MarkdownPayload` and `msg_type: 2`; please double-check whether you want to preserve markdown rendering for non-template messages and adjust the branching accordingly.
## Individual Comments
### Comment 1
<location path="astrbot/core/platform/sources/qqofficial/qqofficial_message_event.py" line_range="150-151" />
<code_context>
+ template_data = json.loads(plain_text)
+ if isinstance(template_data, dict) and "markdown" in template_data and "custom_template_id" in template_data["markdown"]:
+ is_template_message = True
+ except:
+ pass
+
+ if is_template_message and template_data:
</code_context>
<issue_to_address>
**issue (bug_risk):** Avoid bare `except` and silently swallowing all exceptions during JSON parsing.
Swallowing all exceptions here makes real bugs harder to detect. Prefer catching `json.JSONDecodeError` (and possibly `TypeError`) to treat only malformed JSON as “not a template”, and let other exceptions either propagate or be logged so unexpected issues remain visible.
</issue_to_address>
### Comment 2
<location path="astrbot/core/platform/sources/qqofficial/qqofficial_message_event.py" line_range="144-151" />
<code_context>
+ # 尝试解析plain_text为JSON，检查是否包含custom_template_id
+ if plain_text:
+ template_data = json.loads(plain_text)
+ if isinstance(template_data, dict) and "markdown" in template_data and "custom_template_id" in template_data["markdown"]:
+ is_template_message = True
+ except:
+ pass
</code_context>
<issue_to_address>
**suggestion (bug_risk):** Guard against unexpected `markdown` structure when checking for `custom_template_id`.
This condition relies on `template_data["markdown"]` supporting `in` for membership and having the same structure you later send in the payload. To avoid unexpected behavior (e.g., if `markdown` is a list or string) and potential API rejections, ensure `template_data["markdown"]` is a dict before checking for `"custom_template_id"` so the check matches the expected payload shape.
```suggestion
try:
# 尝试解析plain_text为JSON，检查是否包含custom_template_id
if plain_text:
template_data = json.loads(plain_text)
if isinstance(template_data, dict):
markdown = template_data.get("markdown")
# 仅当 markdown 为字典时才检查 custom_template_id，确保结构与后续 payload 一致
if isinstance(markdown, dict) and "custom_template_id" in markdown:
is_template_message = True
except Exception:
# JSON 解析失败或结构异常时，回退为普通文本消息模式
pass
```
</issue_to_address>
### Comment 3
<location path="astrbot/core/platform/sources/qqofficial/qqofficial_message_event.py" line_range="139" />
<code_context>
- "msg_type": 2,
- "msg_id": self.message_obj.message_id,
- }
+ # 检查消息是否包含模板信息
+ import json
+ is_template_message = False
</code_context>
<issue_to_address>
**issue (complexity):** Consider extracting the JSON template parsing into a helper and using it to drive a simpler payload-selection branch in `_post_send`.
You can keep the new behavior but isolate the complexity and make the flow clearer with two small helpers and a narrower exception.
### 1. Extract template parsing into a helper
This keeps `_post_send` from doing ad‑hoc JSON parsing and reduces nesting:
```python
# at module level
import json
def _parse_template_markdown(plain_text: str) -> dict | None:
if not plain_text:
return None
try:
data = json.loads(plain_text)
except json.JSONDecodeError:
return None
if not isinstance(data, dict):
return None
markdown = data.get("markdown")
if isinstance(markdown, dict) and "custom_template_id" in markdown:
return markdown
return None
```
### 2. Simplify payload construction in `_post_send`
Use the helper to drive a shallow, explicit branch instead of inline parsing and `bare except`:
```python
template_markdown = _parse_template_markdown(plain_text)
if template_markdown is not None:
# 模板消息模式
payload = {
"markdown": template_markdown,
"msg_type": 2,
"msg_id": self.message_obj.message_id,
}
else:
# 普通文本消息模式
payload = {
"content": plain_text,
"msg_type": 0,
"msg_id": self.message_obj.message_id,
}
```
This keeps all current behavior (auto-detection via JSON, `msg_type` 2 vs 0, same payload shapes), but:
- Removes the inline `import json`.
- Replaces `except:` with a targeted `json.JSONDecodeError`.
- Keeps `_post_send` focused on “choose payload shape” rather than parsing and validating JSON.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| except: | ||
| pass |
There was a problem hiding this comment.
issue (bug_risk): 请避免使用裸 except，并且不要在 JSON 解析时静默吞掉所有异常。
在这里吞掉所有异常会让真实的 bug 更难被发现。建议只捕获 json.JSONDecodeError（以及可能的 TypeError），仅将格式错误的 JSON 视为“不是模板”；对其他异常要么允许其继续向外抛出，要么记录日志，以确保意料之外的问题仍然可见。
Original comment in English
issue (bug_risk): Avoid bare except and silently swallowing all exceptions during JSON parsing.
Swallowing all exceptions here makes real bugs harder to detect. Prefer catching json.JSONDecodeError (and possibly TypeError) to treat only malformed JSON as “not a template”, and let other exceptions either propagate or be logged so unexpected issues remain visible.
| try: | ||
| # 尝试解析plain_text为JSON，检查是否包含custom_template_id | ||
| if plain_text: | ||
| template_data = json.loads(plain_text) | ||
| if isinstance(template_data, dict) and "markdown" in template_data and "custom_template_id" in template_data["markdown"]: | ||
| is_template_message = True | ||
| except: | ||
| pass |
There was a problem hiding this comment.
suggestion (bug_risk): 在检查 custom_template_id 时，应当防范 markdown 结构异常的情况。
当前条件依赖 template_data["markdown"] 支持成员检验操作 in，并假设它与后续发送到 payload 中的结构一致。为避免异常行为（例如 markdown 是列表或字符串）和潜在的 API 拒绝，建议在检查 "custom_template_id" 之前，先确保 template_data["markdown"] 是一个字典，这样检查逻辑才能与期望的 payload 结构保持一致。
| try: | |
| # 尝试解析plain_text为JSON，检查是否包含custom_template_id | |
| if plain_text: | |
| template_data = json.loads(plain_text) | |
| if isinstance(template_data, dict) and "markdown" in template_data and "custom_template_id" in template_data["markdown"]: | |
| is_template_message = True | |
| except: | |
| pass | |
| try: | |
| # 尝试解析plain_text为JSON，检查是否包含custom_template_id | |
| if plain_text: | |
| template_data = json.loads(plain_text) | |
| if isinstance(template_data, dict): | |
| markdown = template_data.get("markdown") | |
| # 仅当 markdown 为字典时才检查 custom_template_id，确保结构与后续 payload 一致 | |
| if isinstance(markdown, dict) and "custom_template_id" in markdown: | |
| is_template_message = True | |
| except Exception: | |
| # JSON 解析失败或结构异常时，回退为普通文本消息模式 | |
| pass |
Original comment in English
suggestion (bug_risk): Guard against unexpected markdown structure when checking for custom_template_id.
This condition relies on template_data["markdown"] supporting in for membership and having the same structure you later send in the payload. To avoid unexpected behavior (e.g., if markdown is a list or string) and potential API rejections, ensure template_data["markdown"] is a dict before checking for "custom_template_id" so the check matches the expected payload shape.
| try: | |
| # 尝试解析plain_text为JSON，检查是否包含custom_template_id | |
| if plain_text: | |
| template_data = json.loads(plain_text) | |
| if isinstance(template_data, dict) and "markdown" in template_data and "custom_template_id" in template_data["markdown"]: | |
| is_template_message = True | |
| except: | |
| pass | |
| try: | |
| # 尝试解析plain_text为JSON，检查是否包含custom_template_id | |
| if plain_text: | |
| template_data = json.loads(plain_text) | |
| if isinstance(template_data, dict): | |
| markdown = template_data.get("markdown") | |
| # 仅当 markdown 为字典时才检查 custom_template_id，确保结构与后续 payload 一致 | |
| if isinstance(markdown, dict) and "custom_template_id" in markdown: | |
| is_template_message = True | |
| except Exception: | |
| # JSON 解析失败或结构异常时，回退为普通文本消息模式 | |
| pass |
| "msg_type": 2, | ||
| "msg_id": self.message_obj.message_id, | ||
| } | ||
| # 检查消息是否包含模板信息 |
There was a problem hiding this comment.
issue (complexity): 建议将 JSON 模板解析逻辑抽取为一个辅助函数，并据此简化 _post_send 中选择 payload 的分支。
你可以保留当前的新行为，但通过两个小的辅助函数和更窄的异常捕获来隔离复杂度，让整体流程更清晰。
1. 将模板解析抽取为辅助函数
这样可以避免 _post_send 中临时进行 JSON 解析，并减少嵌套层级：
# at module level
import json
def _parse_template_markdown(plain_text: str) -> dict | None:
if not plain_text:
return None
try:
data = json.loads(plain_text)
except json.JSONDecodeError:
return None
if not isinstance(data, dict):
return None
markdown = data.get("markdown")
if isinstance(markdown, dict) and "custom_template_id" in markdown:
return markdown
return None2. 简化 _post_send 中的 payload 构造逻辑
通过这个辅助函数驱动一个更浅、更明确的分支，而不是在内联代码中解析 JSON 并使用裸 except：
template_markdown = _parse_template_markdown(plain_text)
if template_markdown is not None:
# 模板消息模式
payload = {
"markdown": template_markdown,
"msg_type": 2,
"msg_id": self.message_obj.message_id,
}
else:
# 普通文本消息模式
payload = {
"content": plain_text,
"msg_type": 0,
"msg_id": self.message_obj.message_id,
}这样既保持了当前的行为（通过 JSON 自动检测、msg_type 2 与 0 的切换、相同的 payload 结构），又能：
- 去掉内联的
import json。 - 将
except:替换为更精确的json.JSONDecodeError。 - 让
_post_send更专注于“选择 payload 结构”，而不是同时处理 JSON 解析和校验。
Original comment in English
issue (complexity): Consider extracting the JSON template parsing into a helper and using it to drive a simpler payload-selection branch in _post_send.
You can keep the new behavior but isolate the complexity and make the flow clearer with two small helpers and a narrower exception.
1. Extract template parsing into a helper
This keeps _post_send from doing ad‑hoc JSON parsing and reduces nesting:
# at module level
import json
def _parse_template_markdown(plain_text: str) -> dict | None:
if not plain_text:
return None
try:
data = json.loads(plain_text)
except json.JSONDecodeError:
return None
if not isinstance(data, dict):
return None
markdown = data.get("markdown")
if isinstance(markdown, dict) and "custom_template_id" in markdown:
return markdown
return None2. Simplify payload construction in _post_send
Use the helper to drive a shallow, explicit branch instead of inline parsing and bare except:
template_markdown = _parse_template_markdown(plain_text)
if template_markdown is not None:
# 模板消息模式
payload = {
"markdown": template_markdown,
"msg_type": 2,
"msg_id": self.message_obj.message_id,
}
else:
# 普通文本消息模式
payload = {
"content": plain_text,
"msg_type": 0,
"msg_id": self.message_obj.message_id,
}This keeps all current behavior (auto-detection via JSON, msg_type 2 vs 0, same payload shapes), but:
- Removes the inline
import json. - Replaces
except:with a targetedjson.JSONDecodeError. - Keeps
_post_sendfocused on “choose payload shape” rather than parsing and validating JSON.
1 participant
修复了QQ官方平台自定义模板的发送被拒绝或者没传参的问题
修改了 astrbot/core/platform/sources/qqofficial/qqofficial_message_event.py 文件
修复了QQ官方平台自定义模板的发送被拒绝或者没传参的问题，并且普通文本消息直接使用 content 模式
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
根据 QQ 公众号平台发送消息的类型（自定义 Markdown 模板或纯文本），采用不同的处理方式。
Bug Fixes:
Enhancements:
Original summary in English
Summary by Sourcery
Handle QQ Official platform outgoing messages differently based on whether they are custom markdown templates or plain text.
Bug Fixes:
Enhancements: