Skip to content
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

Update create-html.ts DOM text reinterpreted as HTML #4757

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

Shivam7-1
Copy link
Contributor

By using innerText, it will avoid the risk of HTML injection, as these properties automatically escape any HTML special characters in the provided text. This helps prevent cross-site scripting (XSS) vulnerabilities by treating the input as plain text rather than interpreted HTML. Always be cautious when dealing with user input or dynamic content to prevent security risks.

@Shivam7-1
Copy link
Contributor Author

Hi @elalish Could You Please Review This PR
Regards

@elalish
Copy link
Collaborator

elalish commented Apr 18, 2024

Have you tested the docs pages with this change to verify nothing has regressed?

@Shivam7-1
Copy link
Contributor Author

Hi @elalish Thanks For Replying
I think it shouldn't cause any issue
Regards

@elalish
Copy link
Collaborator

elalish commented Apr 18, 2024

I think it shouldn't cause any issue

This kind of sentiment makes me very nervous. Please provide screenshots.

@Shivam7-1
Copy link
Contributor Author

Hi @elalish Thanks For Reviewing
It Passes All Test As i check Here Unit test and fidelity test also
Regards
image
image

@elalish
Copy link
Collaborator

elalish commented Apr 19, 2024

Sorry, I may have been imprecise - the docs pages don't have automated tests - you have to manually look at them. What I mean is npm run serve and look at them to ensure they aren't broken. e.g.
image

@NeilFraser
Copy link
Member

This PR would break this product since raw HTML would be printed on the user's screen.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants