fix: accept UTF-8 BOM in plugin schemas#9223
Conversation
There was a problem hiding this comment.
Code Review
This pull request adds support for UTF-8 BOM when loading plugin internationalization (i18n) files and configuration schemas. It introduces a helper method _load_plugin_config_schema with robust error handling for encoding and JSON decoding issues, accompanied by new unit tests. The feedback suggests enhancing the schema loader to explicitly validate that the parsed JSON root is a dictionary (JSON object) and adding a corresponding test case to ensure robust error reporting.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| @staticmethod | ||
| def _load_plugin_config_schema(schema_path: str) -> dict: | ||
| """Load a plugin config schema, accepting an optional UTF-8 BOM.""" | ||
| try: | ||
| with open(schema_path, encoding="utf-8-sig") as f: | ||
| return json.load(f) | ||
| except UnicodeDecodeError as exc: | ||
| raise ValueError( | ||
| f"插件配置 schema 必须使用 UTF-8 编码: {schema_path}" | ||
| ) from exc | ||
| except json.JSONDecodeError as exc: | ||
| raise ValueError( | ||
| f"插件配置 schema 不是有效的 JSON: {schema_path} " | ||
| f"(line {exc.lineno}, column {exc.colno})" | ||
| ) from exc |
There was a problem hiding this comment.
为了提高代码的健壮性并提供更清晰的错误信息,建议在 _load_plugin_config_schema 中验证解析后的 JSON 根节点是否为字典(JSON 对象)。如果插件开发者提供了非字典格式的 schema(例如 JSON 数组或字符串),在后续解析时会抛出不直观的 AttributeError。提前进行类型检查并抛出明确的 ValueError 可以显著提升开发者的调试体验。
| @staticmethod | |
| def _load_plugin_config_schema(schema_path: str) -> dict: | |
| """Load a plugin config schema, accepting an optional UTF-8 BOM.""" | |
| try: | |
| with open(schema_path, encoding="utf-8-sig") as f: | |
| return json.load(f) | |
| except UnicodeDecodeError as exc: | |
| raise ValueError( | |
| f"插件配置 schema 必须使用 UTF-8 编码: {schema_path}" | |
| ) from exc | |
| except json.JSONDecodeError as exc: | |
| raise ValueError( | |
| f"插件配置 schema 不是有效的 JSON: {schema_path} " | |
| f"(line {exc.lineno}, column {exc.colno})" | |
| ) from exc | |
| @staticmethod | |
| def _load_plugin_config_schema(schema_path: str) -> dict: | |
| """Load a plugin config schema, accepting an optional UTF-8 BOM.""" | |
| try: | |
| with open(schema_path, encoding="utf-8-sig") as f: | |
| schema = json.load(f) | |
| if not isinstance(schema, dict): | |
| raise ValueError( | |
| f"插件配置 schema 根节点必须是一个 JSON 对象: {schema_path}" | |
| ) | |
| return schema | |
| except UnicodeDecodeError as exc: | |
| raise ValueError( | |
| f"插件配置 schema 必须使用 UTF-8 编码: {schema_path}" | |
| ) from exc | |
| except json.JSONDecodeError as exc: | |
| raise ValueError( | |
| f"插件配置 schema 不是有效的 JSON: {schema_path} " | |
| f"(line {exc.lineno}, column {exc.colno})" | |
| ) from exc |
| def test_load_plugin_config_schema_reports_invalid_json(tmp_path: Path): | ||
| schema_path = tmp_path / "_conf_schema.json" | ||
| schema_path.write_text("{invalid", encoding="utf-8") | ||
|
|
||
| with pytest.raises(ValueError, match="不是有效的 JSON"): | ||
| PluginManager._load_plugin_config_schema(str(schema_path)) | ||
|
|
There was a problem hiding this comment.
配合 _load_plugin_config_schema 中新增的 JSON 根节点类型校验,建议在此处添加对应的单元测试,以确保当 schema 不是有效的 JSON 对象(例如是一个 JSON 数组)时,能够正确抛出预期的 ValueError 并包含清晰的错误提示。
| def test_load_plugin_config_schema_reports_invalid_json(tmp_path: Path): | |
| schema_path = tmp_path / "_conf_schema.json" | |
| schema_path.write_text("{invalid", encoding="utf-8") | |
| with pytest.raises(ValueError, match="不是有效的 JSON"): | |
| PluginManager._load_plugin_config_schema(str(schema_path)) | |
| def test_load_plugin_config_schema_reports_invalid_json(tmp_path: Path): | |
| schema_path = tmp_path / "_conf_schema.json" | |
| schema_path.write_text("{invalid", encoding="utf-8") | |
| with pytest.raises(ValueError, match="不是有效的 JSON"): | |
| PluginManager._load_plugin_config_schema(str(schema_path)) | |
| def test_load_plugin_config_schema_reports_non_dict(tmp_path: Path): | |
| schema_path = tmp_path / "_conf_schema.json" | |
| schema_path.write_text('["not", "a", "dict"]', encoding="utf-8") | |
| with pytest.raises(ValueError, match="根节点必须是一个 JSON 对象"): | |
| PluginManager._load_plugin_config_schema(str(schema_path)) |
There was a problem hiding this comment.
Hey - I've found 2 issues, and left some high level feedback:
- In
_load_plugin_config_schema, consider acceptingos.PathLike/Pathobjects instead of onlystr(and usingPath(schema_path).open(...)) to align with the rest of the codebase’s path handling and make the helper more flexible. - When wrapping
json.JSONDecodeErrorin_load_plugin_config_schema, you might includeexc.msgin the error text so callers get the original parsing reason in addition to the line/column information.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In `_load_plugin_config_schema`, consider accepting `os.PathLike`/`Path` objects instead of only `str` (and using `Path(schema_path).open(...)`) to align with the rest of the codebase’s path handling and make the helper more flexible.
- When wrapping `json.JSONDecodeError` in `_load_plugin_config_schema`, you might include `exc.msg` in the error text so callers get the original parsing reason in addition to the line/column information.
## Individual Comments
### Comment 1
<location path="tests/test_plugin_manager.py" line_range="41-45" />
<code_context>
+ }
+
+
+def test_load_plugin_config_schema_reports_invalid_json(tmp_path: Path):
+ schema_path = tmp_path / "_conf_schema.json"
+ schema_path.write_text("{invalid", encoding="utf-8")
+
+ with pytest.raises(ValueError, match="不是有效的 JSON"):
+ PluginManager._load_plugin_config_schema(str(schema_path))
+
</code_context>
<issue_to_address>
**suggestion (testing):** Extend the invalid JSON schema test to assert that line and column information are included in the error message.
To make this behavior part of the contract, consider capturing the `ValueError` and asserting the presence of line/column info in the message, e.g.:
```python
with pytest.raises(ValueError) as excinfo:
PluginManager._load_plugin_config_schema(str(schema_path))
msg = str(excinfo.value)
assert "不是有效的 JSON" in msg
assert "line " in msg
assert "column " in msg
```
This will help prevent future refactors from dropping the improved diagnostics.
</issue_to_address>
### Comment 2
<location path="tests/test_plugin_manager.py" line_range="23-28" />
<code_context>
TEST_PLUGIN_DIR = "helloworld"
+def test_load_plugin_config_schema_accepts_utf8_bom(tmp_path: Path):
+ schema_path = tmp_path / "_conf_schema.json"
+ schema_path.write_bytes(b'\xef\xbb\xbf{"type": "object"}')
+
+ assert PluginManager._load_plugin_config_schema(str(schema_path)) == {
+ "type": "object"
+ }
+
</code_context>
<issue_to_address>
**suggestion (testing):** Add a test for non-UTF-8 schemas to cover the UnicodeDecodeError branch in `_load_plugin_config_schema`.
Current tests cover UTF-8 (with/without BOM) and invalid JSON, but not files encoded with a different charset (e.g. GBK/ISO-8859-1). Since `_load_plugin_config_schema` has a specific `UnicodeDecodeError` branch that raises a `ValueError` about UTF-8, please add a test that writes the schema with a non-UTF-8 encoding and asserts that this `ValueError` (with the expected message) is raised, so that branch is covered.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| def test_load_plugin_config_schema_reports_invalid_json(tmp_path: Path): | ||
| schema_path = tmp_path / "_conf_schema.json" | ||
| schema_path.write_text("{invalid", encoding="utf-8") | ||
|
|
||
| with pytest.raises(ValueError, match="不是有效的 JSON"): |
There was a problem hiding this comment.
suggestion (testing): Extend the invalid JSON schema test to assert that line and column information are included in the error message.
To make this behavior part of the contract, consider capturing the ValueError and asserting the presence of line/column info in the message, e.g.:
with pytest.raises(ValueError) as excinfo:
PluginManager._load_plugin_config_schema(str(schema_path))
msg = str(excinfo.value)
assert "不是有效的 JSON" in msg
assert "line " in msg
assert "column " in msgThis will help prevent future refactors from dropping the improved diagnostics.
| def test_load_plugin_config_schema_accepts_utf8_bom(tmp_path: Path): | ||
| schema_path = tmp_path / "_conf_schema.json" | ||
| schema_path.write_bytes(b'\xef\xbb\xbf{"type": "object"}') | ||
|
|
||
| assert PluginManager._load_plugin_config_schema(str(schema_path)) == { | ||
| "type": "object" |
There was a problem hiding this comment.
suggestion (testing): Add a test for non-UTF-8 schemas to cover the UnicodeDecodeError branch in _load_plugin_config_schema.
Current tests cover UTF-8 (with/without BOM) and invalid JSON, but not files encoded with a different charset (e.g. GBK/ISO-8859-1). Since _load_plugin_config_schema has a specific UnicodeDecodeError branch that raises a ValueError about UTF-8, please add a test that writes the schema with a non-UTF-8 encoding and asserts that this ValueError (with the expected message) is raised, so that branch is covered.
fix #9222
Modifications / 改动点
修改
astrbot/core/star/star_manager.py:_load_plugin_config_schema(),统一处理插件_conf_schema.json的读取与解析。utf-8-sig读取插件配置 schema,同时兼容 UTF-8 无 BOM 和 UTF-8 BOM。utf-8-sig读取.astrbot-plugin/i18n/*.json,避免带 BOM 的插件翻译文件加载失败。修改
tests/test_plugin_manager.py:Screenshots or Test Results / 运行截图或测试结果
Checklist / 检查清单
😊 If there are new features added in the PR, I have discussed it with the authors through issues/emails, etc.
/ 如果 PR 中有新加入的功能,已经通过 Issue / 邮件等方式和作者讨论过。
👀 My changes have been well-tested, and "Verification Steps" and "Screenshots" have been provided above.
/ 我的更改经过了良好的测试,并已在上方提供了“验证步骤”和“运行截图”。
🤓 I have ensured that no new dependencies are introduced, OR if new dependencies are introduced, they have been added to the appropriate locations in
requirements.txtandpyproject.toml./ 我确保没有引入新依赖库,或者引入了新依赖库的同时将其添加到
requirements.txt和pyproject.toml文件相应位置。😮 My changes do not introduce malicious code.
/ 我的更改没有引入恶意代码。
Summary by Sourcery
Handle plugin config schemas and i18n locale files encoded with UTF-8 BOM while improving error reporting.
Bug Fixes:
Enhancements: