Commit db9606da authored by Kushal Pandya's avatar Kushal Pandya

Merge branch 'katex-dos' into 'master'

Fix DOS on Math blocks

See merge request gitlab-org/gitlab!54898
parents e0bd8382 857acccb
import { deprecatedCreateFlash as flash } from '~/flash'; import { spriteIcon } from '~/lib/utils/common_utils';
import { differenceInMilliseconds } from '~/lib/utils/datetime_utility'; import { differenceInMilliseconds } from '~/lib/utils/datetime_utility';
import { s__, sprintf } from '~/locale'; import { s__ } from '~/locale';
// Renders math using KaTeX in any element with the // Renders math using KaTeX in any element with the
// `js-render-math` class // `js-render-math` class
...@@ -13,30 +13,10 @@ import { s__, sprintf } from '~/locale'; ...@@ -13,30 +13,10 @@ import { s__, sprintf } from '~/locale';
const MAX_MATH_CHARS = 1000; const MAX_MATH_CHARS = 1000;
const MAX_RENDER_TIME_MS = 2000; const MAX_RENDER_TIME_MS = 2000;
// These messages might be used with inline errors in the future. Keep them around. For now, we will
// display a single error message using flash().
// const CHAR_LIMIT_EXCEEDED_MSG = sprintf(
// s__(
// 'math|The following math is too long. For performance reasons, math blocks are limited to %{maxChars} characters. Try splitting up this block, or include an image instead.',
// ),
// { maxChars: MAX_MATH_CHARS },
// );
// const RENDER_TIME_EXCEEDED_MSG = s__(
// "math|The math in this entry is taking too long to render. Any math below this point won't be shown. Consider splitting it among multiple entries.",
// );
const RENDER_FLASH_MSG = sprintf(
s__(
'math|The math in this entry is taking too long to render and may not be displayed as expected. For performance reasons, math blocks are also limited to %{maxChars} characters. Consider splitting up large formulae, splitting math blocks among multiple entries, or using an image instead.',
),
{ maxChars: MAX_MATH_CHARS },
);
// Wait for the browser to reflow the layout. Reflowing SVG takes time. // Wait for the browser to reflow the layout. Reflowing SVG takes time.
// This has to wrap the inner function, otherwise IE/Edge throw "invalid calling object". // This has to wrap the inner function, otherwise IE/Edge throw "invalid calling object".
const waitForReflow = (fn) => { const waitForReflow = (fn) => {
window.requestAnimationFrame(fn); window.requestIdleCallback(fn);
}; };
/** /**
...@@ -67,37 +47,69 @@ class SafeMathRenderer { ...@@ -67,37 +47,69 @@ class SafeMathRenderer {
this.renderElement = this.renderElement.bind(this); this.renderElement = this.renderElement.bind(this);
this.render = this.render.bind(this); this.render = this.render.bind(this);
this.attachEvents = this.attachEvents.bind(this);
} }
renderElement() { renderElement(chosenEl) {
if (!this.queue.length) { if (!this.queue.length && !chosenEl) {
return; return;
} }
const el = this.queue.shift(); const el = chosenEl || this.queue.shift();
const forceRender = Boolean(chosenEl);
const text = el.textContent; const text = el.textContent;
el.removeAttribute('style'); el.removeAttribute('style');
if (!forceRender && (this.totalMS >= MAX_RENDER_TIME_MS || text.length > MAX_MATH_CHARS)) {
if (this.totalMS >= MAX_RENDER_TIME_MS || text.length > MAX_MATH_CHARS) {
if (!this.flashShown) {
flash(RENDER_FLASH_MSG);
this.flashShown = true;
}
// Show unrendered math code // Show unrendered math code
const wrapperElement = document.createElement('div');
const codeElement = document.createElement('pre'); const codeElement = document.createElement('pre');
codeElement.className = 'code'; codeElement.className = 'code';
codeElement.textContent = el.textContent; codeElement.textContent = el.textContent;
el.parentNode.replaceChild(codeElement, el);
const { parentNode } = el;
parentNode.replaceChild(wrapperElement, el);
const html = `
<div class="alert gl-alert gl-alert-warning alert-dismissible lazy-render-math-container js-lazy-render-math-container fade show" role="alert">
${spriteIcon('warning', 'text-warning-600 s16 gl-alert-icon')}
<div class="display-flex gl-alert-content">
<div>${s__(
'math|Displaying this math block may cause performance issues on this page',
)}</div>
<div class="gl-alert-actions">
<button class="js-lazy-render-math btn gl-alert-action btn-primary btn-md gl-button">Display anyway</button>
</div>
</div>
<button type="button" class="close" data-dismiss="alert" aria-label="Close">
${spriteIcon('close', 's16')}
</button>
</div>
`;
if (!wrapperElement.classList.contains('lazy-alert-shown')) {
wrapperElement.innerHTML = html;
wrapperElement.append(codeElement);
wrapperElement.classList.add('lazy-alert-shown');
}
// Render the next math // Render the next math
this.renderElement(); this.renderElement();
} else { } else {
this.startTime = Date.now(); this.startTime = Date.now();
/* Get the correct reference to the display container when:
* a.) Happy path: when the math block is present, and
* b.) When we've replace the block with <pre> for lazy rendering
*/
let displayContainer = el;
if (el.tagName === 'PRE') {
displayContainer = el.parentElement;
}
try { try {
el.innerHTML = this.katex.renderToString(text, { displayContainer.innerHTML = this.katex.renderToString(text, {
displayMode: el.getAttribute('data-math-style') === 'display', displayMode: el.getAttribute('data-math-style') === 'display',
throwOnError: true, throwOnError: true,
maxSize: 20, maxSize: 20,
...@@ -135,6 +147,22 @@ class SafeMathRenderer { ...@@ -135,6 +147,22 @@ class SafeMathRenderer {
// and less prone to timeouts. // and less prone to timeouts.
setTimeout(this.renderElement, 400); setTimeout(this.renderElement, 400);
} }
attachEvents() {
document.body.addEventListener('click', (event) => {
if (!event.target.classList.contains('js-lazy-render-math')) {
return;
}
const parent = event.target.closest('.js-lazy-render-math-container');
const pre = parent.nextElementSibling;
parent.remove();
this.renderElement(pre);
});
}
} }
export default function renderMath($els) { export default function renderMath($els) {
...@@ -146,6 +174,7 @@ export default function renderMath($els) { ...@@ -146,6 +174,7 @@ export default function renderMath($els) {
.then(([katex]) => { .then(([katex]) => {
const renderer = new SafeMathRenderer($els.get(), katex); const renderer = new SafeMathRenderer($els.get(), katex);
renderer.render(); renderer.render();
renderer.attachEvents();
}) })
.catch(() => {}); .catch(() => {});
} }
---
title: Fix DOS on Math blocks
merge_request: 54898
author:
type: changed
...@@ -35863,7 +35863,7 @@ msgstr "" ...@@ -35863,7 +35863,7 @@ msgstr ""
msgid "manual" msgid "manual"
msgstr "" msgstr ""
msgid "math|The math in this entry is taking too long to render and may not be displayed as expected. For performance reasons, math blocks are also limited to %{maxChars} characters. Consider splitting up large formulae, splitting math blocks among multiple entries, or using an image instead." msgid "math|Displaying this math block may cause performance issues on this page"
msgstr "" msgstr ""
msgid "math|There was an error rendering this math block" msgid "math|There was an error rendering this math block"
......
...@@ -39,4 +39,20 @@ RSpec.describe 'Math rendering', :js do ...@@ -39,4 +39,20 @@ RSpec.describe 'Math rendering', :js do
expect(page).to have_selector('.katex-html a', text: 'Gitlab') expect(page).to have_selector('.katex-html a', text: 'Gitlab')
end end
end end
it 'renders lazy load button' do
description = <<~MATH
```math
\Huge \sqrt{\sqrt{\sqrt{\sqrt{\sqrt{\sqrt{\sqrt{\sqrt{\sqrt{\sqrt{\sqrt{\sqrt{\sqrt{\sqrt{\sqrt{\sqrt{\sqrt{\sqrt{\sqrt{\sqrt{\sqrt{\sqrt{\sqrt{\sqrt{\sqrt{\sqrt{\sqrt{\sqrt{\sqrt{\sqrt{\sqrt{\sqrt{\sqrt{\sqrt{\sqrt{\sqrt{\sqrt{\sqrt{\sqrt{\sqrt{\sqrt{\sqrt{\sqrt{\sqrt{\sqrt{\sqrt{\sqrt{\sqrt{\sqrt{\sqrt{\sqrt{\sqrt{\sqrt{\sqrt{\sqrt{\sqrt{\sqrt{\sqrt{\sqrt{\sqrt{\sqrt{\sqrt{\sqrt{\sqrt{\sqrt{\sqrt{\sqrt{\sqrt{\sqrt{\sqrt{\sqrt{\sqrt{\sqrt{\sqrt{\sqrt{\sqrt{\sqrt{\sqrt{\sqrt{\sqrt{\sqrt{\sqrt{\sqrt{\sqrt{\sqrt{\sqrt{\sqrt{\sqrt{\sqrt{\sqrt{\sqrt{\sqrt{\sqrt{\sqrt{\sqrt{\sqrt{\sqrt{\sqrt{\sqrt{\sqrt{\sqrt{\sqrt{\sqrt{\sqrt{\sqrt{\sqrt{\sqrt{\sqrt{\sqrt{\sqrt{\sqrt{\sqrt{\sqrt{\sqrt{\sqrt{\sqrt{\sqrt{\sqrt{\sqrt{\sqrt{\sqrt{\sqrt{\sqrt{\sqrt{\sqrt{\sqrt{\sqrt{\sqrt{\sqrt{\sqrt{\sqrt{\sqrt{\sqrt{\sqrt{\sqrt{\sqrt{\sqrt{\sqrt{\sqrt{\sqrt{\sqrt{\sqrt{\sqrt{\sqrt{\sqrt{\sqrt{\sqrt{\sqrt{\sqrt{\sqrt{\sqrt{\sqrt{\sqrt{\sqrt{\sqrt{\sqrt{\sqrt{\sqrt{\sqrt{\sqrt{\sqrt{\sqrt{\sqrt{\sqrt{\sqrt{\sqrt{\sqrt{\sqrt{\sqrt{\sqrt{\sqrt{\sqrt{\sqrt{\sqrt{\sqrt{\sqrt{\sqrt{\sqrt{\sqrt{\sqrt{\sqrt{\sqrt{\sqrt{\sqrt{\sqrt{\sqrt{\sqrt{\sqrt{\sqrt{\sqrt{\sqrt{\sqrt{\sqrt{\sqrt{\sqrt{\sqrt{\sqrt{\sqrt{\sqrt{\sqrt{\sqrt{\sqrt{\sqrt{\sqrt{\sqrt{\sqrt{\sqrt{\sqrt{\sqrt{\sqrt{\sqrt{\sqrt{}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}}
```
MATH
issue = create(:issue, project: project, description: description)
visit project_issue_path(project, issue)
page.within '.description > .md' do
expect(page).to have_selector('.js-lazy-render-math')
end
end
end end
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment