-
-
Notifications
You must be signed in to change notification settings - Fork 288
支持自定义html的插入,并且修改bug,实现更为安全,同时当存在多个顶层元素时也可以添加 #760
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: main
Are you sure you want to change the base?
Conversation
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
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.
Summary of Changes
Hello @ganyudedog, 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!
This pull request introduces a significant new feature allowing the dynamic insertion of custom HTML content within the visual novel engine. This enhancement provides greater flexibility for developers to display rich, interactive elements directly within game scenes. It also includes important security measures to prevent XSS vulnerabilities and improves the handling of multiple top-level HTML elements.
Highlights
- Custom HTML Command (setCustomHtml): A new command is introduced, enabling game scripts to inject arbitrary HTML content into the scene.
- Secure HTML Parsing and Rendering: A dedicated module (setCustomHtml.ts) is added to safely parse incoming HTML, apply styles, and actively filter out potentially malicious on* event handlers to mitigate Cross-Site Scripting (XSS) risks.
- Dynamic Positioning and React Integration: A new React component (CustomHtml.tsx) leverages React Portals to render the custom HTML, supporting dynamic positioning relative to existing game figures (left, center, right) and falling back to the document body if needed.
- State Management for Custom Content: The application's state management (stageReducer) is extended to track and control the lifecycle of inserted HTML elements, allowing for their addition, removal, and clearing.
- Demo Scene Update: The demo_zh_cn.txt scene is updated to showcase the practical usage of the new setCustomHtml command, demonstrating both content insertion and removal.
Using Gemini Code Assist
The 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 in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command>
or @gemini-code-assist <command>
. Below is a summary of the supported commands.
Feature | Command | Description |
---|---|---|
Code Review | /gemini review |
Performs a code review for the current pull request in its current state. |
Pull Request Summary | /gemini summary |
Provides a summary of the current pull request in its current state. |
Comment | @gemini-code-assist | Responds in comments when explicitly tagged, both in issue comments and review comments. |
Help | /gemini help |
Displays a list of available commands. |
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 .gemini/
folder in the base of the repository. Detailed instructions can be found here.
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
-
Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩
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.
Code Review
本次 PR 新增了强大的自定义 HTML 插入功能,整体实现思路清晰,使用了 DOMParser 和 Redux 进行处理,值得肯定。然而,目前的实现中存在严重的安全漏洞,可能导致 XSS 攻击,需要立即修复。此外,在 CSS 解析和代码效率方面也存在一些可以改进的地方。请关注以下几点具体的反馈。
|
||
// 遍历所有子节点 | ||
node.childNodes.forEach((child) => { | ||
if (child.nodeType === Node.ELEMENT_NODE) { |
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.
存在严重的安全漏洞。当前实现没有对 HTML 进行充分的净化,允许执行任意 JavaScript 代码(XSS)。由于最终内容会通过 dangerouslySetInnerHTML
渲染,这会带来严重的安全风险。
具体来说:
- 危险标签未被过滤:像
<script>
,<iframe>
,<object>
,<embed>
等标签可以被直接注入并执行。 - 危险属性值未被过滤:例如,
<a>
标签的href
属性可以设置为javascript:alert('XSS')
。
强烈建议使用成熟的 HTML 净化库(如 DOMPurify
)来处理用户输入的 HTML,以确保安全。如果不想引入新库,则必须实现一个严格的白名单机制,只允许安全的标签和属性,并对属性值进行校验。
例如,在 parseNode
中可以增加一个标签白名单:
const ALLOWED_TAGS = ['div', 'span', 'p', 'b', 'i', 'u', 'strong', 'em'];
// ...
if (child.nodeType === Node.ELEMENT_NODE) {
const element = child as HTMLElement;
const tagName = element.tagName.toLowerCase();
if (!ALLOWED_TAGS.includes(tagName)) {
// 跳过或移除不允许的标签
return;
}
// ...
}
function parseCssString(css: string): { styleObj: Record<string, string>; feature?: string } { | ||
const styleObj: Record<string, string> = {}; | ||
let feature: string | undefined; | ||
css | ||
.replace(/[{}]/g, '') | ||
.split(',') | ||
.map((s) => s.trim()) | ||
.filter(Boolean) | ||
.forEach((item) => { | ||
const [key, value] = item.split(':').map((s) => s.trim()); | ||
if (key && value) { | ||
if (key === 'feature') { | ||
feature = value; | ||
} else { | ||
// 驼峰化 | ||
const camelKey = key.replace(/-([a-z])/g, (_, c) => c.toUpperCase()); | ||
styleObj[camelKey] = value; | ||
} | ||
} | ||
}); | ||
return { styleObj, feature }; | ||
} |
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.
在 parseCssString
函数中,您使用逗号 ,
作为 CSS 属性的分隔符。这不是标准的 CSS 语法,会导致在处理包含逗号的合法 CSS 值(例如 font-family: "Times New Roman", serif
或 box-shadow
的多重阴影)时解析错误。这使得该功能不够健壮,并限制了其可用性。
建议遵循标准的 CSS 语法,使用分号 ;
作为属性分隔符。这样开发者可以直接使用标准的 CSS,也使功能更加可靠。对应的, demo_zh_cn.txt
中的 setCustomHtml
命令也需要更新。
function parseCssString(css: string): { styleObj: Record<string, string>; feature?: string } {
const styleObj: Record<string, string> = {};
let feature: string | undefined;
css
.replace(/[{}]/g, '')
.split(';') // 使用分号分割,更符合CSS标准
.map((s) => s.trim())
.filter(Boolean)
.forEach((item) => {
const [key, value] = item.split(':').map((s) => s.trim());
if (key && value) {
if (key === 'feature') {
feature = value;
} else {
// 驼峰化
const camelKey = key.replace(/-([a-z])/g, (_, c) => c.toUpperCase());
styleObj[camelKey] = value;
}
}
});
return { styleObj, feature };
}
elements.forEach((element) => { | ||
const { html: styledHtml, feature } = convertElementToStyledHtml(element); | ||
|
||
// 直接从元素的attributes中提取样式,避免再次解析HTML字符串 | ||
let style: React.CSSProperties = { position: 'absolute' }; | ||
if (element.attributes.style) { | ||
const processedStyle = autoAddPositionAbsolute(element.attributes.style); | ||
const { styleObj } = parseCssString(processedStyle); | ||
// 将styleObj转换为React.CSSProperties | ||
Object.keys(styleObj).forEach((key) => { | ||
// 使用类型断言来避免TypeScript错误 | ||
(style as any)[key] = styleObj[key]; | ||
}); | ||
} | ||
|
||
// 添加到状态管理,带 feature 字段和style对象 | ||
webgalStore.dispatch(stageActions.addCustomHtml({ html: styledHtml, _feature: feature, style })); | ||
}); |
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.
: undefined; | ||
if (!key) return undefined; | ||
// 通过 window.PIXIapp 获取 PixiStage 实例 | ||
const pixiApp: any = (window as any).PIXIapp; |
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.
在代码中使用了 any
类型来访问 window.PIXIapp
,这会绕过 TypeScript 的类型检查,降低代码的可维护性,并且在 PIXIapp
对象结构变更时容易引发运行时错误。
建议为 PIXIapp
中用到的部分定义一个最小化的接口,以增强类型安全和代码可读性。例如:
interface PixiFigure {
key: string;
pixiContainer: {
toGlobal: (point: { x: number; y: number }) => { x: number; y: number };
children?: [{ height?: number }];
};
}
interface PixiApp {
figureObjects: PixiFigure[];
}
const pixiApp: PixiApp | undefined = (window as any).PIXIapp;
这个可以支持表格嘛 |
可以的,不过我觉得其实主要是拿来绑定立绘,通过style的feature(自定义增加的)触发,而不是单独显示一个UI吧 |
No description provided.