Commit 22b3240d authored by Rajat Jain's avatar Rajat Jain

Fix mermaid resource consumption in GFM fields

Since mermaid is an expensive operation, we defer the execution
when the browser is idle.
parent 429836ae
...@@ -18,7 +18,13 @@ import { __, sprintf } from '~/locale'; ...@@ -18,7 +18,13 @@ import { __, sprintf } from '~/locale';
// //
// This is an arbitrary number; Can be iterated upon when suitable. // This is an arbitrary number; Can be iterated upon when suitable.
const MAX_CHAR_LIMIT = 5000; const MAX_CHAR_LIMIT = 2000;
// Max # of mermaid blocks that can be rendered in a page.
const MAX_MERMAID_BLOCK_LIMIT = 50;
// Keep a map of mermaid blocks we've already rendered.
const elsProcessingMap = new WeakMap();
let renderedMermaidBlocks = 0;
let mermaidModule = {}; let mermaidModule = {};
function importMermaidModule() { function importMermaidModule() {
...@@ -110,13 +116,22 @@ function renderMermaids($els) { ...@@ -110,13 +116,22 @@ function renderMermaids($els) {
let renderedChars = 0; let renderedChars = 0;
$els.each((i, el) => { $els.each((i, el) => {
// Skipping all the elements which we've already queued in requestIdleCallback
if (elsProcessingMap.has(el)) {
return;
}
const { source } = fixElementSource(el); const { source } = fixElementSource(el);
/** /**
* Restrict the rendering to a certain amount of character to * Restrict the rendering to a certain amount of character
* prevent mermaidjs from hanging up the entire thread and * and mermaid blocks to prevent mermaidjs from hanging
* causing a DoS. * up the entire thread and causing a DoS.
*/ */
if ((source && source.length > MAX_CHAR_LIMIT) || renderedChars > MAX_CHAR_LIMIT) { if (
(source && source.length > MAX_CHAR_LIMIT) ||
renderedChars > MAX_CHAR_LIMIT ||
renderedMermaidBlocks >= MAX_MERMAID_BLOCK_LIMIT
) {
const html = ` const html = `
<div class="alert gl-alert gl-alert-warning alert-dismissible lazy-render-mermaid-container js-lazy-render-mermaid-container fade show" role="alert"> <div class="alert gl-alert gl-alert-warning alert-dismissible lazy-render-mermaid-container js-lazy-render-mermaid-container fade show" role="alert">
<div> <div>
...@@ -146,9 +161,14 @@ function renderMermaids($els) { ...@@ -146,9 +161,14 @@ function renderMermaids($els) {
} }
renderedChars += source.length; renderedChars += source.length;
renderedMermaidBlocks += 1;
const requestId = window.requestIdleCallback(() => {
renderMermaidEl(el); renderMermaidEl(el);
}); });
elsProcessingMap.set(el, requestId);
});
}) })
.catch(err => { .catch(err => {
flash(sprintf(__('Encountered an error while rendering: %{err}'), { err })); flash(sprintf(__('Encountered an error while rendering: %{err}'), { err }));
......
---
title: Fix mermaid resource consumption in GFM fields
merge_request:
author:
type: security
...@@ -19,6 +19,9 @@ RSpec.describe 'Mermaid rendering', :js do ...@@ -19,6 +19,9 @@ RSpec.describe 'Mermaid rendering', :js do
visit project_issue_path(project, issue) visit project_issue_path(project, issue)
wait_for_requests
wait_for_mermaid
%w[A B C D].each do |label| %w[A B C D].each do |label|
expect(page).to have_selector('svg text', text: label) expect(page).to have_selector('svg text', text: label)
end end
...@@ -39,6 +42,7 @@ RSpec.describe 'Mermaid rendering', :js do ...@@ -39,6 +42,7 @@ RSpec.describe 'Mermaid rendering', :js do
visit project_issue_path(project, issue) visit project_issue_path(project, issue)
wait_for_requests wait_for_requests
wait_for_mermaid
expected = '<text style=""><tspan xml:space="preserve" dy="1em" x="1">Line 1</tspan><tspan xml:space="preserve" dy="1em" x="1">Line 2</tspan></text>' expected = '<text style=""><tspan xml:space="preserve" dy="1em" x="1">Line 1</tspan><tspan xml:space="preserve" dy="1em" x="1">Line 2</tspan></text>'
expect(page.html.scan(expected).count).to be(4) expect(page.html.scan(expected).count).to be(4)
...@@ -65,6 +69,9 @@ RSpec.describe 'Mermaid rendering', :js do ...@@ -65,6 +69,9 @@ RSpec.describe 'Mermaid rendering', :js do
visit project_issue_path(project, issue) visit project_issue_path(project, issue)
wait_for_requests
wait_for_mermaid
page.within('.description') do page.within('.description') do
expect(page).to have_selector('svg') expect(page).to have_selector('svg')
expect(page).to have_selector('pre.mermaid') expect(page).to have_selector('pre.mermaid')
...@@ -92,6 +99,9 @@ RSpec.describe 'Mermaid rendering', :js do ...@@ -92,6 +99,9 @@ RSpec.describe 'Mermaid rendering', :js do
visit project_issue_path(project, issue) visit project_issue_path(project, issue)
wait_for_requests
wait_for_mermaid
page.within('.description') do page.within('.description') do
page.find('summary').click page.find('summary').click
svg = page.find('svg.mermaid') svg = page.find('svg.mermaid')
...@@ -118,6 +128,9 @@ RSpec.describe 'Mermaid rendering', :js do ...@@ -118,6 +128,9 @@ RSpec.describe 'Mermaid rendering', :js do
visit project_issue_path(project, issue) visit project_issue_path(project, issue)
wait_for_requests
wait_for_mermaid
expect(page).to have_css('svg.mermaid[style*="max-width"][width="100%"]') expect(page).to have_css('svg.mermaid[style*="max-width"][width="100%"]')
end end
...@@ -147,6 +160,7 @@ RSpec.describe 'Mermaid rendering', :js do ...@@ -147,6 +160,7 @@ RSpec.describe 'Mermaid rendering', :js do
end end
wait_for_requests wait_for_requests
wait_for_mermaid
find('.js-lazy-render-mermaid').click find('.js-lazy-render-mermaid').click
...@@ -156,4 +170,55 @@ RSpec.describe 'Mermaid rendering', :js do ...@@ -156,4 +170,55 @@ RSpec.describe 'Mermaid rendering', :js do
expect(page).not_to have_selector('.js-lazy-render-mermaid-container') expect(page).not_to have_selector('.js-lazy-render-mermaid-container')
end end
end end
it 'does not render more than 50 mermaid blocks', :js, quarantine: { issue: 'https://gitlab.com/gitlab-org/gitlab/-/issues/234081' } do
graph_edges = "A-->B;B-->A;"
description = <<~MERMAID
```mermaid
graph LR
#{graph_edges}
```
MERMAID
description *= 51
project = create(:project, :public)
issue = create(:issue, project: project, description: description)
visit project_issue_path(project, issue)
wait_for_requests
wait_for_mermaid
page.within('.description') do
expect(page).to have_selector('svg')
expect(page).to have_selector('.lazy-alert-shown')
expect(page).to have_selector('.js-lazy-render-mermaid-container')
end
end
end
def wait_for_mermaid
run_idle_callback = <<~RUN_IDLE_CALLBACK
window.requestIdleCallback(() => {
window.__CAPYBARA_IDLE_CALLBACK_EXEC__ = 1;
})
RUN_IDLE_CALLBACK
page.evaluate_script(run_idle_callback)
Timeout.timeout(Capybara.default_max_wait_time) do
loop until finished_rendering?
end
end
def finished_rendering?
check_idle_callback = <<~CHECK_IDLE_CALLBACK
window.__CAPYBARA_IDLE_CALLBACK_EXEC__
CHECK_IDLE_CALLBACK
page.evaluate_script(check_idle_callback) == 1
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