-
Notifications
You must be signed in to change notification settings - Fork 35
修复已知问题 #659
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
修复已知问题 #659
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 |
|---|---|---|
|
|
@@ -73,7 +73,7 @@ def test_default_pywebio_assets_are_self_hosted(self): | |
| self.assertIn('href="pywebio_static/css/app.css?v=', response.text) | ||
| self.assertNotIn("cdn.jsdelivr.net", response.text) | ||
|
|
||
| def test_theme_assets_do_not_depend_on_external_font_services(self): | ||
| def test_theme_keeps_random_background_without_external_font_services(self): | ||
| theme_css = (PROJECT_ROOT / "assets/gui/css/advanced-material-alas.css").read_text( | ||
| encoding="utf-8" | ||
| ) | ||
|
|
@@ -83,13 +83,6 @@ def test_theme_assets_do_not_depend_on_external_font_services(self): | |
|
|
||
| self.assertNotIn("fonts.googleapis.com", theme_css) | ||
| self.assertNotIn("fonts.gstatic.com", theme_css) | ||
| self.assertNotIn("api.yppp.net", theme_css) | ||
| self.assertIn('url("https://api.yppp.net/api.php")', theme_css) | ||
|
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): 对背景 API URL 的断言过于严格;可以考虑稍微放宽一些,同时仍然保证预期行为。 当前断言对 CSS 的具体序列化和路径进行了硬编码,因此小幅改动(例如增加查询参数、改变引号/空白)即使不影响行为,也会导致测试失败。可以考虑使用更灵活的匹配方式(例如检查 建议实现: self.assertNotIn("fonts.googleapis.com", theme_css)
self.assertNotIn("fonts.gstatic.com", theme_css)
# 确保随机背景使用指向 api.yppp.net 的 HTTPS URL,而不依赖精确的 CSS 序列化格式
self.assertRegex(
theme_css,
r'url\(\s*[\'"]https://api\.yppp\.net/[^\'"]*[\'"]\s*\)'
)
self.assertNotIn("fonts.googleapis.com", obs_overlay)如果这个测试类目前没有继承自提供 Original comment in Englishsuggestion (testing): The assertion on the exact background API URL may be brittle; consider loosening it slightly while still enforcing the intended behavior. This assertion hard-codes the exact CSS serialization and path, so minor changes (e.g. added query params, different quoting/whitespace) will break the test without changing behavior. Consider asserting on a more flexible pattern (e.g. checking that Suggested implementation: self.assertNotIn("fonts.googleapis.com", theme_css)
self.assertNotIn("fonts.gstatic.com", theme_css)
# Ensure the random background uses an HTTPS URL to api.yppp.net without depending on exact CSS serialization
self.assertRegex(
theme_css,
r'url\(\s*[\'"]https://api\.yppp\.net/[^\'"]*[\'"]\s*\)'
)
self.assertNotIn("fonts.googleapis.com", obs_overlay)If this test class does not already inherit from |
||
| self.assertNotIn("fonts.googleapis.com", obs_overlay) | ||
| self.assertNotIn("fonts.gstatic.com", obs_overlay) | ||
| self.assertIn('url("static/assets/gui/css/bj.jpg")', theme_css) | ||
| self.assertEqual( | ||
| urljoin( | ||
| "https://example.test/azur/", "static/assets/gui/css/bj.jpg" | ||
| ), | ||
| "https://example.test/azur/static/assets/gui/css/bj.jpg", | ||
| ) | ||
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.
suggestion (testing): 建议增加一个检查,确保除了允许的随机背景 API 外,没有引入其他外部资源主机。
目前,这个测试验证了随机背景使用
https://api.yppp.net/api.php,并且避免了外部字体服务。为了更好地落实「自托管资源 + 单一白名单 API」的策略,可以考虑增加一个断言,检查theme_css/obs_overlay中除了api.yppp.net之外不包含其他http/httpsURL(例如,通过提取url(...)的值,或扫描http字样,并只允许白名单主机或相对 URL)。建议实现:
为了让这个测试生效,请确保在
tests/test_webui_static_assets.py顶部已导入re:import pathlib、import os等)附近添加import re。obs_overlay已在测试模块中提前定义(看起来已经在使用;如果它已经被设置为 overlay 的 CSS 内容,则无需更改)。Original comment in English
suggestion (testing): Consider adding a check that no other external asset hosts are introduced besides the allowed random background API.
Currently, the test confirms the random background uses
https://api.yppp.net/api.phpand that external font services are avoided. To better enforce the “self-hosted assets + one whitelisted API” policy, consider adding an assertion thattheme_css/obs_overlaycontain no otherhttp/httpsURLs besidesapi.yppp.net(e.g., by extractingurl(...)values or scanning forhttpand allowing only the whitelisted host or relative URLs).Suggested implementation:
To make this test work, ensure
reis imported at the top oftests/test_webui_static_assets.py:import realongside the other standard-library imports (e.g., near existingimport pathlib,import os, etc.).obs_overlayis defined earlier in the test module (it appears to be used already; no changes needed if it's already set to the overlay CSS text).