Skip to content
Open
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -136,12 +136,34 @@ async def _post_send(self, stream: dict | None = None):
):
return None

payload: dict = {
# "content": plain_text,
"markdown": MarkdownPayload(content=plain_text) if plain_text else None,
"msg_type": 2,
"msg_id": self.message_obj.message_id,
}
# 检查消息是否包含模板信息
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 None

2. 简化 _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 None

2. 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 targeted json.JSONDecodeError.
  • Keeps _post_send focused on “choose payload shape” rather than parsing and validating JSON.

import json
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

为了遵循 PEP 8 规范并提高代码的可读性和性能,import json 语句应置于文件顶部,与其他标准库导入一起,而不是在方法内部。在方法中导入模块会在每次调用该方法时都执行导入操作,这可能会带来不必要的开销。请将此行移至文件顶部,并在此处删除。

is_template_message = False
template_data = None

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
Comment on lines +150 to +151
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Comment on lines +144 to +151
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

suggestion (bug_risk): 在检查 custom_template_id 时,应当防范 markdown 结构异常的情况。

当前条件依赖 template_data["markdown"] 支持成员检验操作 in,并假设它与后续发送到 payload 中的结构一致。为避免异常行为(例如 markdown 是列表或字符串)和潜在的 API 拒绝,建议在检查 "custom_template_id" 之前,先确保 template_data["markdown"] 是一个字典,这样检查逻辑才能与期望的 payload 结构保持一致。

Suggested change
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.

Suggested change
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


if is_template_message and template_data:
# 使用模板消息模式
payload = {
"markdown": template_data["markdown"],
"msg_type": 2,
"msg_id": self.message_obj.message_id,
}
Comment on lines +144 to +159
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

security-medium medium

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:

  1. The empty except: block is too broad and can catch unintended exceptions like SystemExit or KeyboardInterrupt. It should specifically catch json.JSONDecodeError.
  2. The check "custom_template_id" in template_data["markdown"] is not safe if template_data["markdown"] is not a dictionary, which could lead to a TypeError.
  3. The logic could be simplified by refactoring the try-except block and removing the is_template_message flag.
    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,
            }

else:
# 使用普通文本消息模式
payload = {
"content": plain_text,
"msg_type": 0,
"msg_id": self.message_obj.message_id,
}

if not isinstance(source, botpy.message.Message | botpy.message.DirectMessage):
payload["msg_seq"] = random.randint(1, 10000)
Expand Down