-
-
Notifications
You must be signed in to change notification settings - Fork 2.5k
fix: accept UTF-8 BOM in plugin schemas #9223
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -20,6 +20,32 @@ | |||||||||||||||||||||||||||||||||||||||||
| 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" | ||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+23
to
+28
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. suggestion (testing): Add a test for non-UTF-8 schemas to cover the UnicodeDecodeError branch in 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 |
||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||
| def test_load_plugin_config_schema_accepts_utf8_without_bom(tmp_path: Path): | ||||||||||||||||||||||||||||||||||||||||||
| schema_path = tmp_path / "_conf_schema.json" | ||||||||||||||||||||||||||||||||||||||||||
| schema_path.write_text('{"type": "object"}', encoding="utf-8") | ||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||
| assert PluginManager._load_plugin_config_schema(str(schema_path)) == { | ||||||||||||||||||||||||||||||||||||||||||
| "type": "object" | ||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||
| 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"): | ||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+41
to
+45
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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 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. |
||||||||||||||||||||||||||||||||||||||||||
| PluginManager._load_plugin_config_schema(str(schema_path)) | ||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+41
to
+47
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 配合
Suggested change
|
||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||
| class MockStar: | ||||||||||||||||||||||||||||||||||||||||||
| def __init__(self): | ||||||||||||||||||||||||||||||||||||||||||
| self.root_dir_name = TEST_PLUGIN_DIR | ||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -59,9 +85,11 @@ def test_load_plugin_i18n_reads_locale_files(tmp_path: Path): | |||||||||||||||||||||||||||||||||||||||||
| plugin_path = tmp_path / "plugin" | ||||||||||||||||||||||||||||||||||||||||||
| i18n_path = plugin_path / ".astrbot-plugin" / "i18n" | ||||||||||||||||||||||||||||||||||||||||||
| i18n_path.mkdir(parents=True) | ||||||||||||||||||||||||||||||||||||||||||
| (i18n_path / "zh-CN.json").write_text( | ||||||||||||||||||||||||||||||||||||||||||
| json.dumps({"metadata": {"desc": "中文描述"}}, ensure_ascii=False), | ||||||||||||||||||||||||||||||||||||||||||
| encoding="utf-8", | ||||||||||||||||||||||||||||||||||||||||||
| (i18n_path / "zh-CN.json").write_bytes( | ||||||||||||||||||||||||||||||||||||||||||
| b"\xef\xbb\xbf" | ||||||||||||||||||||||||||||||||||||||||||
| + json.dumps({"metadata": {"desc": "中文描述"}}, ensure_ascii=False).encode( | ||||||||||||||||||||||||||||||||||||||||||
| "utf-8" | ||||||||||||||||||||||||||||||||||||||||||
| ), | ||||||||||||||||||||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||||||||||||||||||||
| (i18n_path / "en-US.json").write_text( | ||||||||||||||||||||||||||||||||||||||||||
| json.dumps({"metadata": {"desc": "English description"}}), | ||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
为了提高代码的健壮性并提供更清晰的错误信息,建议在
_load_plugin_config_schema中验证解析后的 JSON 根节点是否为字典(JSON 对象)。如果插件开发者提供了非字典格式的 schema(例如 JSON 数组或字符串),在后续解析时会抛出不直观的AttributeError。提前进行类型检查并抛出明确的ValueError可以显著提升开发者的调试体验。