Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
tests & tests & tests
  • Loading branch information
jennspencer committed May 10, 2024
commit 175589e827f6fd8ea34da75a131130085df0d8c5
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
2 changes: 1 addition & 1 deletion __tests__/browser/markdown.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ describe('visual regression tests', () => {
//'features',
// 'headings',
'images',
// 'lists',
'htmlTests',
// 'tables',
// 'tablesTests',
'codeBlockTests',
Expand Down
Original file line number Diff line number Diff line change
@@ -1,49 +1,43 @@
import { render, screen } from '@testing-library/react';
import { render, screen, cleanup } from '@testing-library/react';
import React from 'react';
import { renderToString } from 'react-dom/server';
import { vi } from 'vitest';

import createHTMLBlock from '../../components/HTMLBlock';
import HTMLBlock from '../../components/HTMLBlock';
import { compile, run } from '../../index';
import createSchema from '../../sanitize.schema';

const HTMLBlock = createHTMLBlock(createSchema(), {});

describe.skip('HTML Block', () => {
describe('HTML Block', () => {
beforeEach(() => {
global.window = true;
global.mockFn = vi.fn();
});

afterEach(() => {
cleanup();
vi.restoreAllMocks();
});

it('runs user scripts in compat mode', () => {
render(<HTMLBlock html="<script>mockFn()</script>" runScripts={true} />);
render(<HTMLBlock runScripts={true}>{`<script>mockFn()</script>`}</HTMLBlock>);
expect(global.mockFn).toHaveBeenCalledTimes(1);
});

it("doesn't run user scripts by default", () => {
render(<HTMLBlock html="<script>mockFn()</script>" runScripts={false} />);
render(<HTMLBlock>{`<script>mockFn()</script>`}</HTMLBlock>);
expect(global.mockFn).toHaveBeenCalledTimes(0);
});

it("doesn't render user scripts by default", () => {
render(<HTMLBlock html="<script>mockFn()</script>" runScripts={false} />);

render(<HTMLBlock>{`<script>mockFn()</script>`}</HTMLBlock>);
expect(screen.queryByText('mockFn()')).not.toBeInTheDocument();
});

it("doesn't render user scripts with weird endings", () => {
render(<HTMLBlock html="<script>mockFn()</script foo='bar'>" runScripts={false} />);

render(<HTMLBlock>{`<script>mockFn()</script foo='bar'>`}</HTMLBlock>);
expect(screen.queryByText('mockFn()')).not.toBeInTheDocument();
});

it("doesn't render user scripts with a malicious string", () => {
render(<HTMLBlock html="<scrip<script></script>t>mockFn()</s<script></script>cript>" runScripts={false} />);

render(<HTMLBlock>{`<scrip<script></script>t>mockFn()</s<script></script>cript>`}</HTMLBlock>);
expect(screen.queryByText('mockFn()')).not.toBeInTheDocument();
});

Expand All @@ -52,24 +46,19 @@ describe.skip('HTML Block', () => {
<h1>Hello World</h1>
<script>mockFn()</script>
`;
const elem = <HTMLBlock html={html} runScripts={true} />;
const elem = <HTMLBlock runScripts={true}>{html}</HTMLBlock>;
const view = renderToString(elem);
expect(elem.props.runScripts).toBe(true);
expect(view.indexOf('<script>')).toBeLessThan(0);
expect(view.indexOf('<h1>')).toBeGreaterThanOrEqual(0);
});

it('renders the html in a `<pre>` tag if safeMode={true}', () => {
const md = `
[block:html]
${JSON.stringify({
html: '<button onload="alert(\'gotcha!\')"/>',
})}
[/block]
`;

expect(renderToString(run(compile(md, { safeMode: true })))).toMatchInlineSnapshot(
'"<pre class=\\"html-unsafe\\" data-reactroot=\\"\\"><code>&lt;button onload=&quot;alert(&#39;gotcha!&#39;)&quot;/&gt;</code></pre>"'
it('renders the html in a `<pre>` tag if safeMode={true}', async () => {
const md = '<HTMLBlock safeMode={true}>{`<button onload="alert(\'gotcha!\')"/>`}</HTMLBlock>';
const code = compile(md);
const Component = await run(code);
expect(renderToString(<Component />)).toMatchInlineSnapshot(
'"<pre class="html-unsafe"><code>&lt;button onload=&quot;alert(&#x27;gotcha!&#x27;)&quot;/&gt;</code></pre>"',
);
});
});
13 changes: 0 additions & 13 deletions __tests__/components/__snapshots__/test.js.snap

This file was deleted.

230 changes: 0 additions & 230 deletions __tests__/components/test.js

This file was deleted.

9 changes: 3 additions & 6 deletions components/HTMLBlock/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -13,12 +13,9 @@ const extractScripts = (html: string = ''): [string, () => void] => {
return [cleaned, () => scripts.map(js => window.eval(js))];
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Like, there's no reason to strip script tags if the whole thing is getting executed?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it only gets executed if you pass runScripts=true? we should probably leave it to the user to decide whether or not they wanna get h4x0rd

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

HTML blocks were originally a way to say, hey this code is dangerous and we know it. All other 'inlined' html would get tags and attributes sanitized. But I don't see how we could even begin to sanitize MDX.

The only thing I can think of, is if some existing page with an html block has an unknown malicious script in it, and all of sudden we gave it life?

};

const HTMLBlock = props => {
const { children, runScripts, safeMode = false } = props;
const HTMLBlock = ({ children = '', runScripts = false, safeMode = false }) => {
let html = children;

if (typeof html !== 'string') html = renderToStaticMarkup(html);

const [cleanedHtml, exec] = extractScripts(html);

useEffect(() => {
Expand All @@ -28,12 +25,12 @@ const HTMLBlock = props => {
if (safeMode) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

safeMode was my silly attempt to make suggested edits safer. Suggested edits could be submitted by anonymous users. And it features a preview mode, so if the submitter wasn't an admin, we wouldn't render the HTML. 🤷🏼

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

that was the usage for us sure, but i'm wondering about the 1000 or downloads a week of @readme/markdown? is this new improved renderer going to be internal only or are we improving on the current one?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

okay, we can figure out the final details in another PR since it's a bit out of scope for this one 😅

return (
<pre className="html-unsafe">
<code>{cleanedHtml}</code>
<code>{html}</code>
</pre>
);
}

return <div className="rdmd-html" dangerouslySetInnerHTML={{ __html: html }} />;
return <div className="rdmd-html" dangerouslySetInnerHTML={{ __html: cleanedHtml }} />;
};

export default HTMLBlock;
Loading