From f9b4efd42c17d7f75b689142b17575a478fe903c Mon Sep 17 00:00:00 2001 From: Yarden Shoham Date: Sat, 16 Mar 2024 15:25:27 +0200 Subject: [PATCH] Forbid HTML injection using jQuery (#29843) See https://github.com/wikimedia/eslint-plugin-no-jquery/blob/master/docs/rules/no-append-html.md Tested the following components and they work as before: - notification table - issue author dropdown - comment edit box attachments div Signed-off-by: Yarden Shoham Co-authored-by: Giteabot --- .eslintrc.yaml | 2 +- web_src/js/features/notification.js | 6 +++--- web_src/js/features/repo-issue-list.js | 4 +++- web_src/js/features/repo-legacy.js | 5 ++--- web_src/js/modules/fomantic/dropdown.js | 4 +++- 5 files changed, 12 insertions(+), 9 deletions(-) diff --git a/.eslintrc.yaml b/.eslintrc.yaml index 5f291f13e7..0003ba95e1 100644 --- a/.eslintrc.yaml +++ b/.eslintrc.yaml @@ -400,7 +400,7 @@ rules: no-jquery/no-and-self: [2] no-jquery/no-animate-toggle: [2] no-jquery/no-animate: [2] - no-jquery/no-append-html: [0] + no-jquery/no-append-html: [2] no-jquery/no-attr: [0] no-jquery/no-bind: [2] no-jquery/no-box-model: [2] diff --git a/web_src/js/features/notification.js b/web_src/js/features/notification.js index 57b7bcb6e8..90e19b683b 100644 --- a/web_src/js/features/notification.js +++ b/web_src/js/features/notification.js @@ -143,8 +143,8 @@ async function updateNotificationCountWithCallback(callback, timeout, lastCount) } async function updateNotificationTable() { - const $notificationDiv = $('#notification_div'); - if ($notificationDiv.length > 0) { + const notificationDiv = document.getElementById('notification_div'); + if (notificationDiv) { try { const params = new URLSearchParams(window.location.search); params.set('div-only', true); @@ -158,7 +158,7 @@ async function updateNotificationTable() { const data = await response.text(); if ($(data).data('sequence-number') === notificationSequenceNumber) { - $notificationDiv.replaceWith(data); + notificationDiv.outerHTML = data; initNotificationsTable(); } } catch (error) { diff --git a/web_src/js/features/repo-issue-list.js b/web_src/js/features/repo-issue-list.js index efc7671204..21f1865732 100644 --- a/web_src/js/features/repo-issue-list.js +++ b/web_src/js/features/repo-issue-list.js @@ -125,7 +125,9 @@ function initRepoIssueListAuthorDropdown() { if (newMenuHtml) { const $newMenuItems = $(newMenuHtml); $newMenuItems.addClass('dynamic-item'); - $menu.append('
', ...$newMenuItems); + const div = document.createElement('div'); + div.classList.add('divider', 'dynamic-item'); + $menu[0].append(div, ...$newMenuItems); } $searchDropdown.dropdown('refresh'); // defer our selection to the next tick, because dropdown will set the selection item after this `menu` function diff --git a/web_src/js/features/repo-legacy.js b/web_src/js/features/repo-legacy.js index 1df409c79e..10c25bf28b 100644 --- a/web_src/js/features/repo-legacy.js +++ b/web_src/js/features/repo-legacy.js @@ -436,13 +436,12 @@ async function onEditContent(event) { const $content = $segment; if (!$content.find('.dropzone-attachments').length) { if (data.attachments !== '') { - $content.append(`
`); - $content.find('.dropzone-attachments').replaceWith(data.attachments); + $content[0].append(data.attachments); } } else if (data.attachments === '') { $content.find('.dropzone-attachments').remove(); } else { - $content.find('.dropzone-attachments').replaceWith(data.attachments); + $content.find('.dropzone-attachments')[0].outerHTML = data.attachments; } if (dz) { dz.emit('submit'); diff --git a/web_src/js/modules/fomantic/dropdown.js b/web_src/js/modules/fomantic/dropdown.js index caba8a2f28..7302078dbd 100644 --- a/web_src/js/modules/fomantic/dropdown.js +++ b/web_src/js/modules/fomantic/dropdown.js @@ -72,7 +72,9 @@ function delegateOne($dropdown) { dropdownTemplates.menu = function(response, fields, preserveHTML, className) { // when the dropdown menu items are loaded from AJAX requests, the items are created dynamically const menuItems = dropdownTemplatesMenuOld(response, fields, preserveHTML, className); - const $wrapper = $('
').append(menuItems); + const div = document.createElement('div'); + div.innerHTML = menuItems; + const $wrapper = $(div); const $items = $wrapper.find('> .item'); $items.each((_, item) => updateMenuItem($dropdown[0], item)); $dropdown[0][ariaPatchKey].deferredRefreshAriaActiveItem();