From 42d700cf08417496f01331e2b917d5fb1b7047b6 Mon Sep 17 00:00:00 2001 From: valer23 Date: Fri, 17 Apr 2026 10:28:38 +0200 Subject: [PATCH] fix: avoid mutating original server array and add null guard Address code review findings: - Sort a shallow copy instead of mutating the caller's array - Add null guard on server.name to handle malformed entries - Use original SPEEDTEST_SERVERS index for classic UI option values --- frontend/javascript/index.js | 18 ++++++++++-------- index-classic.html | 26 ++++++++++++++------------ 2 files changed, 24 insertions(+), 20 deletions(-) diff --git a/frontend/javascript/index.js b/frontend/javascript/index.js index 146ae3d..e7f4aa4 100644 --- a/frontend/javascript/index.js +++ b/frontend/javascript/index.js @@ -231,18 +231,20 @@ function populateDropdown(servers) { } // Sort servers by country, then by city within the same country - servers.sort((a, b) => { - const commaA = a.name.lastIndexOf(","); - const commaB = b.name.lastIndexOf(","); - const countryA = commaA >= 0 ? a.name.substring(commaA + 1).trim() : a.name; - const countryB = commaB >= 0 ? b.name.substring(commaB + 1).trim() : b.name; - const cityA = commaA >= 0 ? a.name.substring(0, commaA).trim() : ""; - const cityB = commaB >= 0 ? b.name.substring(0, commaB).trim() : ""; + const sorted = [...servers].sort((a, b) => { + const nameA = a.name || ""; + const nameB = b.name || ""; + const commaA = nameA.lastIndexOf(","); + const commaB = nameB.lastIndexOf(","); + const countryA = commaA >= 0 ? nameA.substring(commaA + 1).trim() : nameA; + const countryB = commaB >= 0 ? nameB.substring(commaB + 1).trim() : nameB; + const cityA = commaA >= 0 ? nameA.substring(0, commaA).trim() : ""; + const cityB = commaB >= 0 ? nameB.substring(0, commaB).trim() : ""; return countryA.localeCompare(countryB) || cityA.localeCompare(cityB); }); // Populate the list to choose from - servers.forEach((server) => { + sorted.forEach((server) => { const item = document.createElement("li"); const link = document.createElement("a"); link.href = "#"; diff --git a/index-classic.html b/index-classic.html index 7b55926..9a8c0fd 100644 --- a/index-classic.html +++ b/index-classic.html @@ -51,22 +51,24 @@ if (server != null) { //at least 1 server is available I("loading").className = "hidden"; //hide loading message //sort servers by country, then by city - SPEEDTEST_SERVERS.sort(function (a, b) { - var commaA = a.name.lastIndexOf(","); - var commaB = b.name.lastIndexOf(","); - var countryA = commaA >= 0 ? a.name.substring(commaA + 1).trim() : a.name; - var countryB = commaB >= 0 ? b.name.substring(commaB + 1).trim() : b.name; - var cityA = commaA >= 0 ? a.name.substring(0, commaA).trim() : ""; - var cityB = commaB >= 0 ? b.name.substring(0, commaB).trim() : ""; + var sortedServers = SPEEDTEST_SERVERS.slice().sort(function (a, b) { + var nameA = a.name || ""; + var nameB = b.name || ""; + var commaA = nameA.lastIndexOf(","); + var commaB = nameB.lastIndexOf(","); + var countryA = commaA >= 0 ? nameA.substring(commaA + 1).trim() : nameA; + var countryB = commaB >= 0 ? nameB.substring(commaB + 1).trim() : nameB; + var cityA = commaA >= 0 ? nameA.substring(0, commaA).trim() : ""; + var cityB = commaB >= 0 ? nameB.substring(0, commaB).trim() : ""; return countryA.localeCompare(countryB) || cityA.localeCompare(cityB); }); //populate server list for manual selection - for (var i = 0; i < SPEEDTEST_SERVERS.length; i++) { - if (SPEEDTEST_SERVERS[i].pingT == -1) continue; + for (var i = 0; i < sortedServers.length; i++) { + if (sortedServers[i].pingT == -1) continue; var option = document.createElement("option"); - option.value = i; - option.textContent = SPEEDTEST_SERVERS[i].name; - if (SPEEDTEST_SERVERS[i] === server) option.selected = true; + option.value = SPEEDTEST_SERVERS.indexOf(sortedServers[i]); + option.textContent = sortedServers[i].name; + if (sortedServers[i] === server) option.selected = true; I("server").appendChild(option); } //show test UI