-
-
Notifications
You must be signed in to change notification settings - Fork 26.9k
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
Fix parsing HTML/JSX tags to real elements #2796
Conversation
Thank you for your pull request and welcome to our community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file. In order for us to review and merge your code, please sign up at https://code.facebook.com/cla. If you are contributing on behalf of someone else (eg your employer), the individual CLA may not be sufficient and your employer may need the corporate CLA signed. If you have received this in error or have any questions, please contact us at [email protected]. Thanks! |
Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Facebook open source project. Thanks! |
Thanks for the PR! Is this enough, or are there more things that need escaping? I'm thinking HTML entities or something like this. Is there a popular package we can use that does the escaping instead of hand rolling it? |
I think it can fixed most cases, but maybe not enough. I searched an article titled Yahoo created a project named xss-filters, which has so many cases for protecting from XSS, so I think it also can be used here. Have a look? |
Hi, I notice that // ......
var Entities = require('html-entities').AllHtmlEntities;
var ansiHTML = require('./ansiHTML');
var entities = new Entities();
// ......
function showErrorOverlay(message) {
ensureOverlayDivExists(function onOverlayDivReady(overlayDiv) {
// TODO: unify this with our runtime overlay
overlayDiv.innerHTML =
'<div style="' +
overlayHeaderStyle() +
'">Failed to compile</div>' +
'<pre style="' +
'display: block; padding: 0.5em; margin-top: 0; ' +
'margin-bottom: 0.5em; overflow-x: auto; white-space: pre-wrap; ' +
'border-radius: 0.25rem; background-color: rgba(206, 17, 38, 0.05)">' +
'<code style="font-family: Consolas, Menlo, monospace;">' +
ansiHTML(entities.encode(message)) + // <- uses `entities.encode()` to escape HTML entities
'</code></pre>' +
'<div style="' +
'font-family: sans-serif; color: rgb(135, 142, 145); margin-top: 0.5rem; ' +
'flex: 0 0 auto">' +
'This error occurred during the build time and cannot be dismissed.</div>';
});
} However, in // ......
import generateAnsiHtml from 'react-dev-utils/ansiHTML';
// ......
const ansiHighlight = codeFrame(
sourceCode.join('\n'),
lineNum,
columnNum == null ? 0 : columnNum - (isFinite(whiteSpace) ? whiteSpace : 0),
{
forceColor: true,
linesAbove: contextSize,
linesBelow: contextSize,
}
); // <- maybe not parsed correctly, like treating source code as a string, see #2789
const htmlHighlight = generateAnsiHtml(ansiHighlight); // <- ansiHighlight is not escaped I'll try replacing the modification in previous commit with |
Nice catch on the html element escaping! |
LGTM. I trust you that this works. Thank you for digging into this. |
Hi there! This change is out in |
* Fix parsing HTML/JSX tags to real elements * Use `html-entities` to escape instead of pure `replace()` * Remove unnecessary HTML entity replacing
* commit 'bfaee410c502a95076a6bd89721c76ca08e15f7b': (39 commits) Publish Prepare for 1.0.11 release (facebook#2924) Update dev deps (facebook#2923) Update README.md Use env variable to disable source maps (facebook#2818) Make formatWebpackMessages return all messages (facebook#2834) Adjust the `checkIfOnline` check if in a corporate proxy environment (facebook#2884) Fix the order of arguments in spawned child proc (facebook#2913) Feature/webpack 3 4 (facebook#2875) Allow importing package.json (facebook#2468) Re-enable flowtype warning (facebook#2718) Format UglifyJs error (facebook#2650) Unstage yarn.lock pre-commit (facebook#2700) Update README.md Update README.md Add Electrode to alternatives (facebook#2728) Fix parsing HTML/JSX tags to real elements (facebook#2796) Update webpack version note (facebook#2798) Use modern syntax feature (facebook#2873) Allow use of scoped packages with a pinned version (facebook#2853) ... # Conflicts: # packages/react-scripts/config/webpack.config.dev.js # packages/react-scripts/config/webpack.config.prod.js # packages/react-scripts/package.json
* Fix parsing HTML/JSX tags to real elements * Use `html-entities` to escape instead of pure `replace()` * Remove unnecessary HTML entity replacing
* Fix parsing HTML/JSX tags to real elements * Use `html-entities` to escape instead of pure `replace()` * Remove unnecessary HTML entity replacing
* Fix parsing HTML/JSX tags to real elements * Use `html-entities` to escape instead of pure `replace()` * Remove unnecessary HTML entity replacing
This should fixed #2789.
Just replaces all the angle brackets to
<
and>
to avoid parsing them as real HTML elements.Here is the screenshot of the example that mentioned in origin issue, the checkbox is not showing and it displays its source code.