From 75f54ff76a3ee09263f3a67895bba0fd3bc3d689 Mon Sep 17 00:00:00 2001 From: DL6ER Date: Thu, 30 May 2024 12:39:25 +0200 Subject: [PATCH 1/5] Fix/simplify mass deletion of tables, simplify deleting local DNS / CNAME records Signed-off-by: DL6ER --- scripts/pi-hole/js/messages.js | 7 ++----- scripts/pi-hole/js/settings-dhcp.js | 21 ++++----------------- scripts/pi-hole/js/settings-dns-records.js | 15 ++------------- 3 files changed, 8 insertions(+), 35 deletions(-) diff --git a/scripts/pi-hole/js/messages.js b/scripts/pi-hole/js/messages.js index 15a415fd1..7e9a72401 100644 --- a/scripts/pi-hole/js/messages.js +++ b/scripts/pi-hole/js/messages.js @@ -98,13 +98,10 @@ $(function () { className: "btn-sm datatable-bt deleteSelected", action: function () { // For each ".selected" row ... - var ids = []; $("tr.selected").each(function () { - // ... add the row identified by "data-id". - ids.push(parseInt($(this).attr("data-id"), 10)); + // ... delete the row identified by "data-id". + delMsg($(this).attr("data-id")); }); - // Delete all selected rows at once - delMsg(ids); }, }, ], diff --git a/scripts/pi-hole/js/settings-dhcp.js b/scripts/pi-hole/js/settings-dhcp.js index 56b10ab4e..57ca67021 100644 --- a/scripts/pi-hole/js/settings-dhcp.js +++ b/scripts/pi-hole/js/settings-dhcp.js @@ -115,13 +115,10 @@ $(function () { className: "btn-sm datatable-bt deleteSelected", action: function () { // For each ".selected" row ... - var ids = []; $("tr.selected").each(function () { - // ... add the row identified by "data-id". - ids.push(parseInt($(this).attr("data-id"), 10)); + // ... delete the row identified by "data-id". + delLease($(this).attr("data-id")); }); - // Delete all selected rows at once - delLease(ids); }, }, ], @@ -161,17 +158,7 @@ $(function () { function deleteLease() { // Passes the button data-del-id attribute as IP - var ips = [$(this).attr("data-del-ip")]; - - // Check input validity - if (!Array.isArray(ips)) return; - - // Exploit prevention: Return early for non-numeric IDs - for (var ip in ips) { - if (Object.hasOwnProperty.call(ips, ip)) { - delLease(ips); - } - } + delLease($(this).attr("data-del-ip")); } function delLease(ip) { @@ -179,7 +166,7 @@ function delLease(ip) { utils.showAlert("info", "", "Deleting lease..."); $.ajax({ - url: "/api/dhcp/leases/" + ip, + url: "/api/dhcp/leases/" + encodeURIComponent(ip), method: "DELETE", }) .done(function (response) { diff --git a/scripts/pi-hole/js/settings-dns-records.js b/scripts/pi-hole/js/settings-dns-records.js index d664d8ee6..28f61de08 100644 --- a/scripts/pi-hole/js/settings-dns-records.js +++ b/scripts/pi-hole/js/settings-dns-records.js @@ -141,19 +141,8 @@ $(function () { }); function deleteRecord() { - // Get the tags - var tags = [$(this).attr("data-tag")]; - var types = [$(this).attr("data-type")]; - // Check input validity - if (!Array.isArray(tags)) return; - - // Exploit prevention: Return early for non-numeric IDs - for (var tag in tags) { - if (Object.hasOwnProperty.call(tags, tag)) { - if (types[0] === "hosts") delHosts(tags); - else delCNAME(tags); - } - } + if ($(this).attr("data-type") === "hosts") delHosts($(this).attr("data-tag")); + else delCNAME($(this).attr("data-tag")); } function delHosts(elem) { From 2d12a28a56996630b1aafbe882cf4453658d202b Mon Sep 17 00:00:00 2001 From: DL6ER Date: Mon, 3 Jun 2024 09:38:56 +0200 Subject: [PATCH 2/5] Fix multi-deletion of DHCP leases Signed-off-by: DL6ER --- scripts/pi-hole/js/settings-dhcp.js | 8 ++++---- scripts/pi-hole/js/utils.js | 32 ++++++++++++++++++++--------- 2 files changed, 26 insertions(+), 14 deletions(-) diff --git a/scripts/pi-hole/js/settings-dhcp.js b/scripts/pi-hole/js/settings-dhcp.js index 57ca67021..239889cae 100644 --- a/scripts/pi-hole/js/settings-dhcp.js +++ b/scripts/pi-hole/js/settings-dhcp.js @@ -163,7 +163,7 @@ function deleteLease() { function delLease(ip) { utils.disableAll(); - utils.showAlert("info", "", "Deleting lease..."); + const toast = utils.showAlert("info", "", "Deleting lease...", ip, null); $.ajax({ url: "/api/dhcp/leases/" + encodeURIComponent(ip), @@ -172,10 +172,10 @@ function delLease(ip) { .done(function (response) { utils.enableAll(); if (response === undefined) { - utils.showAlert("success", "far fa-trash-alt", "Successfully deleted lease", ""); + utils.showAlert("success", "far fa-trash-alt", "Successfully deleted lease", ip, toast); dhcpLeaesTable.ajax.reload(null, false); } else { - utils.showAlert("error", "", "Error while deleting lease: " + ip, response.lease); + utils.showAlert("error", "", "Error while deleting lease: " + ip, response.lease, toast); } // Clear selection after deletion @@ -184,7 +184,7 @@ function delLease(ip) { }) .fail(function (jqXHR, exception) { utils.enableAll(); - utils.showAlert("error", "", "Error while deleting lease: " + ip, jqXHR.responseText); + utils.showAlert("error", "", "Error while deleting lease: " + ip, jqXHR.responseText, toast); console.log(exception); // eslint-disable-line no-console }); } diff --git a/scripts/pi-hole/js/utils.js b/scripts/pi-hole/js/utils.js index 74bf07b6e..caca985b8 100644 --- a/scripts/pi-hole/js/utils.js +++ b/scripts/pi-hole/js/utils.js @@ -84,7 +84,7 @@ function padNumber(num) { } var showAlertBox = null; -function showAlert(type, icon, title, message) { +function showAlert(type, icon, title, message, toast) { const options = { title: " " + escapeHtml(title) + "
", message: escapeHtml(message), @@ -138,16 +138,28 @@ function showAlert(type, icon, title, message) { return; } - if (type === "info") { - // Create a new notification for info boxes - showAlertBox = $.notify(options, settings); - } else if (showAlertBox !== null) { - // Update existing notification for other boxes (if available) - showAlertBox.update(options); - showAlertBox.update(settings); + if (toast === undefined) { + if (type === "info") { + // Create a new notification for info boxes + showAlertBox = $.notify(options, settings); + return showAlertBox; + } else if (showAlertBox !== null) { + // Update existing notification for other boxes (if available) + showAlertBox.update(options); + showAlertBox.update(settings); + return showAlertBox; + } else { + // Create a new notification for other boxes if no previous info box exists + return $.notify(options, settings); + } + } else if (toast === null) { + // Always create a new toast + return $.notify(options, settings); } else { - // Create a new notification for other boxes if no previous info box exists - $.notify(options, settings); + // Update existing toast + toast.update(options); + toast.update(settings); + return toast; } } From 420c5a83b4c9090ce64aaa79739b3d35950d4514 Mon Sep 17 00:00:00 2001 From: DL6ER Date: Mon, 3 Jun 2024 19:17:20 +0200 Subject: [PATCH 3/5] Toasts need to be stored globally or the variables may overwrite each other causeing a race-collision Signed-off-by: DL6ER --- scripts/pi-hole/js/settings-dhcp.js | 29 ++++++++++++++++++++++++----- 1 file changed, 24 insertions(+), 5 deletions(-) diff --git a/scripts/pi-hole/js/settings-dhcp.js b/scripts/pi-hole/js/settings-dhcp.js index 239889cae..6e467668f 100644 --- a/scripts/pi-hole/js/settings-dhcp.js +++ b/scripts/pi-hole/js/settings-dhcp.js @@ -7,7 +7,8 @@ /* global utils:false, setConfigValues: false, apiFailure: false */ -var dhcpLeaesTable = null; +var dhcpLeaesTable = null, + toasts = {}; // DHCP leases tooltips $(function () { @@ -163,7 +164,7 @@ function deleteLease() { function delLease(ip) { utils.disableAll(); - const toast = utils.showAlert("info", "", "Deleting lease...", ip, null); + toasts[ip] = utils.showAlert("info", "", "Deleting lease...", ip, null); $.ajax({ url: "/api/dhcp/leases/" + encodeURIComponent(ip), @@ -172,10 +173,22 @@ function delLease(ip) { .done(function (response) { utils.enableAll(); if (response === undefined) { - utils.showAlert("success", "far fa-trash-alt", "Successfully deleted lease", ip, toast); + utils.showAlert( + "success", + "far fa-trash-alt", + "Successfully deleted lease", + ip, + toasts[ip] + ); dhcpLeaesTable.ajax.reload(null, false); } else { - utils.showAlert("error", "", "Error while deleting lease: " + ip, response.lease, toast); + utils.showAlert( + "error", + "", + "Error while deleting lease: " + ip, + response.lease, + toasts[ip] + ); } // Clear selection after deletion @@ -184,7 +197,13 @@ function delLease(ip) { }) .fail(function (jqXHR, exception) { utils.enableAll(); - utils.showAlert("error", "", "Error while deleting lease: " + ip, jqXHR.responseText, toast); + utils.showAlert( + "error", + "", + "Error while deleting lease: " + ip, + jqXHR.responseText, + toasts[ip] + ); console.log(exception); // eslint-disable-line no-console }); } From edda66f95e43b0cce92c8cdc7c7e7d51e9c8a1e8 Mon Sep 17 00:00:00 2001 From: DL6ER Date: Mon, 3 Jun 2024 19:19:00 +0200 Subject: [PATCH 4/5] Apply the same fixes for the messages table Signed-off-by: DL6ER --- scripts/pi-hole/js/messages.js | 51 +++++++++++++++++----------------- 1 file changed, 25 insertions(+), 26 deletions(-) diff --git a/scripts/pi-hole/js/messages.js b/scripts/pi-hole/js/messages.js index 7e9a72401..c9256e087 100644 --- a/scripts/pi-hole/js/messages.js +++ b/scripts/pi-hole/js/messages.js @@ -6,7 +6,8 @@ * Please see LICENSE file for your rights under this license. */ /* global utils:false */ -var table; +var table, + toasts = {}; $(function () { var ignoreNonfatal = localStorage @@ -147,26 +148,16 @@ $.fn.dataTable.Buttons.defaults.dom.container.className = "dt-buttons"; function deleteMessage() { // Passes the button data-del-id attribute as ID - var ids = [parseInt($(this).attr("data-del-id"), 10)]; - - // Check input validity - if (!Array.isArray(ids)) return; - - // Exploit prevention: Return early for non-numeric IDs - for (var id in ids) { - if (Object.hasOwnProperty.call(ids, id)) { - if (typeof ids[id] !== "number") return; - delMsg(ids); - } - } + delMsg($(this).attr("data-del-id")); } -function delMsg(ids) { +function delMsg(id) { + id = parseInt(id, 10); utils.disableAll(); - utils.showAlert("info", "", "Deleting message..."); + toasts[id] = utils.showAlert("info", "", "Deleting message...", null, null); $.ajax({ - url: "/api/info/messages/" + ids, + url: "/api/info/messages/" + id, method: "DELETE", }) .done(function (response) { @@ -175,19 +166,21 @@ function delMsg(ids) { utils.showAlert( "success", "far fa-trash-alt", - "Successfully deleted " + ids.length + " message" + (ids.length > 1 ? "s" : ""), - "" + "Successfully deleted message", + "", + toasts[id] ); - // Loop over id in case of multiple IDs - for (var id in ids) { - if (Object.hasOwnProperty.call(ids, id)) { - table.row(id).remove(); - } - } + table.row(id).remove(); table.draw(false).ajax.reload(null, false); } else { - utils.showAlert("error", "", "Error while deleting message: " + ids, response.message); + utils.showAlert( + "error", + "", + "Error while deleting message: " + id, + response.message, + toasts[id] + ); } // Clear selection after deletion @@ -199,7 +192,13 @@ function delMsg(ids) { ) .fail(function (jqXHR, exception) { utils.enableAll(); - utils.showAlert("error", "", "Error while deleting message: " + ids, jqXHR.responseText); + utils.showAlert( + "error", + "", + "Error while deleting message: " + id, + jqXHR.responseText, + toasts[id] + ); console.log(exception); // eslint-disable-line no-console }); } From 61acb4ad33e99e3439fc40f8ab9a8d3ebe114ba9 Mon Sep 17 00:00:00 2001 From: Dominik Date: Thu, 6 Jun 2024 09:01:33 +0200 Subject: [PATCH 5/5] Apply suggestions from code review Signed-off-by: Dominik --- scripts/pi-hole/js/messages.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/scripts/pi-hole/js/messages.js b/scripts/pi-hole/js/messages.js index c9256e087..e97f5cfd6 100644 --- a/scripts/pi-hole/js/messages.js +++ b/scripts/pi-hole/js/messages.js @@ -154,7 +154,7 @@ function deleteMessage() { function delMsg(id) { id = parseInt(id, 10); utils.disableAll(); - toasts[id] = utils.showAlert("info", "", "Deleting message...", null, null); + toasts[id] = utils.showAlert("info", "", "Deleting message...", "ID: " + id, null); $.ajax({ url: "/api/info/messages/" + id, @@ -167,7 +167,7 @@ function delMsg(id) { "success", "far fa-trash-alt", "Successfully deleted message", - "", + "ID: " + id, toasts[id] ); table.row(id).remove();