From 8793fe61fe72e8ae6b484cd844dcbee3ada56aa7 Mon Sep 17 00:00:00 2001 From: alcatron <27321443+alcatron@users.noreply.github.com> Date: Sun, 11 Oct 2026 20:45:05 +1030 Subject: [PATCH 01/41] Implement separate image libraries with validated uploads --- css/editor.css | 10 ++ js/editor.js | 57 ++++++++++- lib/editor.actions.php | 27 ++++++ lib/editor.assets.php | 97 +++++++++++++++++++ .../Support/EditorImageFormatsRegression.php | 70 +++++++++++++ tests/Unit/EditorImageFormatsTest.php | 19 ++++ weathermap-cacti-plugin-editor.php | 33 ++++--- 7 files changed, 296 insertions(+), 17 deletions(-) create mode 100644 lib/editor.assets.php create mode 100644 tests/Support/EditorImageFormatsRegression.php create mode 100644 tests/Unit/EditorImageFormatsTest.php diff --git a/css/editor.css b/css/editor.css index 4f8571b1..eb974fab 100644 --- a/css/editor.css +++ b/css/editor.css @@ -316,3 +316,13 @@ body#mainView #wm-node-delete-bar { min-height: 32px; } .ui-dialog.wm-editor-dialog { overflow: visible; } + +.wm-image-delete { color: #c62828 !important; background: #fff !important; border: 1px solid #c62828 !important; font-size: 20px; } +#dlgImages .wm-image-library { width: 100%; box-sizing: border-box; display: grid; grid-template-columns: repeat(auto-fill, minmax(140px, 1fr)); gap: 10px; max-height: 240px; overflow: auto; align-content: start; padding: 4px; } +#dlgImages .wm-image-card { min-width: 0; padding: 8px; border: 1px solid #999; display: flex; flex-direction: column; gap: 6px; overflow-wrap: anywhere; align-items: center; } +#dlgImages .wm-image-card img { width: 100%; max-width: 130px; height: 70px; object-fit: contain; flex: none; } +#dlgImages .wm-image-uploader { display: flex; flex-wrap: wrap; align-items: center; gap: 8px; margin: 10px 0; } +/* Use Cacti's own theme heading colours in every grouped editor dialog. */ +.dlgProperties .wm-editor-section { display: table; width: 100%; box-sizing: border-box; margin: 10px 0 6px; } +.dlgProperties table .wm-editor-section { margin: 0; } +#wm-images-error:not(:empty) { color: inherit; border-left: 3px solid #c62828; padding-left: 8px; font-weight: bold; } diff --git a/js/editor.js b/js/editor.js index dad82c1a..afd61bef 100644 --- a/js/editor.js +++ b/js/editor.js @@ -700,6 +700,7 @@ function manage_images() { $('#action').val('set_image'); show_dialog('dlgImages'); + wmOpenImages(); } function prefs() { @@ -1159,7 +1160,7 @@ function show_dialog(dlg) { $('#'+dlg).dialog({ autoOpen: true, - width: 600, + width: dlg === 'dlgImages' ? Math.min(900, $(window).width() - 40) : 600, height: 'auto', modal: false, resizable: false, @@ -1663,3 +1664,57 @@ function wmFriendlyLinkNames() { function wmDecodeEditorUrl(value) { return String(value || '').replace(/&/g, '&'); } +function wmAssetRequest(action, extra, callback, errorId) { + var values = {action: action, mapname: $('#mapname').val()}; + if (typeof csrfMagicToken !== 'undefined') values.__csrf_magic = csrfMagicToken; + $.ajax({url: editor_url, type: action === 'image_data' ? 'GET' : 'POST', dataType: 'json', data: $.extend(values, extra), + success: callback, error: function(xhr) { $('#' + errorId).text(xhr.responseJSON && xhr.responseJSON.error || 'The request failed. Please try again.'); }}); +} +function wmAssetImages(images) { + $('#wm-images-objects, #wm-images-backgrounds').empty(); + images.forEach(function(item) { + var card = $('
').appendTo('#wm-images-' + item.kind); + $('').attr({src: item.url, alt: item.name, loading: 'lazy'}).appendTo(card); + $('').text(item.name).appendTo(card); + if (item.deletable !== false) $('
+

+
Background Images
+
+

+
Close
+
PNG, JPG/JPEG or GIF files up to 5 MB and 4 million pixels. GIF uploads use the first frame. Uploads are saved immediately. Images referenced by a map configuration cannot be deleted.
From 7a6f5b83b11b967acbe93807fcecbc99f24a0960 Mon Sep 17 00:00:00 2001 From: alcatron <27321443+alcatron@users.noreply.github.com> Date: Sun, 11 Oct 2026 21:00:38 +1030 Subject: [PATCH 02/41] Preserve full-width image library dialog layout --- css/editor.css | 3 +++ 1 file changed, 3 insertions(+) diff --git a/css/editor.css b/css/editor.css index eb974fab..740af4c7 100644 --- a/css/editor.css +++ b/css/editor.css @@ -326,3 +326,6 @@ body#mainView #wm-node-delete-bar { min-height: 32px; } .dlgProperties .wm-editor-section { display: table; width: 100%; box-sizing: border-box; margin: 10px 0 6px; } .dlgProperties table .wm-editor-section { margin: 0; } #wm-images-error:not(:empty) { color: inherit; border-left: 3px solid #c62828; padding-left: 8px; font-weight: bold; } + +#dlgImages > .dlgBody { display: block; width: 100%; box-sizing: border-box; } +#dlgImages .wm-editor-section { float: none; } From eaf031be22ad4a13742a67af1864ee544c82f107 Mon Sep 17 00:00:00 2001 From: alcatron <27321443+alcatron@users.noreply.github.com> Date: Mon, 12 Oct 2026 00:52:58 +1030 Subject: [PATCH 03/41] Protect image references and expose library usage metadata --- js/editor.js | 20 +- lib/editor.actions.php | 42 ++-- lib/editor.assets.php | 63 ++++-- lib/editor.inc.php | 19 ++ locales/po/cacti.pot | 196 +++++++++++++++++- tests/Browser/EditorImagesTest.cjs | 23 ++ .../Support/EditorImageFormatsRegression.php | 70 +++++-- tests/Unit/EditorImageFormatsTest.php | 46 ++-- weathermap-cacti-plugin-editor.php | 18 +- 9 files changed, 403 insertions(+), 94 deletions(-) create mode 100644 tests/Browser/EditorImagesTest.cjs diff --git a/js/editor.js b/js/editor.js index afd61bef..f260bf5e 100644 --- a/js/editor.js +++ b/js/editor.js @@ -1668,7 +1668,7 @@ function wmAssetRequest(action, extra, callback, errorId) { var values = {action: action, mapname: $('#mapname').val()}; if (typeof csrfMagicToken !== 'undefined') values.__csrf_magic = csrfMagicToken; $.ajax({url: editor_url, type: action === 'image_data' ? 'GET' : 'POST', dataType: 'json', data: $.extend(values, extra), - success: callback, error: function(xhr) { $('#' + errorId).text(xhr.responseJSON && xhr.responseJSON.error || 'The request failed. Please try again.'); }}); + success: callback, error: function(xhr) { $('#' + errorId).text(xhr.responseJSON && xhr.responseJSON.error || wmEditorText.imageText3); }}); } function wmAssetImages(images) { $('#wm-images-objects, #wm-images-backgrounds').empty(); @@ -1676,18 +1676,20 @@ function wmAssetImages(images) { var card = $('
').appendTo('#wm-images-' + item.kind); $('').attr({src: item.url, alt: item.name, loading: 'lazy'}).appendTo(card); $('').text(item.name).appendTo(card); - if (item.deletable !== false) $('',{runScripts:'outside-only',pretendToBeVisual:true}); +const w=dom.window,$=jquery(w);w.$=w.jQuery=$;w.editor_url='/editor';w.csrfMagicToken='fixture';$.fn.dialog=function(){return this;}; +const inc=fs.readFileSync(path.join(__dirname,'../../lib/editor.inc.php'),'utf8');w.wmEditorText={}; +for(const m of inc.matchAll(/'(imageText[0-9]+|imageUsed|imageUnused)' => __\('((?:\\.|[^'])+)'/g))w.wmEditorText[m[1]]=m[2].replace(/\\'/g,"'"); +w.wmEditorText.imageUnused='Non utilisée'; +const source=fs.readFileSync(path.join(__dirname,'../../js/editor.js'),'utf8');w.eval(source.slice(source.indexOf('function wmAssetRequest('))); +const images=[{kind:'objects',name:'hospital.png',url:'images/objects/hospital.png',width:96,height:96,users:['map.conf'],deletable:false},{kind:'backgrounds',name:'background.jpg',url:'images/backgrounds/background.jpg',width:800,height:600,users:[],deletable:true}]; +w.wmAssetImages(images); +assert.equal($('#wm-images-objects .wm-image-delete').length,0);assert.match($('#wm-images-objects').text(),/96 × 96 px/);assert.match($('#wm-images-objects').text(),/map.conf/);assert.match($('#wm-images-backgrounds').text(),/Non utilisée/); +assert.equal($('#node_iconfilename option[value="images/objects/hospital.png"]').length,1); +let request;$.ajax=opts=>request=opts;w.confirm=()=>false;$('#wm-images-backgrounds .wm-image-delete').trigger('click');assert.equal(request,undefined); +w.confirm=()=>true;$('#wm-images-backgrounds .wm-image-delete').trigger('click');assert.equal(request.type,'POST');assert.equal(request.data.action,'delete_image');assert.equal(request.data.image_kind,'backgrounds');assert.equal(request.data.__csrf_magic,'fixture'); +request.error({responseJSON:{error:'In use'}});assert.equal($('#wm-images-error').text(),'In use'); +w.wmOpenImages();assert.equal(request.type,'GET');assert.equal(request.data.action,'image_data');request.success({images}); +$('.wm-image-upload').trigger('click');assert.match($('#wm-images-error').text(),/Choose a PNG/); +Object.defineProperty($('#wm-image-file-objects')[0],'files',{value:[new w.File(['image'],'test.png',{type:'image/png'})]});$('.wm-image-upload').trigger('click');assert.equal(request.data.get('action'),'upload_image');assert.equal(request.data.get('image_kind'),'objects');assert.equal(request.data.get('__csrf_magic'),'fixture');assert.equal($('.wm-image-upload').prop('disabled'),true);request.complete();assert.equal($('.wm-image-upload').prop('disabled'),false); +dom.window.close();console.log('PASS: image metadata, translations, protected delete, confirmation, request errors and upload wiring'); diff --git a/tests/Support/EditorImageFormatsRegression.php b/tests/Support/EditorImageFormatsRegression.php index c504718a..af528a6c 100644 --- a/tests/Support/EditorImageFormatsRegression.php +++ b/tests/Support/EditorImageFormatsRegression.php @@ -1,4 +1,26 @@ 'objects', 'image_name' => $deleteName]; - ob_start(); wm_editor_assets($config, 'delete_image'); $response = json_decode(ob_get_clean(), true); - image_format_assert(isset($response['error']) && is_file($deletePath), 'GET cannot delete an image'); - $_SERVER['REQUEST_METHOD'] = 'POST'; - file_put_contents($config, "ICON images/objects/" . $deleteName . "\n"); - ob_start(); wm_editor_assets($config, 'delete_image'); $response = json_decode(ob_get_clean(), true); - image_format_assert(isset($response['error']) && is_file($deletePath), 'referenced image cannot be deleted'); - file_put_contents($config, "# no image reference\n"); - ob_start(); wm_editor_assets($config, 'delete_image'); $response = json_decode(ob_get_clean(), true); - image_format_assert(isset($response['images']) && !is_file($deletePath), 'delete removes the actual server file'); + $included = $temporary . '/included.conf'; + file_put_contents($included, "BACKGROUND images/backgrounds/included-image.png\n"); + file_put_contents($config, "INCLUDE " . $included . "\n"); + image_format_assert(in_array(basename($included), wm_asset_image_users('included-image.png'), true), 'included configurations protect images from deletion'); + // Read-only listings, reference protection and the final server unlink. + $_SERVER['REQUEST_METHOD'] = 'GET'; + ob_start(); wm_editor_assets($config, 'image_data'); $response = json_decode(ob_get_clean(), true); + image_format_assert(isset($response['images']), 'library JSON returned'); + $deletePath = $created[0]; $deleteName = basename($deletePath); + $GLOBALS['__test_nfilter_request'] = ['image_kind' => 'objects', 'image_name' => $deleteName]; + ob_start(); wm_editor_assets($config, 'delete_image'); $response = json_decode(ob_get_clean(), true); + image_format_assert(isset($response['error']) && is_file($deletePath), 'GET cannot delete an image'); + $_SERVER['REQUEST_METHOD'] = 'POST'; + file_put_contents($config, "ICON images/objects/" . $deleteName . "\n"); + ob_start(); wm_editor_assets($config, 'delete_image'); $response = json_decode(ob_get_clean(), true); + image_format_assert(isset($response['error']) && is_file($deletePath), 'referenced image cannot be deleted'); + file_put_contents($config, "# no image reference\n"); + ob_start(); wm_editor_assets($config, 'delete_image'); $response = json_decode(ob_get_clean(), true); + image_format_assert(isset($response['images']) && !is_file($deletePath), 'delete removes the actual server file'); echo "Editor image formats regression PASS\n"; } finally { foreach ($created as $path) if (is_file($path)) unlink($path); diff --git a/tests/Unit/EditorImageFormatsTest.php b/tests/Unit/EditorImageFormatsTest.php index d3f7d225..4304d156 100644 --- a/tests/Unit/EditorImageFormatsTest.php +++ b/tests/Unit/EditorImageFormatsTest.php @@ -1,19 +1,41 @@ -
+
'>
-
Node Icons
-
-

-
Background Images
-
-

+
+
+

'>
+
+
+

'>
- -
PNG, JPG/JPEG or GIF files up to 5 MB and 4 million pixels. GIF uploads use the first frame. Uploads are saved immediately. Images referenced by a map configuration cannot be deleted.
+
+
From d74b3eb521a8cafb1583189fc7bcba10c522b3ec Mon Sep 17 00:00:00 2001 From: alcatron <27321443+alcatron@users.noreply.github.com> Date: Mon, 12 Oct 2026 00:56:05 +1030 Subject: [PATCH 04/41] Clean up incomplete image writes in the finalizer --- lib/editor.assets.php | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-) diff --git a/lib/editor.assets.php b/lib/editor.assets.php index 06943b8b..28783664 100644 --- a/lib/editor.assets.php +++ b/lib/editor.assets.php @@ -110,15 +110,17 @@ function wm_asset_store_image($kind, $source, $original_name) { $path = wm_asset_image_path($kind, $name); $handle = @fopen($path, 'x'); if (!$handle) { imagedestroy($image); throw new RuntimeException(__('Could not reserve an image filename.', 'weathermap')); } + $written = false; try { // Re-encode decoded pixels; never publish the original uploaded bytes. - $written = $size[2] === IMAGETYPE_JPEG ? $encode($image, $handle, 90) : $encode($image, $handle); - if (!$written) throw new RuntimeException(__('Could not save the image.', 'weathermap')); - if (!fflush($handle)) throw new RuntimeException(__('Could not save the image.', 'weathermap')); - chmod($path, 0644); - } catch (Throwable $error) { - unlink($path); - throw $error; - } finally { fclose($handle); imagedestroy($image); } + $encoded = $size[2] === IMAGETYPE_JPEG ? $encode($image, $handle, 90) : $encode($image, $handle); + $written = $encoded && fflush($handle); + } finally { + fclose($handle); + imagedestroy($image); + if (!$written) unlink($path); + } + if (!$written) throw new RuntimeException(__('Could not save the image.', 'weathermap')); + chmod($path, 0644); return $name; } From 4548ed4e73be8f4fc53c89e37487c83610165acb Mon Sep 17 00:00:00 2001 From: alcatron <27321443+alcatron@users.noreply.github.com> Date: Mon, 12 Oct 2026 01:30:27 +1030 Subject: [PATCH 05/41] Guard image library parents and reuse configuration reference indexes --- CHANGELOG.md | 2 + js/editor.js | 2 +- lib/editor.assets.php | 39 +++++++++++++------ lib/editor.inc.php | 3 +- locales/po/cacti.pot | 13 ++----- .../Support/EditorImageFormatsRegression.php | 23 +++++++++++ 6 files changed, 59 insertions(+), 23 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index d13427ee..22ae57b5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -16,6 +16,8 @@ IMPORTANT NOTE: This version only works on CACTI 1.x++! --- develop --- +* Add separate icon and background image upload and management libraries + * issue#299: Keep editor picker results open when browsing and allow changing an existing selection * feature#298: Fit or stretch background images within the map canvas diff --git a/js/editor.js b/js/editor.js index f260bf5e..3a04ecdf 100644 --- a/js/editor.js +++ b/js/editor.js @@ -1685,7 +1685,7 @@ function wmAssetImages(images) { }); ['objects', 'backgrounds'].forEach(function(kind) { var count = images.filter(function(item) { return item.kind === kind; }).length; - $('#wm-images-' + kind + '-count').text((count === 1 ? wmEditorText.imageText11 : wmEditorText.imageText12).replace('%s', String(count))); + $('#wm-images-' + kind + '-count').text(wmEditorText.imageText11.replace('%s', String(count))); if (!count) $('#wm-images-' + kind).append($('

').text(wmEditorText.imageText1)); }); // Rebuild the existing image selectors so uploads are available without reloading the editor. diff --git a/lib/editor.assets.php b/lib/editor.assets.php index 28783664..f1963858 100644 --- a/lib/editor.assets.php +++ b/lib/editor.assets.php @@ -25,7 +25,10 @@ /** Editor image library management. */ function wm_asset_directory($kind) { if (!in_array($kind, ['objects', 'backgrounds'], true)) throw new InvalidArgumentException(__('Invalid image category.', 'weathermap')); - return dirname(__DIR__) . '/images/' . $kind; + $parent = dirname(__DIR__) . '/images'; + $dir = $parent . '/' . $kind; + if (is_link($parent) || is_link($dir)) throw new InvalidArgumentException(__('Symbolic links cannot be managed here.', 'weathermap')); + return $dir; } function wm_asset_image_path($kind, $name) { $dir = wm_asset_directory($kind); @@ -34,8 +37,8 @@ function wm_asset_image_path($kind, $name) { if (is_link($dir) || is_link($path)) throw new InvalidArgumentException(__('Symbolic links cannot be managed here.', 'weathermap')); return $path; } -function wm_asset_image_users($name, $kind = null) { - $users = []; +function wm_asset_image_reference_index() { + $references = []; $queue = []; $root = dirname(__DIR__); $iterator = new RecursiveIteratorIterator(new RecursiveDirectoryIterator($root . '/configs', FilesystemIterator::SKIP_DOTS)); @@ -62,30 +65,43 @@ function wm_asset_image_users($name, $kind = null) { $image = '/' . implode('/', $parts); // Dynamic token values are unknown here: protect all possible paths. $pattern = implode('.*', array_map(function($part) { return preg_quote($part, '~'); }, preg_split('/\{[^}]+\}/', $image))); - foreach ($kind === null ? ['objects', 'backgrounds'] : [$kind] as $library) { - if (preg_match('~^' . $pattern . '$~D', $root . '/images/' . $library . '/' . $name)) { $users[] = basename($real); break 2; } - } + $references[] = ['pattern' => '~^' . $pattern . '$~D', 'user' => basename($real)]; } } if (preg_match_all('/^\s*INCLUDE\s+(.+)$/mi', $text, $matches)) foreach ($matches[1] as $include) { $include = trim($include); $include = strpos($include, '/') === 0 ? $include : $root . '/' . $include; $matches = glob($include); - if (!$matches) $queue[] = $include; - else foreach ($matches as $match) $queue[] = $match; + // The renderer tolerates absent optional includes; unreadable matches still fail closed. + foreach ($matches ?: [] as $match) $queue[] = $match; + } + } + return $references; +} +function wm_asset_image_users($name, $kind = null, $references = null) { + $references = $references ?? wm_asset_image_reference_index(); + $users = []; + $root = dirname(__DIR__); + foreach ($references as $reference) { + foreach ($kind === null ? ['objects', 'backgrounds'] : [$kind] as $library) { + if (preg_match($reference['pattern'], $root . '/images/' . $library . '/' . $name)) { + $users[] = $reference['user']; + break; + } } } - return $users; + return array_values(array_unique($users)); } function wm_asset_image_list() { $result = []; + $references = wm_asset_image_reference_index(); foreach (['objects', 'backgrounds'] as $kind) { $dir = wm_asset_directory($kind); foreach (scandir($dir) ?: [] as $name) { $path = $dir . '/' . $name; if (!is_file($path) || is_link($path) || !is_readable($path) || !preg_match('/\.(png|gif|jpe?g)$/iD', $name)) continue; $size = @getimagesize($path); - $users = wm_asset_image_users($name, $kind); + $users = wm_asset_image_users($name, $kind, $references); $result[] = ['width' => $size ? $size[0] : null, 'height' => $size ? $size[1] : null, 'users' => $users, 'kind' => $kind, 'name' => $name, 'url' => 'images/' . $kind . '/' . rawurlencode($name), 'deletable' => !$users && (bool) preg_match('/^[A-Za-z0-9_.-]+\.(png|gif|jpe?g)$/iD', $name) && strpos($name, '..') === false]; } } @@ -116,7 +132,8 @@ function wm_asset_store_image($kind, $source, $original_name) { $encoded = $size[2] === IMAGETYPE_JPEG ? $encode($image, $handle, 90) : $encode($image, $handle); $written = $encoded && fflush($handle); } finally { - fclose($handle); + $closed = fclose($handle); + $written = $written && $closed; imagedestroy($image); if (!$written) unlink($path); } diff --git a/lib/editor.inc.php b/lib/editor.inc.php index 26201ebd..843e6ad0 100644 --- a/lib/editor.inc.php +++ b/lib/editor.inc.php @@ -1023,8 +1023,7 @@ function getEditorJs() { 'imageText8' => __('Remove range', 'weathermap'), 'imageText9' => __('Delete %s', '%s', 'weathermap'), 'imageText10' => __('Delete image "%s" from the server?', '%s', 'weathermap'), - 'imageText11' => __('%s image', '%s', 'weathermap'), - 'imageText12' => __('%s images', '%s', 'weathermap'), + 'imageText11' => __('Images: %s', '%s', 'weathermap'), 'imageText13' => __('Uploaded %s', '%s', 'weathermap'), 'imageText14' => __('Label Only', 'weathermap'), 'imageText15' => __('No background', 'weathermap'), diff --git a/locales/po/cacti.pot b/locales/po/cacti.pot index 3df11429..070b8c88 100644 --- a/locales/po/cacti.pot +++ b/locales/po/cacti.pot @@ -8,7 +8,7 @@ msgid "" msgstr "" "Project-Id-Version: Cacti 1.7\n" "Report-Msgid-Bugs-To: developers@cacti.net\n" -"POT-Creation-Date: 2026-10-12 00:51+1030\n" +"POT-Creation-Date: 2026-10-12 01:28+1030\n" "PO-Revision-Date: YEAR-MO-DA HO:MI+ZONE\n" "Last-Translator: Cacti Developers \n" "Language-Team: Cacti Developers \n" @@ -39,11 +39,11 @@ msgid "Invalid image category." msgstr "" #: lib/editor.assets.php -msgid "Invalid image filename." +msgid "Symbolic links cannot be managed here." msgstr "" #: lib/editor.assets.php -msgid "Symbolic links cannot be managed here." +msgid "Invalid image filename." msgstr "" #: lib/editor.assets.php @@ -276,12 +276,7 @@ msgstr "" #: lib/editor.inc.php #, php-format -msgid "%s image" -msgstr "" - -#: lib/editor.inc.php -#, php-format -msgid "%s images" +msgid "Images: %s" msgstr "" #: lib/editor.inc.php diff --git a/tests/Support/EditorImageFormatsRegression.php b/tests/Support/EditorImageFormatsRegression.php index af528a6c..5eb1c758 100644 --- a/tests/Support/EditorImageFormatsRegression.php +++ b/tests/Support/EditorImageFormatsRegression.php @@ -78,6 +78,29 @@ function image_format_assert($value, $message) { if (!$value) throw new RuntimeE file_put_contents($included, "BACKGROUND images/backgrounds/included-image.png\n"); file_put_contents($config, "INCLUDE " . $included . "\n"); image_format_assert(in_array(basename($included), wm_asset_image_users('included-image.png'), true), 'included configurations protect images from deletion'); + // Missing optional includes must not disable the library, and one index serves all names. + file_put_contents($config, "INCLUDE " . $temporary . "/missing-*.conf\nINCLUDE " . $included . "\n"); + $index = wm_asset_image_reference_index(); + file_put_contents($included, "# changed after indexing\n"); + image_format_assert(in_array(basename($included), wm_asset_image_users('included-image.png', 'backgrounds', $index), true), 'reference snapshot reused without rescanning configurations'); + image_format_assert(wm_asset_image_users('included-image.png', 'backgrounds') === [], 'fresh request reflects changed configuration'); + image_format_assert(is_array(wm_asset_image_list()), 'unmatched optional include permits listing'); + // Test symlink rejection only inside this isolated fixture, and always restore its library. + $library = $root . '/images'; + $originalLibrary = $temporary . '/library'; + image_format_assert(rename($library, $originalLibrary), 'reserve fixture library'); + try { + image_format_assert(symlink($originalLibrary, $library), 'create linked fixture library'); + foreach (['objects', 'backgrounds'] as $kind) { + try { wm_asset_directory($kind); throw new RuntimeException('accepted symlinked library parent'); } catch (InvalidArgumentException $expected) {} + } + try { wm_asset_image_list(); throw new RuntimeException('listed linked library'); } catch (InvalidArgumentException $expected) {} + try { wm_asset_store_image('objects', $source, 'linked.png'); throw new RuntimeException('uploaded to linked library'); } catch (InvalidArgumentException $expected) {} + try { wm_asset_image_path('objects', basename($created[0])); throw new RuntimeException('accepted deletion path in linked library'); } catch (InvalidArgumentException $expected) {} + } finally { + if (is_link($library)) unlink($library); + rename($originalLibrary, $library); + } // Read-only listings, reference protection and the final server unlink. $_SERVER['REQUEST_METHOD'] = 'GET'; ob_start(); wm_editor_assets($config, 'image_data'); $response = json_decode(ob_get_clean(), true); From 1d14e0c98b4c154a81994a3dec6c921acce90939 Mon Sep 17 00:00:00 2001 From: alcatron <27321443+alcatron@users.noreply.github.com> Date: Mon, 12 Oct 2026 01:33:38 +1030 Subject: [PATCH 06/41] Reuse the image reference snapshot after deletion --- lib/editor.actions.php | 5 +++-- lib/editor.assets.php | 4 ++-- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/lib/editor.actions.php b/lib/editor.actions.php index 7f4a0b61..3463ecda 100644 --- a/lib/editor.actions.php +++ b/lib/editor.actions.php @@ -1152,9 +1152,10 @@ function wm_editor_assets($mapfile, $action) { } else { $name = get_nfilter_request_var('image_name'); $path = wm_asset_image_path(get_nfilter_request_var('image_kind'), $name); - if (wm_asset_image_users($name, get_nfilter_request_var('image_kind'))) throw new InvalidArgumentException(__('This image is still referenced by a map configuration.', 'weathermap')); + $references = wm_asset_image_reference_index(); + if (wm_asset_image_users($name, get_nfilter_request_var('image_kind'), $references)) throw new InvalidArgumentException(__('This image is still referenced by a map configuration.', 'weathermap')); if (!is_file($path) || !unlink($path)) throw new RuntimeException(__('Could not delete the image.', 'weathermap')); - $result = ['images' => wm_asset_image_list()]; + $result = ['images' => wm_asset_image_list($references)]; } echo json_encode($result); } catch (Throwable $error) { diff --git a/lib/editor.assets.php b/lib/editor.assets.php index f1963858..66abbce1 100644 --- a/lib/editor.assets.php +++ b/lib/editor.assets.php @@ -92,9 +92,9 @@ function wm_asset_image_users($name, $kind = null, $references = null) { } return array_values(array_unique($users)); } -function wm_asset_image_list() { +function wm_asset_image_list($references = null) { $result = []; - $references = wm_asset_image_reference_index(); + $references = $references ?? wm_asset_image_reference_index(); foreach (['objects', 'backgrounds'] as $kind) { $dir = wm_asset_directory($kind); foreach (scandir($dir) ?: [] as $name) { From b1f1e2a2b2f75b1aa1d30452f9c31936292aad00 Mon Sep 17 00:00:00 2001 From: alcatron <27321443+alcatron@users.noreply.github.com> Date: Mon, 12 Oct 2026 09:08:24 +1030 Subject: [PATCH 07/41] Validate references before persisting image uploads --- lib/editor.actions.php | 19 +++++++- lib/editor.assets.php | 47 ++++++++++++++++++- locales/po/cacti.pot | 2 +- .../Support/EditorImageFormatsRegression.php | 12 +++++ 4 files changed, 77 insertions(+), 3 deletions(-) diff --git a/lib/editor.actions.php b/lib/editor.actions.php index 3463ecda..73e55656 100644 --- a/lib/editor.actions.php +++ b/lib/editor.actions.php @@ -1136,10 +1136,26 @@ function getImageURL($mapname, $selected) { return $imageurl; } +/** + * Validate a PHP upload and store its decoded image. + * + * @param string $kind The destination library category. + * @param array $file The PHP upload metadata from $_FILES. + * + * @return string The stored image filename. + */ function wm_asset_upload_image($kind, $file) { if (($file['error'] ?? UPLOAD_ERR_NO_FILE) !== UPLOAD_ERR_OK || !is_uploaded_file($file['tmp_name'] ?? '')) throw new InvalidArgumentException(__('Choose a PNG, JPG or GIF file up to 5 MB.', 'weathermap')); return wm_asset_store_image($kind, $file['tmp_name'], $file['name']); } +/** + * Read or mutate an image library and emit a JSON response. + * + * @param string $mapfile The current validated map path. + * @param string $action The library operation requested by the editor. + * + * @return void Errors are returned as HTTP 400 JSON responses. + */ function wm_editor_assets($mapfile, $action) { header('Content-Type: application/json'); try { @@ -1147,8 +1163,9 @@ function wm_editor_assets($mapfile, $action) { if ($action === 'image_data') { $result = ['images' => wm_asset_image_list()]; } elseif ($action === 'upload_image') { + $references = wm_asset_image_reference_index(); $name = wm_asset_upload_image(get_nfilter_request_var('image_kind'), $_FILES['image_file'] ?? []); - $result = ['name' => $name, 'images' => wm_asset_image_list()]; + $result = ['name' => $name, 'images' => wm_asset_image_list($references)]; } else { $name = get_nfilter_request_var('image_name'); $path = wm_asset_image_path(get_nfilter_request_var('image_kind'), $name); diff --git a/lib/editor.assets.php b/lib/editor.assets.php index 66abbce1..1b33f802 100644 --- a/lib/editor.assets.php +++ b/lib/editor.assets.php @@ -22,7 +22,13 @@ +-------------------------------------------------------------------------+ */ -/** Editor image library management. */ +/** + * Resolve a managed image library, rejecting symlinked directories. + * + * @param string $kind The objects or backgrounds library. + * + * @return string The library directory. + */ function wm_asset_directory($kind) { if (!in_array($kind, ['objects', 'backgrounds'], true)) throw new InvalidArgumentException(__('Invalid image category.', 'weathermap')); $parent = dirname(__DIR__) . '/images'; @@ -30,6 +36,14 @@ function wm_asset_directory($kind) { if (is_link($parent) || is_link($dir)) throw new InvalidArgumentException(__('Symbolic links cannot be managed here.', 'weathermap')); return $dir; } +/** + * Validate a filename and resolve its non-symlinked library path. + * + * @param string $kind The library category. + * @param mixed $name The filename to validate. + * + * @return string The validated image path. + */ function wm_asset_image_path($kind, $name) { $dir = wm_asset_directory($kind); if (!is_string($name) || !preg_match('/^[A-Za-z0-9_.-]+\.(png|gif|jpe?g)$/iD', $name) || strpos($name, '..') !== false) throw new InvalidArgumentException(__('Invalid image filename.', 'weathermap')); @@ -37,6 +51,12 @@ function wm_asset_image_path($kind, $name) { if (is_link($dir) || is_link($path)) throw new InvalidArgumentException(__('Symbolic links cannot be managed here.', 'weathermap')); return $path; } +/** + * Read configuration files and includes once to index image references. + * + * + * @return array The reference patterns and source filenames. + */ function wm_asset_image_reference_index() { $references = []; $queue = []; @@ -78,6 +98,15 @@ function wm_asset_image_reference_index() { } return $references; } +/** + * Find map configurations referencing an image. + * + * @param string $name The image filename. + * @param string|null $kind One library category, or both when null. + * @param array|null $references A request-local reference index; null builds an index. + * + * @return string[] Unique configuration filenames using the image. + */ function wm_asset_image_users($name, $kind = null, $references = null) { $references = $references ?? wm_asset_image_reference_index(); $users = []; @@ -92,6 +121,13 @@ function wm_asset_image_users($name, $kind = null, $references = null) { } return array_values(array_unique($users)); } +/** + * List image metadata using one shared configuration reference index. + * + * @param array|null $references A precomputed index, or null to read configurations. + * + * @return array The image dimensions, usage and deletion permissions. + */ function wm_asset_image_list($references = null) { $result = []; $references = $references ?? wm_asset_image_reference_index(); @@ -107,6 +143,15 @@ function wm_asset_image_list($references = null) { } return $result; } +/** + * Validate and re-encode an upload under an exclusively reserved filename. + * + * @param string $kind The destination library category. + * @param string $source The temporary source image path. + * @param string $original_name The original name used to derive a safe filename. + * + * @return string The stored filename; invalid images or write failures raise exceptions. + */ function wm_asset_store_image($kind, $source, $original_name) { $dir = wm_asset_directory($kind); if (is_link($dir) || !is_writable($dir)) throw new RuntimeException(__('The image directory is not writable.', 'weathermap')); diff --git a/locales/po/cacti.pot b/locales/po/cacti.pot index 070b8c88..df8dac5e 100644 --- a/locales/po/cacti.pot +++ b/locales/po/cacti.pot @@ -8,7 +8,7 @@ msgid "" msgstr "" "Project-Id-Version: Cacti 1.7\n" "Report-Msgid-Bugs-To: developers@cacti.net\n" -"POT-Creation-Date: 2026-10-12 01:28+1030\n" +"POT-Creation-Date: 2026-10-12 09:02+1030\n" "PO-Revision-Date: YEAR-MO-DA HO:MI+ZONE\n" "Last-Translator: Cacti Developers \n" "Language-Team: Cacti Developers \n" diff --git a/tests/Support/EditorImageFormatsRegression.php b/tests/Support/EditorImageFormatsRegression.php index 5eb1c758..46f5bb32 100644 --- a/tests/Support/EditorImageFormatsRegression.php +++ b/tests/Support/EditorImageFormatsRegression.php @@ -101,6 +101,18 @@ function image_format_assert($value, $message) { if (!$value) throw new RuntimeE if (is_link($library)) unlink($library); rename($originalLibrary, $library); } + // A reference-scan failure must be reported before upload validation or persistence. + $broken = $temporary . '/broken.conf'; + symlink($temporary . '/missing-target.conf', $broken); + file_put_contents($config, "INCLUDE " . $broken . "\n"); + $_SERVER['REQUEST_METHOD'] = 'POST'; + $_FILES['image_file'] = ['error' => UPLOAD_ERR_OK, 'tmp_name' => $source, 'name' => 'must-not-persist.png']; + $GLOBALS['__test_nfilter_request'] = ['image_kind' => 'objects']; + ob_start(); wm_editor_assets($config, 'upload_image'); $response = json_decode(ob_get_clean(), true); + image_format_assert(str_contains($response['error'], 'Cannot check configuration references'), 'configuration failure precedes upload persistence'); + image_format_assert(glob($root . '/images/objects/must-not-persist-*') === [], 'reference error cannot create an uploaded image'); + unlink($broken); unset($_FILES['image_file']); + file_put_contents($config, "# no image references\n"); // Read-only listings, reference protection and the final server unlink. $_SERVER['REQUEST_METHOD'] = 'GET'; ob_start(); wm_editor_assets($config, 'image_data'); $response = json_decode(ob_get_clean(), true); From 7db90e4d3b903d0df8c9a34ebe34ab723ef98f36 Mon Sep 17 00:00:00 2001 From: alcatron <27321443+alcatron@users.noreply.github.com> Date: Mon, 12 Oct 2026 11:09:56 +1030 Subject: [PATCH 08/41] Synchronize image deletion with portable atomic configuration saves --- js/editor.js | 2 +- lib/WeatherMap.class.php | 53 +++-- lib/WeatherMap.config.php | 107 ++++++++++ lib/editor.actions.php | 13 +- lib/editor.assets.php | 46 +++-- lib/editor.inc.php | 2 +- locales/po/cacti.pot | 14 +- tests/Browser/EditorImagesTest.cjs | 2 + tests/Support/ConfigWriteAtomicRegression.php | 193 ++++++++++++++++++ .../Support/EditorImageFormatsRegression.php | 25 ++- tests/Unit/ConfigWriteAtomicTest.php | 41 ++++ weathermap-cacti-plugin-mgmt.php | 21 +- 12 files changed, 453 insertions(+), 66 deletions(-) create mode 100644 lib/WeatherMap.config.php create mode 100644 tests/Support/ConfigWriteAtomicRegression.php create mode 100644 tests/Unit/ConfigWriteAtomicTest.php diff --git a/js/editor.js b/js/editor.js index 3a04ecdf..8fa055a8 100644 --- a/js/editor.js +++ b/js/editor.js @@ -1693,7 +1693,7 @@ function wmAssetImages(images) { var select = $('#' + spec.id), old = select.val(); var slick = select.data('ddslick'); if (slick) { old = slick.selectedData && slick.selectedData.value; select.ddslick('destroy'); select = $('#' + spec.id); } - select.find('option').filter(function() { return /^images\/.*\.(png|gif|jpe?g)$/i.test(this.value); }).remove(); + select.find('option').filter(function() { return this.value.indexOf('images/' + spec.kind + '/') === 0 && /\.(png|gif|jpe?g)$/i.test(this.value); }).remove(); if (!select.find('option[value="--NONE--"]').length) select.prepend($('

' . __('Path mismatch', 'weathermap') . '

'; } else { require_once __DIR__ . '/lib/editor.config-delete.php'; - $file = basename($file); - $realfile = $weathermap_confdir . '/' . $file; - $lock = wm_config_lock($realfile); - if ($lock === false) { + require_once __DIR__ . '/lib/WeatherMap.config.php'; + $directory_lock = \WeathermapConfig\acquire($weathermap_confdir, false); + if (!$directory_lock) { raise_message('path_missing', __esc('The configuration file is unavailable. Please retry.', 'weathermap'), MESSAGE_LEVEL_ERROR); return; } try { - $title = wmap_get_title($realfile); - - db_execute_prepared("INSERT INTO weathermap_maps - (configfile, titlecache, active, imagefile, htmlfile, filehash, config) - VALUES (?, ?, 'on', '', '', '', '')", - [$file, $title]); - - $last_id = db_fetch_insert_id(); - $myuid = (isset($_SESSION['sess_user_id']) ? intval($_SESSION['sess_user_id']) : 1); - - db_execute_prepared('INSERT INTO weathermap_auth - (mapid, userid) - VALUES (?, ?)', - [$last_id, $myuid]); - - db_execute_prepared('UPDATE weathermap_maps - SET filehash = LEFT(MD5(CONCAT(id, configfile, rand())), 20) - WHERE id = ?', - [$last_id]); - - map_resort(); + $file = basename($file); + $realfile = $weathermap_confdir . '/' . $file; + $lock = wm_config_lock($realfile); + if ($lock === false) { + raise_message('path_missing', __esc('The configuration file is unavailable. Please retry.', 'weathermap'), MESSAGE_LEVEL_ERROR); + return; + } + try { + $title = wmap_get_title($realfile); + + db_execute_prepared("INSERT INTO weathermap_maps + (configfile, titlecache, active, imagefile, htmlfile, filehash, config) + VALUES (?, ?, 'on', '', '', '', '')", + [$file, $title]); + + $last_id = db_fetch_insert_id(); + $myuid = (isset($_SESSION['sess_user_id']) ? intval($_SESSION['sess_user_id']) : 1); + + db_execute_prepared('INSERT INTO weathermap_auth + (mapid, userid) + VALUES (?, ?)', + [$last_id, $myuid]); + + db_execute_prepared('UPDATE weathermap_maps + SET filehash = LEFT(MD5(CONCAT(id, configfile, rand())), 20) + WHERE id = ?', + [$last_id]); + + map_resort(); + } finally { + flock($lock, LOCK_UN); + fclose($lock); + } } finally { - flock($lock, LOCK_UN); - fclose($lock); + \WeathermapConfig\release($directory_lock); } } } From 17a85466d6f4818da386a641b6bf7b3782818d3a Mon Sep 17 00:00:00 2001 From: alcatron <27321443+alcatron@users.noreply.github.com> Date: Mon, 12 Oct 2026 12:25:40 +1030 Subject: [PATCH 13/41] Return a retry when configuration deletion encounters a busy directory --- lib/editor.config-delete.php | 2 +- tests/Support/ConfigWriteAtomicRegression.php | 10 +++++----- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/lib/editor.config-delete.php b/lib/editor.config-delete.php index 9b3b529a..9d445604 100644 --- a/lib/editor.config-delete.php +++ b/lib/editor.config-delete.php @@ -91,7 +91,7 @@ function wm_config_delete($directory, $file) { return 'invalid'; } require_once __DIR__ . '/WeatherMap.config.php'; - $directory_lock = \WeathermapConfig\acquire($directory); + $directory_lock = \WeathermapConfig\acquire($directory, false); if (!$directory_lock) return 'failed'; try { $path = $directory . '/' . $file; diff --git a/tests/Support/ConfigWriteAtomicRegression.php b/tests/Support/ConfigWriteAtomicRegression.php index 75b7989b..0fe0d0e2 100644 --- a/tests/Support/ConfigWriteAtomicRegression.php +++ b/tests/Support/ConfigWriteAtomicRegression.php @@ -176,7 +176,7 @@ function atomic_assert($condition, $message) { pcntl_waitpid($child, $status); \WeathermapConfig\release($lock); atomic_assert(pcntl_wexitstatus($status) === 0, 'shared lock excludes concurrent configuration writers'); } - // Deletion waits for the same stable directory mutex used by atomic replacement. + // Deletion refuses a busy stable directory mutex rather than blocking the UI. if (function_exists('pcntl_fork')) { require_once dirname(__DIR__, 2) . '/lib/editor.config-delete.php'; $GLOBALS['__test_db_fetch_cell_prepared'] = fn () => 0; @@ -189,14 +189,14 @@ function atomic_assert($condition, $message) { file_put_contents($ready, 'ready'); $result = wm_config_delete($dir, 'map.conf'); file_put_contents($done, $result); - exit($result === 'deleted' ? 0 : 1); + exit($result === 'failed' ? 0 : 1); } - for ($wait = 0; $wait < 100 && !is_file($ready); $wait++) usleep(10000); - usleep(50000); - $blocked = !is_file($done) && is_file($file); + for ($wait = 0; $wait < 100 && !is_file($done); $wait++) usleep(10000); + $blocked = is_file($done) && file_get_contents($done) === 'failed' && is_file($file); \WeathermapConfig\release($lock); pcntl_waitpid($child, $status); atomic_assert($blocked && pcntl_wexitstatus($status) === 0, 'config deletion cannot unlink during a directory-locked write'); + atomic_assert(wm_config_delete($dir, 'map.conf') === 'deleted', 'deletion succeeds after the writer releases its lock'); unlink($ready); unlink($done); unset($GLOBALS['__test_db_fetch_cell_prepared'], $GLOBALS['__test_db_fetch_assoc_prepared']); file_put_contents($file, $original); From 04a44cd1a5366c3b350fc9ae7ed19c583a420e9b Mon Sep 17 00:00:00 2001 From: alcatron <27321443+alcatron@users.noreply.github.com> Date: Mon, 12 Oct 2026 13:06:27 +1030 Subject: [PATCH 14/41] Surface editor save failures and isolate upgrade regression stubs --- CHANGELOG.md | 2 + js/editor.js | 1 + lib/editor.actions.php | 138 ++++++++++++------ lib/editor.inc.php | 1 + locales/po/cacti.pot | 6 +- tests/Browser/EditorSaveFailureTest.cjs | 6 + tests/Support/ConfigWriteAtomicRegression.php | 10 +- tests/Unit/WeathermapUpgradeTest.php | 115 +++++++-------- weathermap-cacti-plugin-editor.php | 2 +- 9 files changed, 171 insertions(+), 110 deletions(-) create mode 100644 tests/Browser/EditorSaveFailureTest.cjs diff --git a/CHANGELOG.md b/CHANGELOG.md index 90352cc0..2e1a8a63 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -16,6 +16,8 @@ IMPORTANT NOTE: This version only works on CACTI 1.x++! --- develop --- +* Make configuration saves and repairs atomic; coordinate writes, registration and deletion with destination-directory locks and report failed editor saves + * Add separate icon and background image upload and management libraries * Reduce gallery preview cards by 20% while preserving generated thumbnail resolution * Add a searchable node icon picker and bundled enterprise site icons diff --git a/js/editor.js b/js/editor.js index 41ccd72d..6e5a9bbc 100644 --- a/js/editor.js +++ b/js/editor.js @@ -878,6 +878,7 @@ function form_submit() { type: 'POST', url: editor_url, data: data, + error: function() { window.alert(wmEditorText.saveError); }, success: function(html) { hide_all_dialogs(); diff --git a/lib/editor.actions.php b/lib/editor.actions.php index aab93135..9c89ff5c 100644 --- a/lib/editor.actions.php +++ b/lib/editor.actions.php @@ -48,6 +48,40 @@ * @param mixed $mapfile */ +/** + * Read an existing editor map and reject stale or unreadable targets. + * + * @param WeatherMap $map Engine receiving the configuration. + * @param string $mapfile Configuration path. + * @return bool Whether the map was loaded. + */ +function wm_editor_read_config($map, $mapfile) { + if (!is_file($mapfile) || !is_readable($mapfile)) { + http_response_code(404); + return false; + } + if (!$map->ReadConfig($mapfile)) { + http_response_code(422); + return false; + } + return true; +} + +/** + * Propagate failed editor configuration saves to the AJAX caller. + * + * @param WeatherMap $map Engine containing the edited configuration. + * @param string $mapfile Configuration path. + * @return bool Whether the complete configuration was saved. + */ +function wm_editor_write_config($map, $mapfile) { + if (!$map->WriteConfig($mapfile)) { + http_response_code(500); + return false; + } + return true; +} + /** * @return void * @param mixed $mapfile @@ -59,7 +93,7 @@ function newMap($mapfile) { require_once __DIR__ . '/editor.new-map-preset.php'; wm_new_map_preset($map); - $map->WriteConfig($mapfile); + wm_editor_write_config($map, $mapfile); } /** @@ -86,7 +120,7 @@ function newMapCopy($mapfile) { if (file_exists($sourcemap) && is_readable($sourcemap)) { $map->ReadConfig($sourcemap); - $map->WriteConfig($mapfile); + wm_editor_write_config($map, $mapfile); } } } @@ -100,7 +134,7 @@ function getMapJavaScript($mapfile) { $map->context = 'editor'; - $map->ReadConfig($mapfile); + if (!wm_editor_read_config($map, $mapfile)) return; print $map->asJS(); } @@ -114,7 +148,7 @@ function getMapAreaData($mapfile) { $map->context = 'editor'; - $map->ReadConfig($mapfile); + if (!wm_editor_read_config($map, $mapfile)) return; // we need to draw and throw away a map, to get the // dimensions for the imagemap. @@ -141,7 +175,7 @@ function drawMap($mapfile, $selected, $use_overlay, $use_relative_overlay) { $map->context = 'editor'; - $map->ReadConfig($mapfile); + if (!wm_editor_read_config($map, $mapfile)) return; if ($selected != '') { if (substr($selected, 0, 5) == 'NODE:') { @@ -186,7 +220,7 @@ function fetchConfig($mapfile) { $map->context = 'editor'; - $map->ReadConfig($mapfile); + if (!wm_editor_read_config($map, $mapfile)) return; header('Content-type: text/plain'); @@ -223,7 +257,7 @@ function setNodeConfig($mapfile) { $map->context = 'editor'; - $map->ReadConfig($mapfile); + if (!wm_editor_read_config($map, $mapfile)) return; $node_name = get_nfilter_request_var('node_name'); $node_config = get_nfilter_request_var('item_configtext'); @@ -231,7 +265,7 @@ function setNodeConfig($mapfile) { if (isset($map->nodes[$node_name])) { $map->nodes[$node_name]->config_override = $node_config; - $map->WriteConfig($mapfile); + wm_editor_write_config($map, $mapfile); // now clear and reload the map object, because the in-memory one is out of sync // - we don't know what changes the user made here, so we just have to reload. @@ -241,7 +275,7 @@ function setNodeConfig($mapfile) { $map->context = 'editor'; - $map->ReadConfig($mapfile); + if (!wm_editor_read_config($map, $mapfile)) return; } } @@ -254,7 +288,7 @@ function setLinkConfig($mapfile) { $map->context = 'editor'; - $map->ReadConfig($mapfile); + if (!wm_editor_read_config($map, $mapfile)) return; $link_name = get_nfilter_request_var('link_name'); $link_config = get_nfilter_request_var('item_configtext'); @@ -262,7 +296,7 @@ function setLinkConfig($mapfile) { if (isset($map->links[$link_name])) { $map->links[$link_name]->config_override = $link_config; - $map->WriteConfig($mapfile); + wm_editor_write_config($map, $mapfile); // now clear and reload the map object, because the in-memory one is out of sync // - we don't know what changes the user made here, so we just have to reload. @@ -272,7 +306,7 @@ function setLinkConfig($mapfile) { $map->context = 'editor'; - $map->ReadConfig($mapfile); + if (!wm_editor_read_config($map, $mapfile)) return; } } @@ -285,7 +319,7 @@ function setNodeProperties($mapfile) { $map->context = 'editor'; - $map->ReadConfig($mapfile); + if (!wm_editor_read_config($map, $mapfile)) return; $node_name = get_nfilter_request_var('node_name'); $new_node_name = get_nfilter_request_var('node_new_name'); @@ -362,7 +396,7 @@ function setNodeProperties($mapfile) { $map->nodes[$new_node_name]->iconfile = $iconfile; } - $map->WriteConfig($mapfile); + wm_editor_write_config($map, $mapfile); } /** @@ -374,7 +408,7 @@ function setLinkProperties($mapfile) { $map->context = 'editor'; - $map->ReadConfig($mapfile); + if (!wm_editor_read_config($map, $mapfile)) return; $link_name = get_nfilter_request_var('link_name'); @@ -442,7 +476,7 @@ function setLinkProperties($mapfile) { // $map->links[$link_name]->SetBandwidth($bwin,$bwout); - $map->WriteConfig($mapfile); + wm_editor_write_config($map, $mapfile); } } @@ -455,7 +489,7 @@ function setMapProperties($mapfile) { $map->context = 'editor'; - $map->ReadConfig($mapfile); + if (!wm_editor_read_config($map, $mapfile)) return; $map->title = wm_editor_sanitize_string(get_nfilter_request_var('map_title')); $map->keytext['DEFAULT'] = wm_editor_sanitize_string(get_nfilter_request_var('map_legend')); @@ -528,7 +562,7 @@ function setMapProperties($mapfile) { } } - $map->WriteConfig($mapfile); + wm_editor_write_config($map, $mapfile); } /** @@ -540,7 +574,7 @@ function setMapStyle($mapfile) { $map->context = 'editor'; - $map->ReadConfig($mapfile); + if (!wm_editor_read_config($map, $mapfile)) return; if (wm_editor_validate_one_of(get_nfilter_request_var('mapstyle_htmlstyle'), ['static', 'overlib'], false)) { $map->htmlstyle = strtolower(get_nfilter_request_var('mapstyle_htmlstyle')); @@ -565,7 +599,7 @@ function setMapStyle($mapfile) { wm_comment_update_default($map, 'bwfont', wm_comment_resolve_font($map, get_nfilter_request_var('mapstyle_linkfont'))); wm_comment_apply_style($map, get_nfilter_request_var('mapstyle_commentfont'), get_nfilter_request_var('mapstyle_commentcolour')); - $map->WriteConfig($mapfile); + wm_editor_write_config($map, $mapfile); } /** @@ -577,7 +611,7 @@ function addLink($mapfile) { $map->context = 'editor'; - $map->ReadConfig($mapfile); + if (!wm_editor_read_config($map, $mapfile)) return; $a = get_nfilter_request_var('param'); $b = get_nfilter_request_var('param2'); @@ -609,7 +643,7 @@ function addLink($mapfile) { $map->links[$newlinkname] = $newlink; array_push($map->seen_zlayers[$newlink->zorder], $newlink); - $map->WriteConfig($mapfile); + wm_editor_write_config($map, $mapfile); } } @@ -628,12 +662,12 @@ function placeLegend($mapfile, $grid_snap_value) { $scalename = wm_editor_sanitize_name(get_nfilter_request_var('param')); - $map->ReadConfig($mapfile); + if (!wm_editor_read_config($map, $mapfile)) return; $map->keyx[$scalename] = $x; $map->keyy[$scalename] = $y; - $map->WriteConfig($mapfile); + wm_editor_write_config($map, $mapfile); } /** @@ -649,12 +683,12 @@ function placeStamp($mapfile, $grid_snap_value) { $x = snap(intval(get_nfilter_request_var('x')), $grid_snap_value); $y = snap(intval(get_nfilter_request_var('y')), $grid_snap_value); - $map->ReadConfig($mapfile); + if (!wm_editor_read_config($map, $mapfile)) return; $map->timex = $x; $map->timey = $y; - $map->WriteConfig($mapfile); + wm_editor_write_config($map, $mapfile); } /** @@ -671,11 +705,11 @@ function viaLink($mapfile) { $link_name = wm_editor_sanitize_name(get_nfilter_request_var('link_name')); - $map->ReadConfig($mapfile); + if (!wm_editor_read_config($map, $mapfile)) return; if (isset($map->links[$link_name])) { $map->links[$link_name]->vialist = [[0 =>$x, 1=>$y]]; - $map->WriteConfig($mapfile); + wm_editor_write_config($map, $mapfile); } } @@ -694,8 +728,12 @@ function moveNode($mapfile, $grid_snap_value) { $node_name = wm_editor_sanitize_name(get_nfilter_request_var('node_name')); - $map->ReadConfig($mapfile); + if (!wm_editor_read_config($map, $mapfile)) return; + if (!isset($map->nodes[$node_name])) { + http_response_code(404); + return; + } if (isset($map->nodes[$node_name])) { // This is a complicated bit. Find out if this node is involved in any // links that have VIAs. If it is, we want to rotate those VIA points @@ -788,7 +826,7 @@ function moveNode($mapfile, $grid_snap_value) { $map->nodes[$node_name]->x = $x; $map->nodes[$node_name]->y = $y; - $map->WriteConfig($mapfile); + wm_editor_write_config($map, $mapfile); } } @@ -801,7 +839,7 @@ function linkTidy($mapfile) { $map->context = 'editor'; - $map->ReadConfig($mapfile); + if (!wm_editor_read_config($map, $mapfile)) return; $target = wm_editor_sanitize_name(get_nfilter_request_var('param')); @@ -811,7 +849,7 @@ function linkTidy($mapfile) { tidy_link($map, $target); - $map->WriteConfig($mapfile); + wm_editor_write_config($map, $mapfile); } } @@ -824,13 +862,13 @@ function reTidy($mapfile) { $map->context = 'editor'; - $map->ReadConfig($mapfile); + if (!wm_editor_read_config($map, $mapfile)) return; // draw a map and throw it away, to calculate all the bounding boxes $map->DrawMap('null'); retidy_links($map); - $map->WriteConfig($mapfile); + wm_editor_write_config($map, $mapfile); } /** @@ -842,13 +880,13 @@ function reTidyAll($mapfile) { $map->context = 'editor'; - $map->ReadConfig($mapfile); + if (!wm_editor_read_config($map, $mapfile)) return; // draw a map and throw it away, to calculate all the bounding boxes $map->DrawMap('null'); retidy_links($map,true); - $map->WriteConfig($mapfile); + wm_editor_write_config($map, $mapfile); } /** @@ -860,13 +898,13 @@ function unTidy($mapfile) { $map->context = 'editor'; - $map->ReadConfig($mapfile); + if (!wm_editor_read_config($map, $mapfile)) return; // draw a map and throw it away, to calculate all the bounding boxes $map->DrawMap('null'); untidy_links($map); - $map->WriteConfig($mapfile); + wm_editor_write_config($map, $mapfile); } /** @@ -878,7 +916,7 @@ function deleteLink($mapfile) { $map->context = 'editor'; - $map->ReadConfig($mapfile); + if (!wm_editor_read_config($map, $mapfile)) return; $target = wm_editor_sanitize_name(get_nfilter_request_var('param')); $log = 'delete link ' . $target; @@ -886,7 +924,7 @@ function deleteLink($mapfile) { if (isset($map->links[$target])) { unset($map->links[$target]); - $map->WriteConfig($mapfile); + wm_editor_write_config($map, $mapfile); } } @@ -903,7 +941,7 @@ function addNode($mapfile, $grid_snap_value) { $x = snap(intval(get_nfilter_request_var('x')), $grid_snap_value); $y = snap(intval(get_nfilter_request_var('y')), $grid_snap_value); - $map->ReadConfig($mapfile); + if (!wm_editor_read_config($map, $mapfile)) return; $newnodename = sprintf('node%05d', time() % 10000); @@ -932,7 +970,7 @@ function addNode($mapfile, $grid_snap_value) { $map->nodes[$node->name] = $node; $log = "added a node called $newnodename at $x,$y to $mapfile"; - $map->WriteConfig($mapfile); + wm_editor_write_config($map, $mapfile); } /** @@ -947,7 +985,7 @@ function editorSettings($mapfile) { $map->context = 'editor'; // have to do this, otherwise the editor will be unresponsive afterwards - not actually going to change anything! - $map->ReadConfig($mapfile); + if (!wm_editor_read_config($map, $mapfile)) return; $use_overlay = (isset_request_var('editorsettings_showvias') ? intval(get_nfilter_request_var('editorsettings_showvias')) : false); $use_relative_overlay = (isset_request_var('editorsettings_showrelative') ? intval(get_nfilter_request_var('editorsettings_showrelative')) : false); @@ -963,10 +1001,14 @@ function deleteNode($mapfile) { $map->context = 'editor'; - $map->ReadConfig($mapfile); + if (!wm_editor_read_config($map, $mapfile)) return; $target = wm_editor_sanitize_name(get_nfilter_request_var('param')); + if (!isset($map->nodes[$target])) { + http_response_code(404); + return; + } if (isset($map->nodes[$target])) { $log = 'delete node ' . $target; @@ -980,7 +1022,7 @@ function deleteNode($mapfile) { unset($map->nodes[$target]); - $map->WriteConfig($mapfile); + wm_editor_write_config($map, $mapfile); } } @@ -993,7 +1035,7 @@ function cloneNode($mapfile) { $map->context = 'editor'; - $map->ReadConfig($mapfile); + if (!wm_editor_read_config($map, $mapfile)) return; $target = wm_editor_sanitize_name(get_nfilter_request_var('param')); @@ -1024,7 +1066,7 @@ function cloneNode($mapfile) { array_push($map->seen_zlayers[$node->zorder], $node); - $map->WriteConfig($mapfile); + wm_editor_write_config($map, $mapfile); } } @@ -1037,7 +1079,7 @@ function displayFontSamples($mapfile) { $map->context = 'editor'; - $map->ReadConfig($mapfile); + if (!wm_editor_read_config($map, $mapfile)) return; ksort($map->fonts); header('Content-type: image/png'); diff --git a/lib/editor.inc.php b/lib/editor.inc.php index e314229a..19a0cb6a 100644 --- a/lib/editor.inc.php +++ b/lib/editor.inc.php @@ -1036,6 +1036,7 @@ function getEditorJs() { 'iconMatchCount' => __('Matching icons: %s', '%s', 'weathermap'), 'iconNoMatches' => __('No matching icons', 'weathermap'), + 'saveError' => __('Unable to save changes. Please try again.', 'weathermap'), 'currentInterface' => __('Current interface', 'weathermap'), 'advanced' => __('Advanced settings', 'weathermap'), 'hideAdvanced' => __('Hide advanced settings', 'weathermap'), diff --git a/locales/po/cacti.pot b/locales/po/cacti.pot index c13cbea6..d326380e 100644 --- a/locales/po/cacti.pot +++ b/locales/po/cacti.pot @@ -8,7 +8,7 @@ msgid "" msgstr "" "Project-Id-Version: Cacti 1.7\n" "Report-Msgid-Bugs-To: developers@cacti.net\n" -"POT-Creation-Date: 2026-10-12 12:29+1030\n" +"POT-Creation-Date: 2026-10-12 13:03+1030\n" "PO-Revision-Date: YEAR-MO-DA HO:MI+ZONE\n" "Last-Translator: Cacti Developers \n" "Language-Team: Cacti Developers \n" @@ -330,6 +330,10 @@ msgstr "" msgid "No matching icons" msgstr "" +#: lib/editor.inc.php +msgid "Unable to save changes. Please try again." +msgstr "" + #: lib/editor.inc.php msgid "Current interface" msgstr "" diff --git a/tests/Browser/EditorSaveFailureTest.cjs b/tests/Browser/EditorSaveFailureTest.cjs new file mode 100644 index 00000000..447a1c29 --- /dev/null +++ b/tests/Browser/EditorSaveFailureTest.cjs @@ -0,0 +1,6 @@ +const {JSDOM}=require('jsdom'),jquery=require('jquery'),fs=require('node:fs'),path=require('node:path'),assert=require('node:assert/strict'); +const dom=new JSDOM('',{runScripts:'outside-only'}),w=dom.window,$=jquery(w);w.$=w.jQuery=$;w.editor_url='/editor';w.wmEditorText={saveError:'Unable to save changes'}; +let request,message,closed=0,refreshed=0;$.ajax=opts=>request=opts;w.alert=text=>message=text;w.hide_all_dialogs=()=>closed++;$.get=()=>refreshed++; +const source=fs.readFileSync(path.join(__dirname,'../../js/editor.js'),'utf8');w.eval(source.slice(source.indexOf('function form_submit()'),source.indexOf('function map_properties()'))); +w.form_submit();request.error();assert.equal(message,'Unable to save changes');assert.equal(closed,0);assert.equal(refreshed,0);assert.equal($('#action').val(),'set_map_properties'); +w.close();console.log('PASS: save failure reports retry and preserves the form'); diff --git a/tests/Support/ConfigWriteAtomicRegression.php b/tests/Support/ConfigWriteAtomicRegression.php index 0fe0d0e2..df55f377 100644 --- a/tests/Support/ConfigWriteAtomicRegression.php +++ b/tests/Support/ConfigWriteAtomicRegression.php @@ -241,12 +241,20 @@ function atomic_assert($condition, $message) { $map = new \WeatherMap; $map->ReadConfig($file); $map->title = 'Changed'; atomic_assert(!$map->WriteConfig($file) && file_get_contents($file) === $original, 'engine failure preserves original'); $GLOBALS['__test_nfilter_request'] = ['node_name' => 'note1', 'param' => 'note1', 'x' => '70', 'y' => '80']; - if (function_exists('wm_editor_save_note')) { + { http_response_code(200); moveNode($file, 0); atomic_assert(http_response_code() === 500 && file_get_contents($file) === $original, 'failed move surfaces error without changing file'); http_response_code(200); deleteNode($file); atomic_assert(http_response_code() === 500 && file_get_contents($file) === $original, 'failed delete surfaces error without changing file'); } + $GLOBALS['config_fault'] = ''; + $GLOBALS['__test_nfilter_request'] = ['node_name' => 'missing', 'param' => 'missing', 'x' => '70', 'y' => '80']; + http_response_code(200); moveNode($file, 0); + atomic_assert(http_response_code() === 404 && file_get_contents($file) === $original, 'missing move target is an error'); + http_response_code(200); deleteNode($file); + atomic_assert(http_response_code() === 404 && file_get_contents($file) === $original, 'missing delete target is an error'); + unlink($file); http_response_code(200); moveNode($file, 0); + atomic_assert(http_response_code() === 404 && !file_exists($file), 'deleted map cannot report a successful edit'); echo "Atomic configuration write and locking regression PASS\n"; } finally { $GLOBALS['config_fault'] = ''; diff --git a/tests/Unit/WeathermapUpgradeTest.php b/tests/Unit/WeathermapUpgradeTest.php index 24ca2c17..9a3bdad8 100644 --- a/tests/Unit/WeathermapUpgradeTest.php +++ b/tests/Unit/WeathermapUpgradeTest.php @@ -5,66 +5,63 @@ +-------------------------------------------------------------------------+ */ -/* - * Unit coverage for plugin_weathermap_upgrade() in setup.php: the - * page-guard. - * - * The rest of the function (reached on any allowed page) is intentionally - * NOT covered here: it include_once()s lib/poller-common.php, which itself - * include_once()s lib/WeatherMap.class.php - a file this plugin's own - * other test suites (e.g. tests/Handoff) already load directly by a - * different path, and loading it a second time via a different resolved - * path fatals with "Cannot declare class WeatherMapDataSource, because - * the name is already in use". The version-drift branch also calls - * weathermap_repair_maps(), which does a real chdir() and reads/writes - * map config files on disk - a much larger and riskier surface than this - * suite stubs. - */ - -// Only upgrade tests stub the filesystem repair; isolated engine tests load it. -if (!function_exists('weathermap_repair_maps')) { - function weathermap_repair_maps() {} -} - -beforeAll(function () { - require_once __DIR__ . '/../../setup.php'; -}); - -beforeEach(function () { - $GLOBALS['__test_db_calls'] = []; - $GLOBALS['__test_current_page'] = ''; -}); +use PHPUnit\Framework\Attributes\RunTestsInSeparateProcesses; +use PHPUnit\Framework\Attributes\PreserveGlobalState; +use PHPUnit\Framework\TestCase; -it('does nothing on a page that does not need the version check', function () { - test_set_current_page('graphs.php'); +/** Exercise upgrade dispatch without sharing production-function stubs. */ +#[RunTestsInSeparateProcesses] +#[PreserveGlobalState(false)] +final class WeathermapUpgradeTest extends TestCase { + /** + * Install the upgrade-only repair stub in this isolated process. + * + * @return void + */ + protected function setUp(): void { + require_once __DIR__ . '/../../setup.php'; + if (!function_exists('weathermap_repair_maps')) { + function weathermap_repair_maps() {} + } + $GLOBALS['__test_db_calls'] = []; + $GLOBALS['__test_current_page'] = ''; + } - plugin_weathermap_upgrade(); + /** + * Verify unrelated pages do not trigger upgrade queries. + * + * @return void + */ + public function testUnrelatedPageSkipsUpgrade(): void { + test_set_current_page('graphs.php'); + plugin_weathermap_upgrade(); + self::assertEmpty($GLOBALS['__test_db_calls']); + } - expect($GLOBALS['__test_db_calls'])->toBeEmpty(); -}); - -it('runs the realm/version updates and prune on a version drift', function () { -test_set_current_page('plugins.php'); - -// Sandbox base_path with an empty poller-common stub + minimal INFO so the -// upgrade-time prune runs against a throwaway tree, never the real checkout, -// and the heavy lib include is a no-op. -$restore = $GLOBALS['config']['base_path']; -$base = sys_get_temp_dir() . '/weathermap-upg-' . uniqid(); -mkdir($base . '/plugins/weathermap/lib', 0777, true); -file_put_contents($base . '/plugins/weathermap/lib/poller-common.php', "not->toBeEmpty(); -}); diff --git a/weathermap-cacti-plugin-editor.php b/weathermap-cacti-plugin-editor.php index 8196770b..46efd3c8 100644 --- a/weathermap-cacti-plugin-editor.php +++ b/weathermap-cacti-plugin-editor.php @@ -873,7 +873,7 @@

'>
-
+
From 25e27e1d683ed96e8f311d4464e3e561d0aba4f8 Mon Sep 17 00:00:00 2001 From: alcatron <27321443+alcatron@users.noreply.github.com> Date: Mon, 12 Oct 2026 21:54:12 +1030 Subject: [PATCH 15/41] Compare configuration revisions before saving edits and repairs --- lib/WeatherMap.class.php | 17 +++++-- lib/WeatherMap.config.php | 9 +++- lib/editor.actions.php | 10 ++-- lib/poller-common.php | 3 +- tests/Support/ConfigWriteAtomicRegression.php | 48 +++++++++++++++++++ tests/Unit/ConfigWriteAtomicTest.php | 5 ++ tests/Unit/WeathermapUpgradeTest.php | 5 ++ weathermap-cacti-plugin-mgmt.php | 2 +- 8 files changed, 88 insertions(+), 11 deletions(-) diff --git a/lib/WeatherMap.class.php b/lib/WeatherMap.class.php index 01c46ae3..7bdd2db4 100644 --- a/lib/WeatherMap.class.php +++ b/lib/WeatherMap.class.php @@ -341,6 +341,8 @@ class WeatherMap extends WeatherMapBase { var $colours; /** @var mixed */ var $configfile; + var $config_revision = null; + var $editor_config_revision = null; /** @var mixed */ var $imagefile; /** @var mixed */ @@ -2445,6 +2447,7 @@ function DrawTitle($image, $font, $colour) { */ function ReadConfig($input, $is_include = false) { global $config, $weathermap_error_suppress; + if (!$is_include) $this->config_revision = null; $curnode = null; $curlink = null; @@ -2491,16 +2494,19 @@ function ReadConfig($input, $is_include = false) { } if (is_file($filename)) { - $fd = fopen($filename, 'r'); + $fd = fopen($filename, 'rb'); if ($fd) { + $revision = hash_init('sha256'); while (($buffer = fgets($fd, 4096)) !== false) { + hash_update($revision, $buffer); // strip out any Windows line-endings that have gotten in here $buffer = str_replace("\r", '', $buffer); $lines[] = $buffer; } fclose($fd); + if (!$is_include) $this->config_revision = hash_final($revision); } } } @@ -3604,9 +3610,11 @@ function WriteDataFile($filename) { /** * @param string $filename + * @param bool $createOnly Require a new destination. + * @param string|null $expectedRevision Optional request snapshot revision. * @return bool */ - function WriteConfig($filename) { + function WriteConfig($filename, $createOnly = false, $expectedRevision = null) { require_once __DIR__ . '/WeatherMap.config.php'; $fd = fopen('php://temp', 'w+b'); @@ -3847,7 +3855,10 @@ function WriteConfig($filename) { $contents = stream_get_contents($fd); $closed = fclose($fd); - return $write_ok && $contents !== false && $closed && \WeathermapConfig\atomicWrite($filename, $contents, $this->configfile !== '' && $filename === $this->configfile); + $sameFile = $this->configfile !== '' && $filename === $this->configfile; + $saved = $write_ok && $contents !== false && $closed && \WeathermapConfig\atomicWrite($filename, $contents, $sameFile, $expectedRevision ?? ($sameFile ? $this->config_revision : null), $createOnly); + if ($saved && $sameFile) $this->config_revision = hash('sha256', $contents); + return $saved; } else { wm_warn("Couldn't open config file $filename for writing"); diff --git a/lib/WeatherMap.config.php b/lib/WeatherMap.config.php index 7d104001..b89db55e 100644 --- a/lib/WeatherMap.config.php +++ b/lib/WeatherMap.config.php @@ -80,22 +80,27 @@ function writeAll($stream, $contents) { * @param string $filename The destination configuration path. * @param string $contents The complete new configuration. * @param bool $mustExist Refuse to recreate an existing map deleted during editing. + * @param string|null $expectedRevision SHA-256 of the snapshot being edited. + * @param bool $createOnly Refuse to replace an existing destination. * * @return bool Success; failures preserve the existing configuration. */ -function atomicWrite($filename, $contents, $mustExist = false) { +function atomicWrite($filename, $contents, $mustExist = false, $expectedRevision = null, $createOnly = false) { if (strpos($filename, '://') !== false) return false; if (is_link($filename)) $filename = realpath($filename); if ($filename === false || !is_dir(dirname($filename)) || !is_writable(dirname($filename))) return false; if (file_exists($filename) && (!is_file($filename) || !is_writable($filename))) return false; - $mustExist = $mustExist || file_exists($filename); + $existed = file_exists($filename); + $mustExist = $mustExist || $existed; $lock = acquire(dirname($filename)); if (!$lock) return false; $temp = false; $stream = false; try { clearstatcache(true, $filename); + if (($createOnly || !$existed) && file_exists($filename)) return false; if ($mustExist && !is_file($filename)) return false; + if ($expectedRevision !== null && @hash_file('sha256', $filename) !== $expectedRevision) return false; if (file_exists($filename) && (!is_file($filename) || !is_writable($filename))) return false; $temp = @tempnam(dirname($filename), '.wm-config-'); if ($temp === false) return false; diff --git a/lib/editor.actions.php b/lib/editor.actions.php index 9c89ff5c..9b0e09cf 100644 --- a/lib/editor.actions.php +++ b/lib/editor.actions.php @@ -64,6 +64,7 @@ function wm_editor_read_config($map, $mapfile) { http_response_code(422); return false; } + $map->editor_config_revision = $map->config_revision; return true; } @@ -72,10 +73,11 @@ function wm_editor_read_config($map, $mapfile) { * * @param WeatherMap $map Engine containing the edited configuration. * @param string $mapfile Configuration path. + * @param bool $createOnly Require a new destination for map creation. * @return bool Whether the complete configuration was saved. */ -function wm_editor_write_config($map, $mapfile) { - if (!$map->WriteConfig($mapfile)) { +function wm_editor_write_config($map, $mapfile, $createOnly = false) { + if (!$map->WriteConfig($mapfile, $createOnly, $map->editor_config_revision)) { http_response_code(500); return false; } @@ -93,7 +95,7 @@ function newMap($mapfile) { require_once __DIR__ . '/editor.new-map-preset.php'; wm_new_map_preset($map); - wm_editor_write_config($map, $mapfile); + wm_editor_write_config($map, $mapfile, true); } /** @@ -120,7 +122,7 @@ function newMapCopy($mapfile) { if (file_exists($sourcemap) && is_readable($sourcemap)) { $map->ReadConfig($sourcemap); - wm_editor_write_config($map, $mapfile); + wm_editor_write_config($map, $mapfile, true); } } } diff --git a/lib/poller-common.php b/lib/poller-common.php index ca63f097..362516f3 100644 --- a/lib/poller-common.php +++ b/lib/poller-common.php @@ -144,6 +144,7 @@ function weathermap_repair_maps() { if (file_exists($configfile) && is_writable($configfile)) { $contents = file($configfile) ?: []; + $revision = hash('sha256', implode('', $contents)); foreach ($contents as $line) { if (strpos($line, 'BACKGROUND') !== false) { @@ -272,7 +273,7 @@ function weathermap_repair_maps() { if ($changes) { wm_debug('Writing updated file to: ' . $configfile); require_once __DIR__ . '/WeatherMap.config.php'; - if (!\WeathermapConfig\atomicWrite($configfile, implode("\n", $outcontents), true)) { + if (!\WeathermapConfig\atomicWrite($configfile, implode("\n", $outcontents), true, $revision)) { wm_warn('Unable to save repaired configuration: ' . $configfile . ' [WMPOLL13]'); } } else { diff --git a/tests/Support/ConfigWriteAtomicRegression.php b/tests/Support/ConfigWriteAtomicRegression.php index df55f377..2f4fb4cf 100644 --- a/tests/Support/ConfigWriteAtomicRegression.php +++ b/tests/Support/ConfigWriteAtomicRegression.php @@ -44,6 +44,7 @@ function fopen($path, $mode) { */ function flock($stream, $mode) { if (($GLOBALS['config_fault'] ?? '') === 'lock' && $mode === LOCK_EX) return false; + if (($mode & LOCK_EX) && isset($GLOBALS['config_before_lock'])) ($GLOBALS['config_before_lock'])(); return \flock($stream, $mode); } /** @@ -204,6 +205,44 @@ function atomic_assert($condition, $message) { atomic_assert(!\WeathermapConfig\acquire($dir . '/missing'), 'missing lock directory rejected'); atomic_assert(!\WeathermapConfig\atomicWrite($dir . '/deleted.conf', 'Replacement', true), 'deleted existing maps are not recreated'); atomic_assert(is_file($dir . '/.weathermap-config.lock'), 'arbitrary output uses its destination directory lock'); + atomic_assert(!\WeathermapConfig\atomicWrite($file, 'Overwrite', false, null, true), 'create-only destination refuses existing files'); + $first = new \WeatherMap; $second = new \WeatherMap; + $first->ReadConfig($file); $second->ReadConfig($file); + $first->title = 'First edit'; $second->title = 'Stale edit'; + atomic_assert($first->WriteConfig($file), 'first revision saves'); + $current = file_get_contents($file); + atomic_assert(!$second->WriteConfig($file) && file_get_contents($file) === $current, 'stale reader cannot overwrite the first edit'); + $first->title = 'Next edit'; + atomic_assert($first->WriteConfig($file), 'successful save updates its revision for subsequent writes'); + $raw = new \WeatherMap; wm_editor_read_config($raw, $file); + $raw->ReadConfig("TITLE Fragment edit\n"); + file_put_contents($file, $original); + http_response_code(200); + atomic_assert(!wm_editor_write_config($raw, $file) && http_response_code() === 500 && file_get_contents($file) === $original, 'raw fragment edits retain the original request revision'); + if (function_exists('pcntl_fork')) { + $createFile = $dir . '/create.conf'; + $lock = \WeathermapConfig\acquire($dir); $children = []; + for ($index = 0; $index < 2; $index++) { + $child = pcntl_fork(); + if ($child === 0) { + \fclose($lock); + $GLOBALS['config_before_lock'] = function() use ($dir, $index) { file_put_contents($dir . '/create-ready-' . $index, 'ready'); }; + $saved = \WeathermapConfig\atomicWrite($createFile, 'Creator ' . $index, false, null, true); + file_put_contents($dir . '/create-result-' . $index, $saved ? '1' : '0'); + exit(0); + } + $children[] = $child; + } + for ($wait = 0; $wait < 100 && cacti_count(glob($dir . '/create-ready-*')) < 2; $wait++) usleep(10000); + \WeathermapConfig\release($lock); + foreach ($children as $child) pcntl_waitpid($child, $status); + atomic_assert((int)file_get_contents($dir . '/create-result-0') + (int)file_get_contents($dir . '/create-result-1') === 1, 'exactly one concurrent creator commits'); + atomic_assert(in_array(file_get_contents($createFile), ['Creator 0', 'Creator 1'], true), 'winning creator content remains intact'); + } + $crlfFile = $dir . '/crlf.conf'; + $crlf = "WIDTH 600\r\nHEIGHT 300\r\n"; file_put_contents($crlfFile, $crlf); + $crlfMap = new \WeatherMap; $crlfMap->ReadConfig($crlfFile); + atomic_assert($crlfMap->config_revision === hash('sha256', $crlf) && $crlfMap->WriteConfig($crlfFile), 'revision hashes preserve raw CRLF bytes'); $templateFile = $dir . '/templates.conf'; file_put_contents($templateFile, "NODE templateNode\n LABEL Template\nNODE A\n POSITION 10 10\nNODE B\n POSITION 50 50\nLINK templateLink\n WIDTH 8\nLINK trafficLink\n NODES A B\n"); $templateMap = new \WeatherMap; $templateMap->ReadConfig($templateFile); @@ -228,6 +267,15 @@ function atomic_assert($condition, $message) { $GLOBALS['config_fault'] = ''; weathermap_repair_maps(); atomic_assert(file_get_contents($repairFile) === 'BACKGROUND images/backgrounds/' . basename($repairImage), 'repair uses complete atomic replacement'); + file_put_contents($repairFile, $legacy); + $newerRepair = 'TITLE Concurrent editor save' . "\n" . $legacy; + $GLOBALS['config_before_lock'] = function() use ($repairFile, $newerRepair) { + unset($GLOBALS['config_before_lock']); + file_put_contents($repairFile, $newerRepair); + }; + $GLOBALS['weathermap_error_suppress'] = ['WMPOLL13']; + try { weathermap_repair_maps(); } finally { $GLOBALS['weathermap_error_suppress'] = $oldSuppress; } + atomic_assert(file_get_contents($repairFile) === $newerRepair, 'repair CAS preserves a concurrent editor save'); } finally { $GLOBALS['config'] = $oldConfig; unset($GLOBALS['__test_db_fetch_assoc']); diff --git a/tests/Unit/ConfigWriteAtomicTest.php b/tests/Unit/ConfigWriteAtomicTest.php index 48736556..913fe485 100644 --- a/tests/Unit/ConfigWriteAtomicTest.php +++ b/tests/Unit/ConfigWriteAtomicTest.php @@ -26,6 +26,11 @@ use PHPUnit\Framework\TestCase; final class ConfigWriteAtomicTest extends TestCase { + /** + * Verify real engine persistence and coordinated filesystem failures. + * + * @return void + */ #[RunInSeparateProcess] #[PreserveGlobalState(false)] public function testRealEngineRegression(): void { diff --git a/tests/Unit/WeathermapUpgradeTest.php b/tests/Unit/WeathermapUpgradeTest.php index 9a3bdad8..fd63cdbd 100644 --- a/tests/Unit/WeathermapUpgradeTest.php +++ b/tests/Unit/WeathermapUpgradeTest.php @@ -21,6 +21,11 @@ final class WeathermapUpgradeTest extends TestCase { protected function setUp(): void { require_once __DIR__ . '/../../setup.php'; if (!function_exists('weathermap_repair_maps')) { + /** + * Stub filesystem repair only within an isolated upgrade test. + * + * @return void + */ function weathermap_repair_maps() {} } $GLOBALS['__test_db_calls'] = []; diff --git a/weathermap-cacti-plugin-mgmt.php b/weathermap-cacti-plugin-mgmt.php index 81271715..28d68cde 100644 --- a/weathermap-cacti-plugin-mgmt.php +++ b/weathermap-cacti-plugin-mgmt.php @@ -2026,7 +2026,7 @@ function map_duplicate($id, $titlecache, $configfile = null) { if ($contents !== false) { $contents = str_replace("TITLE {$map['titlecache']}", "TITLE {$save['titlecache']}", $contents); require_once __DIR__ . '/lib/WeatherMap.config.php'; - if (!\WeathermapConfig\atomicWrite($newfile, $contents)) { + if (!\WeathermapConfig\atomicWrite($newfile, $contents, false, null, true)) { raise_message('new_map_' . $newid, __('Could not save the map configuration.', 'weathermap'), MESSAGE_LEVEL_ERROR); } } else { From fff23c61d51f8853b5d1a7b2dfba689ac5bdf5ee Mon Sep 17 00:00:00 2001 From: alcatron <27321443+alcatron@users.noreply.github.com> Date: Mon, 12 Oct 2026 23:21:49 +1030 Subject: [PATCH 16/41] Protect literal image includes and prevent failed clone registrations --- js/editor.js | 3 +- lib/editor.actions.php | 16 ++++-- lib/editor.assets.php | 10 ++-- lib/editor.inc.php | 1 + locales/po/cacti.pot | 20 +++++--- tests/Browser/EditorSaveFailureTest.cjs | 3 +- tests/Support/ConfigWriteAtomicRegression.php | 39 +++++++++++++++ .../Support/EditorImageFormatsRegression.php | 25 +++++----- tests/Support/MapDuplicatePayload.php | 16 ++++-- tests/bootstrap-unit.php | 2 + weathermap-cacti-plugin-mgmt.php | 49 ++++++++++++------- 11 files changed, 129 insertions(+), 55 deletions(-) diff --git a/js/editor.js b/js/editor.js index 6e5a9bbc..0ce9fd0e 100644 --- a/js/editor.js +++ b/js/editor.js @@ -873,12 +873,13 @@ function delete_link() { function form_submit() { var data = $('input, select, textarea').serialize(); + if (typeof WMConfigRevision !== 'undefined') data += '&config_revision=' + encodeURIComponent(WMConfigRevision); $.ajax({ type: 'POST', url: editor_url, data: data, - error: function() { window.alert(wmEditorText.saveError); }, + error: function(xhr) { window.alert(xhr && xhr.status === 409 ? wmEditorText.staleError : wmEditorText.saveError); }, success: function(html) { hide_all_dialogs(); diff --git a/lib/editor.actions.php b/lib/editor.actions.php index 9b0e09cf..47d5347e 100644 --- a/lib/editor.actions.php +++ b/lib/editor.actions.php @@ -64,6 +64,11 @@ function wm_editor_read_config($map, $mapfile) { http_response_code(422); return false; } + $revision = get_nfilter_request_var('config_revision'); + if ($revision !== null && $revision !== '' && (!is_string($revision) || !hash_equals($map->config_revision, $revision))) { + http_response_code(409); + return false; + } $map->editor_config_revision = $map->config_revision; return true; } @@ -139,6 +144,7 @@ function getMapJavaScript($mapfile) { if (!wm_editor_read_config($map, $mapfile)) return; print $map->asJS(); + print 'var WMConfigRevision = ' . json_encode($map->config_revision) . ';'; } /** @@ -515,10 +521,6 @@ function setMapProperties($mapfile) { $map->add_hint('background_sizing', $background_sizing); } - db_execute_prepared('UPDATE weathermap_maps - SET titlecache = ? - WHERE configfile = ?', - [$map->title, basename($mapfile)]); $inheritables = [ ['link', 'width', 'map_linkdefaultwidth', 'float'] @@ -564,7 +566,11 @@ function setMapProperties($mapfile) { } } - wm_editor_write_config($map, $mapfile); + if (!wm_editor_write_config($map, $mapfile)) return; + db_execute_prepared('UPDATE weathermap_maps + SET titlecache = ? + WHERE configfile = ?', + [$map->title, basename($mapfile)]); } /** diff --git a/lib/editor.assets.php b/lib/editor.assets.php index b25a1136..d309715c 100644 --- a/lib/editor.assets.php +++ b/lib/editor.assets.php @@ -26,12 +26,13 @@ * Resolve a managed image library, rejecting symlinked directories. * * @param string $kind The objects or backgrounds library. + * @param string|null $root Optional library root; defaults to the plugin directory. * * @return string The library directory. */ -function wm_asset_directory($kind) { +function wm_asset_directory($kind, $root = null) { if (!in_array($kind, ['objects', 'backgrounds'], true)) throw new InvalidArgumentException(__('Invalid image category.', 'weathermap')); - $parent = dirname(__DIR__) . '/images'; + $parent = ($root ?? dirname(__DIR__)) . '/images'; $dir = $parent . '/' . $kind; if (is_link($parent) || is_link($dir)) throw new InvalidArgumentException(__('Symbolic links cannot be managed here.', 'weathermap')); return $dir; @@ -107,9 +108,8 @@ function wm_asset_image_reference_index() { } if (preg_match_all('/^\s*INCLUDE\s+(.+)$/mi', $text, $matches)) foreach ($matches[1] as $include) { $include = wm_asset_normalize_path(trim($include), $root); - $matches = glob($include); - // The renderer tolerates absent optional includes; unreadable matches still fail closed. - foreach ($matches ?: [] as $match) $queue[] = $match; + // INCLUDE is a literal filename in the renderer, including glob metacharacters. + if (file_exists($include) || is_link($include)) $queue[] = $include; } } return $references; diff --git a/lib/editor.inc.php b/lib/editor.inc.php index 19a0cb6a..40a420b2 100644 --- a/lib/editor.inc.php +++ b/lib/editor.inc.php @@ -1036,6 +1036,7 @@ function getEditorJs() { 'iconMatchCount' => __('Matching icons: %s', '%s', 'weathermap'), 'iconNoMatches' => __('No matching icons', 'weathermap'), + 'staleError' => __('This map has changed since you opened it. Reload the editor before saving your changes.', 'weathermap'), 'saveError' => __('Unable to save changes. Please try again.', 'weathermap'), 'currentInterface' => __('Current interface', 'weathermap'), 'advanced' => __('Advanced settings', 'weathermap'), diff --git a/locales/po/cacti.pot b/locales/po/cacti.pot index d326380e..fd3d1932 100644 --- a/locales/po/cacti.pot +++ b/locales/po/cacti.pot @@ -8,7 +8,7 @@ msgid "" msgstr "" "Project-Id-Version: Cacti 1.7\n" "Report-Msgid-Bugs-To: developers@cacti.net\n" -"POT-Creation-Date: 2026-10-12 13:03+1030\n" +"POT-Creation-Date: 2026-10-12 23:20+1030\n" "PO-Revision-Date: YEAR-MO-DA HO:MI+ZONE\n" "Last-Translator: Cacti Developers \n" "Language-Team: Cacti Developers \n" @@ -330,6 +330,10 @@ msgstr "" msgid "No matching icons" msgstr "" +#: lib/editor.inc.php +msgid "This map has changed since you opened it. Reload the editor before saving your changes." +msgstr "" + #: lib/editor.inc.php msgid "Unable to save changes. Please try again." msgstr "" @@ -1934,17 +1938,17 @@ msgid "(No Title)" msgstr "" #: weathermap-cacti-plugin-mgmt.php -#, php-format -msgid "The new Map with the name %s was created using config file %s" +msgid "Could not save the map configuration." msgstr "" #: weathermap-cacti-plugin-mgmt.php -msgid "Could not save the map configuration." +#, php-format +msgid "The new Map with the name %s was unable to locate the config file %s to copy" msgstr "" #: weathermap-cacti-plugin-mgmt.php #, php-format -msgid "The new Map with the name %s was unable to locate the config file %s to copy" +msgid "The new Map with the name %s was created using config file %s" msgstr "" #: weathermap-cacti-plugin-mgmt.php @@ -2225,12 +2229,12 @@ msgid "The New Map File name %s already exists!" msgstr "" #: weathermap-cacti-plugin-mgmt.php -#, php-format -msgid "New Map file %s created from %s" +msgid "The Source Map File name is not readable or does not exist!" msgstr "" #: weathermap-cacti-plugin-mgmt.php -msgid "The Source Map File name is not readable or does not exist!" +#, php-format +msgid "New Map file %s created from %s" msgstr "" #: weathermap-cacti-plugin-mgmt.php diff --git a/tests/Browser/EditorSaveFailureTest.cjs b/tests/Browser/EditorSaveFailureTest.cjs index 447a1c29..108b9acf 100644 --- a/tests/Browser/EditorSaveFailureTest.cjs +++ b/tests/Browser/EditorSaveFailureTest.cjs @@ -1,6 +1,7 @@ const {JSDOM}=require('jsdom'),jquery=require('jquery'),fs=require('node:fs'),path=require('node:path'),assert=require('node:assert/strict'); -const dom=new JSDOM('',{runScripts:'outside-only'}),w=dom.window,$=jquery(w);w.$=w.jQuery=$;w.editor_url='/editor';w.wmEditorText={saveError:'Unable to save changes'}; +const dom=new JSDOM('',{runScripts:'outside-only'}),w=dom.window,$=jquery(w);w.$=w.jQuery=$;w.editor_url='/editor';w.wmEditorText={staleError:'Reload the editor',saveError:'Unable to save changes'}; let request,message,closed=0,refreshed=0;$.ajax=opts=>request=opts;w.alert=text=>message=text;w.hide_all_dialogs=()=>closed++;$.get=()=>refreshed++; const source=fs.readFileSync(path.join(__dirname,'../../js/editor.js'),'utf8');w.eval(source.slice(source.indexOf('function form_submit()'),source.indexOf('function map_properties()'))); w.form_submit();request.error();assert.equal(message,'Unable to save changes');assert.equal(closed,0);assert.equal(refreshed,0);assert.equal($('#action').val(),'set_map_properties'); +w.WMConfigRevision='b'.repeat(64);w.form_submit();assert.match(request.data,/config_revision=b{64}/);request.error({status:409});assert.equal(message,'Reload the editor');assert.equal(closed,0);assert.equal(refreshed,0); w.close();console.log('PASS: save failure reports retry and preserves the form'); diff --git a/tests/Support/ConfigWriteAtomicRegression.php b/tests/Support/ConfigWriteAtomicRegression.php index 2f4fb4cf..aecd9104 100644 --- a/tests/Support/ConfigWriteAtomicRegression.php +++ b/tests/Support/ConfigWriteAtomicRegression.php @@ -214,6 +214,13 @@ function atomic_assert($condition, $message) { atomic_assert(!$second->WriteConfig($file) && file_get_contents($file) === $current, 'stale reader cannot overwrite the first edit'); $first->title = 'Next edit'; atomic_assert($first->WriteConfig($file), 'successful save updates its revision for subsequent writes'); + $GLOBALS['__test_nfilter_request'] = ['config_revision' => hash('sha256', $original), 'node_name' => 'note1', 'x' => 90, 'y' => 90]; + http_response_code(200); moveNode($file, 0); + atomic_assert(http_response_code() === 409 && file_get_contents($file) !== $original, 'stale loaded page cannot overwrite another user save'); + $GLOBALS['__test_nfilter_request'] = ['config_revision' => hash_file('sha256', $file)]; + $matching = new \WeatherMap; + atomic_assert(wm_editor_read_config($matching, $file), 'matching loaded page revision accepted'); + $GLOBALS['__test_nfilter_request'] = []; $raw = new \WeatherMap; wm_editor_read_config($raw, $file); $raw->ReadConfig("TITLE Fragment edit\n"); file_put_contents($file, $original); @@ -281,6 +288,31 @@ function atomic_assert($condition, $message) { unset($GLOBALS['__test_db_fetch_assoc']); unlink($repairFile); unlink($repairImage); } + $management = file_get_contents($plugin . '/weathermap-cacti-plugin-mgmt.php'); + $start = strpos($management, 'function newMap('); + eval(str_replace(['__DIR__', 'function newMap('], [var_export($plugin, true), 'function wm_management_newMap('], substr($management, $start))); + $GLOBALS['weathermap_confdir'] = $dir; + foreach (['', 'map.conf'] as $source) { + $destination = $dir . '/management-create.conf'; + $GLOBALS['config_before_lock'] = function() use ($destination) { file_put_contents($destination, 'Concurrent creator'); unset($GLOBALS['config_before_lock']); }; + wm_management_newMap('management-create.conf', $source); + atomic_assert(file_get_contents($destination) === 'Concurrent creator', 'management create-only preserves concurrent destination'); + unlink($destination); + } + $start = strpos($management, 'function map_get_next_name('); $end = strpos($management, "\n/**", $start); + eval(str_replace('__DIR__', var_export($plugin, true), substr($management, $start, $end - $start))); + $start = strpos($management, 'function map_duplicate('); $end = strpos($management, "\n/**", $start); + eval(str_replace('__DIR__', var_export($plugin, true), substr($management, $start, $end - $start))); + $duplicateSource = 'atomic-duplicate-' . bin2hex(random_bytes(4)) . '.conf'; + file_put_contents($plugin . '/configs/' . $duplicateSource, "TITLE Source\n"); + $GLOBALS['__test_db_fetch_row_prepared'] = fn () => ['configfile' => $duplicateSource, 'titlecache' => 'Source', 'group_id' => 1, 'active' => 'on', 'thumb_height' => 10, 'thumb_width' => 10, 'schedule' => '', 'archiving' => '',]; + $GLOBALS['__test_db_calls'] = []; $GLOBALS['atomic_sql_saves'] = []; + $GLOBALS['__test_db_fetch_cell'] = fn () => 0; + $GLOBALS['__test_sql_save'] = function($record) { $GLOBALS['atomic_sql_saves'][] = $record; return 1; }; $GLOBALS['config_fault'] = 'rename'; + try { + map_duplicate(1, 'Clone', 'clone'); + atomic_assert($GLOBALS['__test_db_calls'] === [] && $GLOBALS['atomic_sql_saves'] === [], 'failed clone never registers auth or settings or invokes runner'); + } finally { unset($GLOBALS['__test_db_fetch_row_prepared'], $GLOBALS['__test_sql_save'], $GLOBALS['__test_db_fetch_cell']); unlink($plugin . '/configs/' . $duplicateSource); $GLOBALS['config_fault'] = ''; } $deletedMap = new \WeatherMap; $deletedMap->ReadConfig($file); unlink($file); atomic_assert(!$deletedMap->WriteConfig($file) && !file_exists($file), 'saving a previously loaded deleted map fails without resurrection'); @@ -288,6 +320,13 @@ function atomic_assert($condition, $message) { $GLOBALS['config_fault'] = 'rename'; $map = new \WeatherMap; $map->ReadConfig($file); $map->title = 'Changed'; atomic_assert(!$map->WriteConfig($file) && file_get_contents($file) === $original, 'engine failure preserves original'); + $GLOBALS['__test_nfilter_request'] = ['map_title' => 'Failed title', 'map_legend' => 'Traffic', 'map_stamp' => 'Created', 'map_width' => 600, 'map_height' => 300, 'map_bgfile' => '--NONE--', 'map_linkdefaultwidth' => 7, 'map_linkdefaultbwin' => '100M', 'map_linkdefaultbwout' => '100M']; + $GLOBALS['__test_db_calls'] = []; + http_response_code(200); setMapProperties($file); + atomic_assert(http_response_code() === 500 && $GLOBALS['__test_db_calls'] === [], 'failed properties save cannot change titlecache'); + $GLOBALS['config_fault'] = ''; http_response_code(200); setMapProperties($file); + atomic_assert(http_response_code() === 200 && cacti_count($GLOBALS['__test_db_calls']) === 1, 'successful properties save updates titlecache'); + file_put_contents($file, $original); $GLOBALS['config_fault'] = 'rename'; $GLOBALS['__test_nfilter_request'] = ['node_name' => 'note1', 'param' => 'note1', 'x' => '70', 'y' => '80']; { http_response_code(200); moveNode($file, 0); diff --git a/tests/Support/EditorImageFormatsRegression.php b/tests/Support/EditorImageFormatsRegression.php index 3d292fdc..fb59333d 100644 --- a/tests/Support/EditorImageFormatsRegression.php +++ b/tests/Support/EditorImageFormatsRegression.php @@ -107,22 +107,21 @@ function image_format_assert($value, $message) { image_format_assert(in_array(basename($included), wm_asset_image_users('included-image.png', 'backgrounds', $index), true), 'reference snapshot reused without rescanning configurations'); image_format_assert(wm_asset_image_users('included-image.png', 'backgrounds') === [], 'fresh request reflects changed configuration'); image_format_assert(is_array(wm_asset_image_list()), 'unmatched optional include permits listing'); - // Test symlink rejection only inside this isolated fixture, and always restore its library. - $library = $root . '/images'; - $originalLibrary = $temporary . '/library'; - image_format_assert(rename($library, $originalLibrary), 'reserve fixture library'); + // Test directory rejection without renaming any shared checkout assets. + $fixtureRoot = $temporary . '/fixture'; mkdir($fixtureRoot); + $library = $fixtureRoot . '/images'; + image_format_assert(symlink($root . '/images', $library), 'create linked fixture library'); try { - image_format_assert(symlink($originalLibrary, $library), 'create linked fixture library'); foreach (['objects', 'backgrounds'] as $kind) { - try { wm_asset_directory($kind); throw new RuntimeException('accepted symlinked library parent'); } catch (InvalidArgumentException $expected) {} + try { wm_asset_directory($kind, $fixtureRoot); throw new RuntimeException('accepted symlinked library parent'); } catch (InvalidArgumentException $expected) {} } - try { wm_asset_image_list(); throw new RuntimeException('listed linked library'); } catch (InvalidArgumentException $expected) {} - try { wm_asset_store_image('objects', $source, 'linked.png'); throw new RuntimeException('uploaded to linked library'); } catch (InvalidArgumentException $expected) {} - try { wm_asset_image_path('objects', basename($created[0])); throw new RuntimeException('accepted deletion path in linked library'); } catch (InvalidArgumentException $expected) {} - } finally { - if (is_link($library)) unlink($library); - rename($originalLibrary, $library); - } + } finally { unlink($library); rmdir($fixtureRoot); } + $literal = $temporary . '/site[1].conf'; + file_put_contents($literal, "BACKGROUND images/backgrounds/literal-image.png\n"); + file_put_contents($config, "INCLUDE " . $literal . "\n"); + image_format_assert(in_array(basename($literal), wm_asset_image_users('literal-image.png'), true), 'literal brackets in INCLUDE protect referenced images'); + file_put_contents($config, "INCLUDE " . $temporary . "/site*.conf\n"); + image_format_assert(wm_asset_image_users('literal-image.png') === [], 'wildcard text is not expanded beyond renderer semantics'); // A reference-scan failure must be reported before upload validation or persistence. $broken = $temporary . '/broken.conf'; symlink($temporary . '/missing-target.conf', $broken); diff --git a/tests/Support/MapDuplicatePayload.php b/tests/Support/MapDuplicatePayload.php index ad558dd5..64da377e 100644 --- a/tests/Support/MapDuplicatePayload.php +++ b/tests/Support/MapDuplicatePayload.php @@ -14,7 +14,11 @@ if ($start === false || $end === false) { throw new RuntimeException('Unable to locate the map duplication function.'); } -eval(substr($source, $start, $end - $start)); +$fixture = sys_get_temp_dir() . '/wm-duplicate-' . bin2hex(random_bytes(5)); +mkdir($fixture); mkdir($fixture . '/configs'); +symlink(dirname(__DIR__, 2) . '/lib', $fixture . '/lib'); +file_put_contents($fixture . '/configs/Original.conf', "TITLE Original Map\n"); +eval(str_replace('__DIR__', var_export($fixture, true), substr($source, $start, $end - $start))); /** * Return the source map fixture. @@ -92,5 +96,11 @@ function sql_save($payload, $table) { return 0; } -map_duplicate(5, ' Copy'); -print json_encode($GLOBALS['duplicate_save'], JSON_THROW_ON_ERROR); +try { + map_duplicate(5, ' Copy'); + print json_encode($GLOBALS['duplicate_save'], JSON_THROW_ON_ERROR); +} finally { + foreach (glob($fixture . '/configs/*') as $file) unlink($file); + if (is_file($fixture . '/configs/.weathermap-config.lock')) unlink($fixture . '/configs/.weathermap-config.lock'); + unlink($fixture . '/lib'); rmdir($fixture . '/configs'); rmdir($fixture); +} diff --git a/tests/bootstrap-unit.php b/tests/bootstrap-unit.php index de6f6e41..7cebba05 100644 --- a/tests/bootstrap-unit.php +++ b/tests/bootstrap-unit.php @@ -104,6 +104,7 @@ function db_fetch_row_prepared($sql, $p = []) { if (!function_exists('db_fetch_cell')) { function db_fetch_cell($sql) { + if (isset($GLOBALS['__test_db_fetch_cell'])) return ($GLOBALS['__test_db_fetch_cell'])($sql); return ''; } } @@ -339,6 +340,7 @@ function is_error_message() { if (!function_exists('sql_save')) { function sql_save($a, $t, $k = 'id') { + if (isset($GLOBALS['__test_sql_save'])) return ($GLOBALS['__test_sql_save'])($a, $t, $k); return isset($a['id']) ? $a['id'] : 1; } } diff --git a/weathermap-cacti-plugin-mgmt.php b/weathermap-cacti-plugin-mgmt.php index 28d68cde..0edff281 100644 --- a/weathermap-cacti-plugin-mgmt.php +++ b/weathermap-cacti-plugin-mgmt.php @@ -1998,6 +1998,24 @@ function map_duplicate($id, $titlecache, $configfile = null) { $save['duration'] = 0; $save['last_runtime'] = 0; + $confdir = __DIR__ . '/configs'; + + $oldfile = $confdir . '/' . $map['configfile']; + $newfile = $confdir . '/' . $save['configfile']; + + $contents = is_file($oldfile) ? file_get_contents($oldfile) : false; + if ($contents !== false) { + $contents = str_replace("TITLE {$map['titlecache']}", "TITLE {$save['titlecache']}", $contents); + require_once __DIR__ . '/lib/WeatherMap.config.php'; + if (!\WeathermapConfig\atomicWrite($newfile, $contents, false, null, true)) { + raise_message('map_message', __('Could not save the map configuration.', 'weathermap'), MESSAGE_LEVEL_ERROR); + return; + } + } else { + raise_message('map_message', __('The new Map with the name %s was unable to locate the config file %s to copy', $save['titlecache'], $map['configfile'], 'weathermap'), MESSAGE_LEVEL_ERROR); + return; + } + $newid = sql_save($save, 'weathermap_maps'); if ($newid) { @@ -2017,22 +2035,6 @@ function map_duplicate($id, $titlecache, $configfile = null) { raise_message('new_map_' . $newid, __('The new Map with the name %s was created using config file %s', $save['titlecache'], $save['configfile'], 'weathermap'), MESSAGE_LEVEL_INFO); - $confdir = __DIR__ . '/configs'; - - $oldfile = $confdir . '/' . $map['configfile']; - $newfile = $confdir . '/' . $save['configfile']; - - $contents = is_file($oldfile) ? file_get_contents($oldfile) : false; - if ($contents !== false) { - $contents = str_replace("TITLE {$map['titlecache']}", "TITLE {$save['titlecache']}", $contents); - require_once __DIR__ . '/lib/WeatherMap.config.php'; - if (!\WeathermapConfig\atomicWrite($newfile, $contents, false, null, true)) { - raise_message('new_map_' . $newid, __('Could not save the map configuration.', 'weathermap'), MESSAGE_LEVEL_ERROR); - } - } else { - raise_message('new_map_' . $newid, __('The new Map with the name %s was unable to locate the config file %s to copy', $save['titlecache'], $map['configfile'], 'weathermap'), MESSAGE_LEVEL_ERROR); - } - weathermap_run_maps(__DIR__, true, [$newid]); } } @@ -3432,11 +3434,17 @@ function newMap($mapfile, $sourcemapfile = '', $title = '') { if ($mapfile != '') { if ($sourcemapfile != '') { if (file_exists($sourcemapfile) && is_readable($sourcemapfile)) { - $map->ReadConfig($sourcemapfile); + if (!$map->ReadConfig($sourcemapfile)) { + raise_message('map_message', __('The Source Map File name is not readable or does not exist!', 'weathermap'), MESSAGE_LEVEL_ERROR); + return; + } if ($title !== '') { $map->title = $title; } - $map->WriteConfig($mapfile); + if (!$map->WriteConfig($mapfile, true)) { + raise_message('map_message', __('Could not save the map configuration.', 'weathermap'), MESSAGE_LEVEL_ERROR); + return; + } raise_message('map_message', __('New Map file %s created from %s', basename($mapfile), basename($sourcemapfile), 'weathermap'), MESSAGE_LEVEL_INFO); } else { raise_message('map_message', __('The Source Map File name is not readable or does not exist!', 'weathermap'), MESSAGE_LEVEL_ERROR); @@ -3445,7 +3453,10 @@ function newMap($mapfile, $sourcemapfile = '', $title = '') { if ($title !== '') { $map->title = $title; } - $map->WriteConfig($mapfile); + if (!$map->WriteConfig($mapfile, true)) { + raise_message('map_message', __('Could not save the map configuration.', 'weathermap'), MESSAGE_LEVEL_ERROR); + return; + } raise_message('map_message', __('New Map file %s created.', basename($mapfile), 'weathermap'), MESSAGE_LEVEL_INFO); } } else { From d558f0ea9dbf2a4ea4ef73189cddfd24c2fdb23f Mon Sep 17 00:00:00 2001 From: alcatron <27321443+alcatron@users.noreply.github.com> Date: Tue, 13 Oct 2026 09:09:43 +1030 Subject: [PATCH 17/41] Clean failed clone registrations and preserve successful raw edits --- lib/WeatherMap.config.php | 10 +- lib/editor.actions.php | 28 +-- lib/editor.assets.php | 2 +- lib/poller-common.php | 216 ++++++++++-------- tests/Support/ConfigWriteAtomicRegression.php | 77 ++++++- .../Support/EditorImageFormatsRegression.php | 3 + tests/Support/MapDuplicatePayload.php | 5 + weathermap-cacti-plugin-mgmt.php | 57 +++-- 8 files changed, 248 insertions(+), 150 deletions(-) diff --git a/lib/WeatherMap.config.php b/lib/WeatherMap.config.php index b89db55e..0e9377a8 100644 --- a/lib/WeatherMap.config.php +++ b/lib/WeatherMap.config.php @@ -82,17 +82,21 @@ function writeAll($stream, $contents) { * @param bool $mustExist Refuse to recreate an existing map deleted during editing. * @param string|null $expectedRevision SHA-256 of the snapshot being edited. * @param bool $createOnly Refuse to replace an existing destination. + * @param resource|null $heldLock Optional caller-owned mutex for this destination directory. * * @return bool Success; failures preserve the existing configuration. */ -function atomicWrite($filename, $contents, $mustExist = false, $expectedRevision = null, $createOnly = false) { +function atomicWrite($filename, $contents, $mustExist = false, $expectedRevision = null, $createOnly = false, $heldLock = null) { if (strpos($filename, '://') !== false) return false; if (is_link($filename)) $filename = realpath($filename); if ($filename === false || !is_dir(dirname($filename)) || !is_writable(dirname($filename))) return false; if (file_exists($filename) && (!is_file($filename) || !is_writable($filename))) return false; $existed = file_exists($filename); $mustExist = $mustExist || $existed; - $lock = acquire(dirname($filename)); + $ownsLock = $heldLock === null; + $lockPath = realpath(dirname($filename) . '/.weathermap-config.lock'); + if (!$ownsLock && ($lockPath === false || !is_resource($heldLock) || get_resource_type($heldLock) !== 'stream' || realpath(stream_get_meta_data($heldLock)['uri']) !== $lockPath)) return false; + $lock = $ownsLock ? acquire(dirname($filename)) : $heldLock; if (!$lock) return false; $temp = false; $stream = false; @@ -117,6 +121,6 @@ function atomicWrite($filename, $contents, $mustExist = false, $expectedRevision } finally { if (is_resource($stream)) fclose($stream); if ($temp !== false && file_exists($temp)) unlink($temp); - release($lock); + if ($ownsLock) release($lock); } } diff --git a/lib/editor.actions.php b/lib/editor.actions.php index 47d5347e..681220e3 100644 --- a/lib/editor.actions.php +++ b/lib/editor.actions.php @@ -83,7 +83,9 @@ function wm_editor_read_config($map, $mapfile) { */ function wm_editor_write_config($map, $mapfile, $createOnly = false) { if (!$map->WriteConfig($mapfile, $createOnly, $map->editor_config_revision)) { - http_response_code(500); + $current = is_file($mapfile) ? @hash_file('sha256', $mapfile) : false; + $stale = $map->editor_config_revision !== null && $current !== false && $current !== $map->editor_config_revision; + http_response_code($stale ? 409 : 500); return false; } return true; @@ -273,17 +275,7 @@ function setNodeConfig($mapfile) { if (isset($map->nodes[$node_name])) { $map->nodes[$node_name]->config_override = $node_config; - wm_editor_write_config($map, $mapfile); - - // now clear and reload the map object, because the in-memory one is out of sync - // - we don't know what changes the user made here, so we just have to reload. - unset($map); - - $map = new WeatherMap; - - $map->context = 'editor'; - - if (!wm_editor_read_config($map, $mapfile)) return; + if (!wm_editor_write_config($map, $mapfile)) return; } } @@ -304,17 +296,7 @@ function setLinkConfig($mapfile) { if (isset($map->links[$link_name])) { $map->links[$link_name]->config_override = $link_config; - wm_editor_write_config($map, $mapfile); - - // now clear and reload the map object, because the in-memory one is out of sync - // - we don't know what changes the user made here, so we just have to reload. - unset($map); - - $map = new WeatherMap; - - $map->context = 'editor'; - - if (!wm_editor_read_config($map, $mapfile)) return; + if (!wm_editor_write_config($map, $mapfile)) return; } } diff --git a/lib/editor.assets.php b/lib/editor.assets.php index d309715c..a4ca3f14 100644 --- a/lib/editor.assets.php +++ b/lib/editor.assets.php @@ -87,7 +87,7 @@ function wm_asset_image_reference_index() { $queue = []; $root = dirname(__DIR__); $iterator = new RecursiveIteratorIterator(new RecursiveDirectoryIterator($root . '/configs', FilesystemIterator::SKIP_DOTS)); - foreach ($iterator as $file) if ($file->isFile() && $file->getFilename() !== '.weathermap-config.lock' && strpos($file->getFilename(), '.wm-config-') !== 0) $queue[] = $file->getPathname(); + foreach ($iterator as $file) if ($file->isFile() && strtolower($file->getExtension()) === 'conf') $queue[] = $file->getPathname(); $seen = []; while ($queue) { $path = array_pop($queue); diff --git a/lib/poller-common.php b/lib/poller-common.php index 362516f3..57192986 100644 --- a/lib/poller-common.php +++ b/lib/poller-common.php @@ -111,6 +111,19 @@ function weathermap_check_cron($time, $string) { return ($matched); } +/** + * Stage a complete legacy asset copy while keeping every old map reference usable. + * + * @param string $old Source asset path. + * @param string $new New asset path. + * @return bool Whether a complete copy was reserved at the destination. + */ +function weathermap_stage_asset_copy($old, $new) { + require_once __DIR__ . '/WeatherMap.config.php'; + $contents = @file_get_contents($old); + return $contents !== false && \WeathermapConfig\atomicWrite($new, $contents, false, null, true); +} + /** * @return void */ @@ -142,45 +155,54 @@ function weathermap_repair_maps() { wm_debug('---------------------------------------------'); wm_debug("Processing Map File $configfile"); - if (file_exists($configfile) && is_writable($configfile)) { - $contents = file($configfile) ?: []; - $revision = hash('sha256', implode('', $contents)); - - foreach ($contents as $line) { - if (strpos($line, 'BACKGROUND') !== false) { - $parts = explode('BACKGROUND', $line); - $dirgood = false; - $bgfile = trim($parts[1]); + if (!is_file($configfile) || !is_writable($configfile)) continue; + $contents = file($configfile); + if ($contents === false) continue; + $revision = hash('sha256', implode('', $contents)); + require_once __DIR__ . '/WeatherMap.config.php'; + $repairLock = \WeathermapConfig\acquire($confdir); + if (!$repairLock) { wm_warn('Unable to lock repaired configuration: ' . $configfile . ' [WMPOLL13]'); continue; } + try { + if (file_exists($configfile) && is_writable($configfile)) { - if (strpos($bgfile, 'images/backgrounds/') === false) { - wm_debug('Found background file reference in old location: ' . $bgfile); + foreach ($contents as $line) { + if (strpos($line, 'BACKGROUND') !== false) { + $parts = explode('BACKGROUND', $line); + $dirgood = false; + $bgfile = trim($parts[1]); - if (file_exists($mydir . 'images/backgrounds/' . basename($bgfile))) { - wm_debug('Background file found in new location! Updating path only.'); + if (strpos($bgfile, 'images/backgrounds/') === false) { + wm_debug('Found background file reference in old location: ' . $bgfile); - $line = 'BACKGROUND images/backgrounds/' . basename($bgfile); + if (file_exists($mydir . 'images/backgrounds/' . basename($bgfile))) { + wm_debug('Background file found in new location! Updating path only.'); - $changes++; - } else { - wm_debug('Background file not found in new location! Attempting to move.'); + $line = 'BACKGROUND images/backgrounds/' . basename($bgfile); - if (!is_dir($mydir . 'images/backgrounds')) { - $dirgood = mkdir($mydir . 'images/backgrounds'); + $changes++; } else { - $dirgood = true; - } + wm_debug('Background file not found in new location! Attempting to move.'); - if ($dirgood && file_exists($mydir . $bgfile)) { - wm_debug('Found background file on disk: ' . $mydir . $bgfile . ' relocating!'); + if (!is_dir($mydir . 'images/backgrounds')) { + $dirgood = mkdir($mydir . 'images/backgrounds'); + } else { + $dirgood = true; + } + + if ($dirgood && file_exists($mydir . $bgfile)) { + wm_debug('Found background file on disk: ' . $mydir . $bgfile . ' relocating!'); - $old = $mydir . $bgfile; - $new = $mydir . 'images/backgrounds/' . basename($bgfile); + $old = $mydir . $bgfile; + $new = $mydir . 'images/backgrounds/' . basename($bgfile); - if (is_writable($mydir . $bgfile)) { - if (is_writeable($old) && is_writeable(dirname($new)) && is_writeable($new)) { - if (rename($old, $new)) { - $line = 'BACKGROUND images/backgrounds/' . basename($bgfile); - $changes++; + if (is_readable($mydir . $bgfile)) { + if (is_readable($old) && is_writeable(dirname($new))) { + if (weathermap_stage_asset_copy($old, $new)) { + $line = 'BACKGROUND images/backgrounds/' . basename($bgfile); + $changes++; + } else { + wm_warn(sprintf('Unable to move BACKGROUND %s to %s, Permission error [WMPOLL09]', $old, $new)); + } } else { wm_warn(sprintf('Unable to move BACKGROUND %s to %s, Permission error [WMPOLL09]', $old, $new)); } @@ -188,65 +210,65 @@ function weathermap_repair_maps() { wm_warn(sprintf('Unable to move BACKGROUND %s to %s, Permission error [WMPOLL09]', $old, $new)); } } else { - wm_warn(sprintf('Unable to move BACKGROUND %s to %s, Permission error [WMPOLL09]', $old, $new)); + wm_warn(sprintf('BACKGROUND file not found on disk: %s skipping! [WMPOLL10]', $mydir . $bgfile)); } - } else { - wm_warn(sprintf('BACKGROUND file not found on disk: %s skipping! [WMPOLL10]', $mydir . $bgfile)); } + } elseif (strpos($bgfile, 'images/backgrounds/') !== false) { + wm_debug('Found background file reference in correct location: ' . $bgfile); } - } elseif (strpos($bgfile, 'images/backgrounds/') !== false) { - wm_debug('Found background file reference in correct location: ' . $bgfile); - } - } elseif (strpos($line, 'ICON') !== false) { - /** - * We are looking at the ICON parameter. It can take two forms. The first form is - * a path to a filename, or the pattern as follows - * - * ICON height width type - * - * Types include: - * - * - box - a square cornered box - * - rbox - a round cornered box - * - round - a circle or an ellipse - * - inpie & outpie to produce a pie chart of either the in or out value relative to it's maximum - * - nlink - to produce a circular 'yin-yang' style symbox with each half showing the in and out values - * - */ - $parts = explode('ICON', $line); - $dirgood = false; - $attribs = explode(' ', $parts[1]); - - if (cacti_sizeof($attribs) == 1) { - $objfile = trim($parts[1]); - - if (strpos($objfile, 'images/objects/') === false) { - wm_debug('Found icon file reference in old location: ' . $objfile); - - if (file_exists($mydir . 'images/objects/' . basename($objfile))) { - wm_debug('Found file already in new location! Updating path only.'); - - $line = "\tICON images/objects/" . basename($objfile); - - $changes++; - } else { - if (!is_dir($mydir . 'images/objects')) { - $dirgood = mkdir($mydir . 'images/objects'); + } elseif (strpos($line, 'ICON') !== false) { + /** + * We are looking at the ICON parameter. It can take two forms. The first form is + * a path to a filename, or the pattern as follows + * + * ICON height width type + * + * Types include: + * + * - box - a square cornered box + * - rbox - a round cornered box + * - round - a circle or an ellipse + * - inpie & outpie to produce a pie chart of either the in or out value relative to it's maximum + * - nlink - to produce a circular 'yin-yang' style symbox with each half showing the in and out values + * + */ + $parts = explode('ICON', $line); + $dirgood = false; + $attribs = preg_split('/\s+/', trim($parts[1])); + + if (cacti_sizeof($attribs) == 1) { + $objfile = trim($parts[1]); + + if (strpos($objfile, 'images/objects/') === false) { + wm_debug('Found icon file reference in old location: ' . $objfile); + + if (file_exists($mydir . 'images/objects/' . basename($objfile))) { + wm_debug('Found file already in new location! Updating path only.'); + + $line = "\tICON images/objects/" . basename($objfile); + + $changes++; } else { - $dirgood = true; - } + if (!is_dir($mydir . 'images/objects')) { + $dirgood = mkdir($mydir . 'images/objects'); + } else { + $dirgood = true; + } - if ($dirgood && file_exists($mydir . $objfile)) { - wm_debug('Object file found on disk: ' . $mydir . $objfile . ' relocating!'); + if ($dirgood && file_exists($mydir . $objfile)) { + wm_debug('Object file found on disk: ' . $mydir . $objfile . ' relocating!'); - $old = $mydir . $objfile; - $new = $mydir . 'images/objects/' . basename($objfile); + $old = $mydir . $objfile; + $new = $mydir . 'images/objects/' . basename($objfile); - if (is_writable($mydir . $objfile)) { - if (is_writeable($old) && is_writeable(dirname($new)) && is_writeable($new)) { - if (rename($old, $new)) { - $line = "\tICON images/objects/" . basename($objfile); - $changes++; + if (is_readable($mydir . $objfile)) { + if (is_readable($old) && is_writeable(dirname($new))) { + if (weathermap_stage_asset_copy($old, $new)) { + $line = "\tICON images/objects/" . basename($objfile); + $changes++; + } else { + wm_warn(sprintf('Unable to move ICONFILE %s to %s, Permission error [WMPOLL11]', $old, $new)); + } } else { wm_warn(sprintf('Unable to move ICONFILE %s to %s, Permission error [WMPOLL11]', $old, $new)); } @@ -254,31 +276,29 @@ function weathermap_repair_maps() { wm_warn(sprintf('Unable to move ICONFILE %s to %s, Permission error [WMPOLL11]', $old, $new)); } } else { - wm_warn(sprintf('Unable to move ICONFILE %s to %s, Permission error [WMPOLL11]', $old, $new)); + wm_warn(sprintf('ICONFILE file not found on disk: %s skipping! [WMPOLL12]', $mydir . $objfile)); } - } else { - wm_warn(sprintf('ICONFILE file not found on disk: %s skipping! [WMPOLL12]', $mydir . $objfile)); } + } elseif (strpos($objfile, 'images/objects/') !== false) { + wm_debug('Found object file reference in correct location: ' . $objfile); } - } elseif (strpos($objfile, 'images/objects/') !== false) { - wm_debug('Found object file reference in correct location: ' . $objfile); } } - } - $outcontents[] = rtrim($line); + $outcontents[] = rtrim($line); + } } - } - if ($changes) { - wm_debug('Writing updated file to: ' . $configfile); - require_once __DIR__ . '/WeatherMap.config.php'; - if (!\WeathermapConfig\atomicWrite($configfile, implode("\n", $outcontents), true, $revision)) { - wm_warn('Unable to save repaired configuration: ' . $configfile . ' [WMPOLL13]'); + if ($changes) { + wm_debug('Writing updated file to: ' . $configfile); + require_once __DIR__ . '/WeatherMap.config.php'; + if (!\WeathermapConfig\atomicWrite($configfile, implode("\n", $outcontents), true, $revision, false, $repairLock)) { + wm_warn('Unable to save repaired configuration: ' . $configfile . ' [WMPOLL13]'); + } + } else { + wm_debug('File has correct BACKGROUND and ICON paths: ' . $configfile); } - } else { - wm_debug('File has correct BACKGROUND and ICON paths: ' . $configfile); - } + } finally { \WeathermapConfig\release($repairLock); } } } } diff --git a/tests/Support/ConfigWriteAtomicRegression.php b/tests/Support/ConfigWriteAtomicRegression.php index aecd9104..6ddd100a 100644 --- a/tests/Support/ConfigWriteAtomicRegression.php +++ b/tests/Support/ConfigWriteAtomicRegression.php @@ -225,7 +225,7 @@ function atomic_assert($condition, $message) { $raw->ReadConfig("TITLE Fragment edit\n"); file_put_contents($file, $original); http_response_code(200); - atomic_assert(!wm_editor_write_config($raw, $file) && http_response_code() === 500 && file_get_contents($file) === $original, 'raw fragment edits retain the original request revision'); + atomic_assert(!wm_editor_write_config($raw, $file) && http_response_code() === 409 && file_get_contents($file) === $original, 'raw fragment edits retain the original request revision'); if (function_exists('pcntl_fork')) { $createFile = $dir . '/create.conf'; $lock = \WeathermapConfig\acquire($dir); $children = []; @@ -246,6 +246,17 @@ function atomic_assert($condition, $message) { atomic_assert((int)file_get_contents($dir . '/create-result-0') + (int)file_get_contents($dir . '/create-result-1') === 1, 'exactly one concurrent creator commits'); atomic_assert(in_array(file_get_contents($createFile), ['Creator 0', 'Creator 1'], true), 'winning creator content remains intact'); } + $held = \WeathermapConfig\acquire($dir); + atomic_assert(\WeathermapConfig\atomicWrite($dir . '/held.conf', 'Held write', false, null, true, $held), 'caller-owned mutex supports atomic creation'); + if (function_exists('pcntl_fork')) { + $child = pcntl_fork(); + if ($child === 0) { $probe = \fopen($dir . '/.weathermap-config.lock', 'rb'); $busy = !\flock($probe, LOCK_EX | LOCK_NB); exit($busy ? 0 : 1); } + pcntl_waitpid($child, $status); + atomic_assert(pcntl_wexitstatus($status) === 0, 'atomicWrite leaves caller mutex held'); + } + \WeathermapConfig\release($held); + $wrong = \fopen($file, 'rb'); + atomic_assert(!\WeathermapConfig\atomicWrite($dir . '/wrong-lock.conf', 'Wrong', false, null, true, $wrong), 'different lock file rejected'); \fclose($wrong); $crlfFile = $dir . '/crlf.conf'; $crlf = "WIDTH 600\r\nHEIGHT 300\r\n"; file_put_contents($crlfFile, $crlf); $crlfMap = new \WeatherMap; $crlfMap->ReadConfig($crlfFile); @@ -288,6 +299,33 @@ function atomic_assert($condition, $message) { unset($GLOBALS['__test_db_fetch_assoc']); unlink($repairFile); unlink($repairImage); } + $stageSource = $plugin . '/legacy-stage-' . bin2hex(random_bytes(4)) . '.png'; + $stageTarget = $plugin . '/images/backgrounds/' . basename($stageSource); + file_put_contents($stageSource, 'Legacy asset bytes'); + $stageConfig = $plugin . '/configs/' . basename($stageSource) . '.conf'; + $stageLegacy = 'BACKGROUND ' . basename($stageSource) . "\n"; + file_put_contents($stageConfig, $stageLegacy); + $GLOBALS['__test_db_fetch_assoc'] = fn () => [['configfile' => basename($stageConfig)]]; + $oldConfig = $GLOBALS['config']; $GLOBALS['config']['base_path'] = dirname($plugin, 2); + $oldSuppress = $GLOBALS['weathermap_error_suppress'] ?? []; + try { + // A staged copy succeeds, but the config CAS must fail after a concurrent save. + $staged = weathermap_stage_asset_copy($stageSource, $stageTarget); + atomic_assert($staged && is_file($stageSource) && file_get_contents($stageTarget) === 'Legacy asset bytes', 'staging keeps original and complete destination'); + $stageNewer = "TITLE Concurrent repair edit\n" . $stageLegacy; + $GLOBALS['config_before_lock'] = function() use ($stageConfig, $stageNewer) { unset($GLOBALS['config_before_lock']); file_put_contents($stageConfig, $stageNewer); }; + $GLOBALS['weathermap_error_suppress'] = ['WMPOLL13']; weathermap_repair_maps(); + atomic_assert(file_get_contents($stageConfig) === $stageNewer && is_file($stageSource), 'rejected repair leaves original asset usable'); + $GLOBALS['config_fault'] = 'rename'; weathermap_repair_maps(); + atomic_assert(file_get_contents($stageConfig) === $stageNewer && is_file($stageSource), 'I/O failure leaves original asset usable'); + $GLOBALS['config_fault'] = ''; unlink($stageTarget); file_put_contents($stageConfig, $stageLegacy); + weathermap_repair_maps(); + atomic_assert(str_contains(file_get_contents($stageConfig), 'images/backgrounds/') && is_file($stageSource) && is_file($stageTarget), 'successful relocation copies rather than removes legacy asset'); + } finally { + $GLOBALS['config'] = $oldConfig; $GLOBALS['weathermap_error_suppress'] = $oldSuppress; $GLOBALS['config_fault'] = ''; + unset($GLOBALS['__test_db_fetch_assoc'], $GLOBALS['config_before_lock']); + foreach ([$stageSource, $stageTarget, $stageConfig] as $path) if (is_file($path)) unlink($path); + } $management = file_get_contents($plugin . '/weathermap-cacti-plugin-mgmt.php'); $start = strpos($management, 'function newMap('); eval(str_replace(['__DIR__', 'function newMap('], [var_export($plugin, true), 'function wm_management_newMap('], substr($management, $start))); @@ -313,6 +351,43 @@ function atomic_assert($condition, $message) { map_duplicate(1, 'Clone', 'clone'); atomic_assert($GLOBALS['__test_db_calls'] === [] && $GLOBALS['atomic_sql_saves'] === [], 'failed clone never registers auth or settings or invokes runner'); } finally { unset($GLOBALS['__test_db_fetch_row_prepared'], $GLOBALS['__test_sql_save'], $GLOBALS['__test_db_fetch_cell']); unlink($plugin . '/configs/' . $duplicateSource); $GLOBALS['config_fault'] = ''; } + // Real raw node/link saves must not re-check their own pre-save page token. + $rawFile = $dir . '/raw.conf'; + $rawConfig = "WIDTH 300\nHEIGHT 200\nNODE A\n LABEL A\n POSITION 10 10\nNODE B\n LABEL B\n POSITION 50 50\nLINK AB\n NODES A B\n"; + foreach (['node', 'link'] as $kind) { + file_put_contents($rawFile, $rawConfig); + $GLOBALS['__test_nfilter_request'] = ['config_revision' => hash_file('sha256', $rawFile), 'node_name' => 'A', 'link_name' => 'AB', 'item_configtext' => $kind === 'node' ? "NODE A\n LABEL Updated\n POSITION 10 10\n" : "LINK AB\n NODES A B\n WIDTH 12\n"]; + http_response_code(200); if ($kind === 'node') setNodeConfig($rawFile); else setLinkConfig($rawFile); + atomic_assert(http_response_code() === 200 && file_get_contents($rawFile) !== $rawConfig, 'raw ' . $kind . ' edit saves without false conflict'); + } + // Registration failure must retain a mutex through the callback and remove its reservation. + file_put_contents($plugin . '/configs/' . $duplicateSource, "TITLE Source\n"); + $GLOBALS['__test_db_fetch_row_prepared'] = fn () => ['configfile' => $duplicateSource, 'titlecache' => 'Source', 'group_id' => 1, 'active' => 'on', 'thumb_height' => 10, 'thumb_width' => 10, 'schedule' => '', 'archiving' => '']; + $GLOBALS['__test_db_fetch_cell'] = fn () => 0; + $GLOBALS['weathermap_confdir'] = $plugin . '/configs'; + try { + foreach (['false', 'throw', 'auth-false', 'settings-false', 'auth-throw', 'settings-throw'] as $failure) { + $GLOBALS['__test_sql_save'] = function($record) use ($plugin, $failure) { + atomic_assert(is_file($plugin . '/configs/' . $record['configfile']), 'reserved file exists during registration'); + if (function_exists('pcntl_fork')) { + $child = pcntl_fork(); + if ($child === 0) { $probe = \fopen($plugin . '/configs/.weathermap-config.lock', 'rb'); exit(!\flock($probe, LOCK_EX | LOCK_NB) ? 0 : 1); } + pcntl_waitpid($child, $status); atomic_assert(pcntl_wexitstatus($status) === 0, 'registration retains directory mutex'); + } + if ($failure === 'throw') throw new RuntimeException('registration failed'); + return $failure === 'false' ? false : 987; + }; + $GLOBALS['__test_db_calls'] = []; + $GLOBALS['__test_db_execute_prepared'] = function($sql, $params) use ($failure) { + $GLOBALS['__test_db_calls'][] = ['sql' => $sql, 'params' => $params]; + if ((str_contains($sql, 'INSERT INTO weathermap_auth') && $failure === 'auth-throw') || (str_contains($sql, 'INSERT INTO weathermap_settings') && $failure === 'settings-throw')) throw new RuntimeException('insert failed'); + return !((str_contains($sql, 'INSERT INTO weathermap_auth') && $failure === 'auth-false') || (str_contains($sql, 'INSERT INTO weathermap_settings') && $failure === 'settings-false')); + }; + map_duplicate(1, 'Clone', 'clone'); + atomic_assert(!is_file($plugin . '/configs/clone.conf'), 'failed registration removes its own reserved configuration'); + if (str_ends_with($failure, '-false') || str_ends_with($failure, '-throw')) atomic_assert(cacti_count(array_filter($GLOBALS['__test_db_calls'], fn ($call) => str_starts_with($call['sql'], 'DELETE'))) === 3, 'partial registration removes map/auth/settings records'); + } + } finally { unset($GLOBALS['__test_db_fetch_row_prepared'], $GLOBALS['__test_sql_save'], $GLOBALS['__test_db_fetch_cell'], $GLOBALS['__test_db_execute_prepared']); unlink($plugin . '/configs/' . $duplicateSource); } $deletedMap = new \WeatherMap; $deletedMap->ReadConfig($file); unlink($file); atomic_assert(!$deletedMap->WriteConfig($file) && !file_exists($file), 'saving a previously loaded deleted map fails without resurrection'); diff --git a/tests/Support/EditorImageFormatsRegression.php b/tests/Support/EditorImageFormatsRegression.php index fb59333d..21d5d3db 100644 --- a/tests/Support/EditorImageFormatsRegression.php +++ b/tests/Support/EditorImageFormatsRegression.php @@ -100,6 +100,9 @@ function image_format_assert($value, $message) { file_put_contents($included, "BACKGROUND images/backgrounds/included-image.png\n"); file_put_contents($config, "INCLUDE " . $included . "\n"); image_format_assert(in_array(basename($included), wm_asset_image_users('included-image.png'), true), 'included configurations protect images from deletion'); + $backup = $root . '/configs/ignored-backup-' . bin2hex(random_bytes(4)) . '.bak'; + file_put_contents($backup, "ICON images/objects/backup-only.png\n"); + try { image_format_assert(wm_asset_image_users('backup-only.png') === [], 'backup files are not map configurations'); } finally { unlink($backup); } // Missing optional includes must not disable the library, and one index serves all names. file_put_contents($config, "INCLUDE " . $temporary . "/missing-*.conf\nINCLUDE " . $included . "\n"); $index = wm_asset_image_reference_index(); diff --git a/tests/Support/MapDuplicatePayload.php b/tests/Support/MapDuplicatePayload.php index 64da377e..3e6dce8d 100644 --- a/tests/Support/MapDuplicatePayload.php +++ b/tests/Support/MapDuplicatePayload.php @@ -96,6 +96,11 @@ function sql_save($payload, $table) { return 0; } +/** Ignore expected registration failure notices in this schema-only fixture. */ +function raise_message($id, $text = '', $level = 0) {} +/** Return the untranslated fixture string. */ +function __($text, ...$args) { return $text; } +define('MESSAGE_LEVEL_ERROR', 3); try { map_duplicate(5, ' Copy'); print json_encode($GLOBALS['duplicate_save'], JSON_THROW_ON_ERROR); diff --git a/weathermap-cacti-plugin-mgmt.php b/weathermap-cacti-plugin-mgmt.php index 0edff281..f6851781 100644 --- a/weathermap-cacti-plugin-mgmt.php +++ b/weathermap-cacti-plugin-mgmt.php @@ -2003,40 +2003,49 @@ function map_duplicate($id, $titlecache, $configfile = null) { $oldfile = $confdir . '/' . $map['configfile']; $newfile = $confdir . '/' . $save['configfile']; - $contents = is_file($oldfile) ? file_get_contents($oldfile) : false; - if ($contents !== false) { - $contents = str_replace("TITLE {$map['titlecache']}", "TITLE {$save['titlecache']}", $contents); - require_once __DIR__ . '/lib/WeatherMap.config.php'; - if (!\WeathermapConfig\atomicWrite($newfile, $contents, false, null, true)) { - raise_message('map_message', __('Could not save the map configuration.', 'weathermap'), MESSAGE_LEVEL_ERROR); - return; - } - } else { - raise_message('map_message', __('The new Map with the name %s was unable to locate the config file %s to copy', $save['titlecache'], $map['configfile'], 'weathermap'), MESSAGE_LEVEL_ERROR); + require_once __DIR__ . '/lib/WeatherMap.config.php'; + $lock = \WeathermapConfig\acquire($confdir); + if (!$lock) { + raise_message('map_message', __('Could not lock map configurations.', 'weathermap'), MESSAGE_LEVEL_ERROR); return; } + $newid = 0; + $created = false; + try { + $contents = is_file($oldfile) ? file_get_contents($oldfile) : false; + if ($contents === false) throw new RuntimeException(__('The new Map with the name %s was unable to locate the config file %s to copy', $save['titlecache'], $map['configfile'], 'weathermap')); + $contents = str_replace("TITLE {$map['titlecache']}", "TITLE {$save['titlecache']}", $contents); + if (!\WeathermapConfig\atomicWrite($newfile, $contents, false, null, true, $lock)) throw new RuntimeException(__('Could not save the map configuration.', 'weathermap')); + $created = true; + $newid = sql_save($save, 'weathermap_maps'); + if (!$newid) throw new RuntimeException(__('Could not register the new map.', 'weathermap')); - $newid = sql_save($save, 'weathermap_maps'); - - if ($newid) { - db_execute_prepared("INSERT INTO weathermap_auth + if (db_execute_prepared("INSERT INTO weathermap_auth (userid, mapid) SELECT userid, '$newid' AS mapid FROM weathermap_auth - WHERE mapid = ?", - [$id]); - - db_execute_prepared("INSERT INTO weathermap_settings + WHERE mapid = ?", [$id]) === false) throw new RuntimeException(__('Could not register the new map.', 'weathermap')); + if (db_execute_prepared("INSERT INTO weathermap_settings (mapid, groupid, optname, optvalue) SELECT '$newid' AS mapid, groupid, optname, optvalue FROM weathermap_settings - WHERE mapid = ?", - [$id]); - - raise_message('new_map_' . $newid, __('The new Map with the name %s was created using config file %s', $save['titlecache'], $save['configfile'], 'weathermap'), MESSAGE_LEVEL_INFO); - - weathermap_run_maps(__DIR__, true, [$newid]); + WHERE mapid = ?", [$id]) === false) throw new RuntimeException(__('Could not register the new map.', 'weathermap')); + } catch (Throwable $error) { + // Only this attempt's records and successfully reserved file may be removed. + try { + if ($newid) { + db_execute_prepared('DELETE FROM weathermap_auth WHERE mapid = ?', [$newid]); + db_execute_prepared('DELETE FROM weathermap_settings WHERE mapid = ?', [$newid]); + db_execute_prepared('DELETE FROM weathermap_maps WHERE id = ?', [$newid]); + } + } finally { if ($created) unlink($newfile); } + raise_message('map_message', $error->getMessage(), MESSAGE_LEVEL_ERROR); + return; + } finally { + \WeathermapConfig\release($lock); } + raise_message('new_map_' . $newid, __('The new Map with the name %s was created using config file %s', $save['titlecache'], $save['configfile'], 'weathermap'), MESSAGE_LEVEL_INFO); + weathermap_run_maps(__DIR__, true, [$newid]); } } From f195c65ebfbbb6bb71cdc415458207e0a152ef97 Mon Sep 17 00:00:00 2001 From: alcatron <27321443+alcatron@users.noreply.github.com> Date: Tue, 13 Oct 2026 09:10:17 +1030 Subject: [PATCH 18/41] Refresh translations for registration failure reporting --- locales/po/cacti.pot | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/locales/po/cacti.pot b/locales/po/cacti.pot index fd3d1932..a9624bf1 100644 --- a/locales/po/cacti.pot +++ b/locales/po/cacti.pot @@ -8,7 +8,7 @@ msgid "" msgstr "" "Project-Id-Version: Cacti 1.7\n" "Report-Msgid-Bugs-To: developers@cacti.net\n" -"POT-Creation-Date: 2026-10-12 23:20+1030\n" +"POT-Creation-Date: 2026-10-13 09:09+1030\n" "PO-Revision-Date: YEAR-MO-DA HO:MI+ZONE\n" "Last-Translator: Cacti Developers \n" "Language-Team: Cacti Developers \n" @@ -26,7 +26,7 @@ msgstr "" msgid "Use POST to save changes." msgstr "" -#: lib/editor.actions.php +#: lib/editor.actions.php weathermap-cacti-plugin-mgmt.php msgid "Could not lock map configurations." msgstr "" @@ -1937,13 +1937,17 @@ msgstr "" msgid "(No Title)" msgstr "" +#: weathermap-cacti-plugin-mgmt.php +#, php-format +msgid "The new Map with the name %s was unable to locate the config file %s to copy" +msgstr "" + #: weathermap-cacti-plugin-mgmt.php msgid "Could not save the map configuration." msgstr "" #: weathermap-cacti-plugin-mgmt.php -#, php-format -msgid "The new Map with the name %s was unable to locate the config file %s to copy" +msgid "Could not register the new map." msgstr "" #: weathermap-cacti-plugin-mgmt.php From 858c245819c2d221fd14e50fc0cdf450767c55a9 Mon Sep 17 00:00:00 2001 From: alcatron <27321443+alcatron@users.noreply.github.com> Date: Tue, 13 Oct 2026 09:13:59 +1030 Subject: [PATCH 19/41] Cover legacy background and icon repair failure paths --- tests/Support/ConfigWriteAtomicRegression.php | 34 +++++++++++++++++++ 1 file changed, 34 insertions(+) diff --git a/tests/Support/ConfigWriteAtomicRegression.php b/tests/Support/ConfigWriteAtomicRegression.php index 6ddd100a..23fe13c2 100644 --- a/tests/Support/ConfigWriteAtomicRegression.php +++ b/tests/Support/ConfigWriteAtomicRegression.php @@ -326,6 +326,40 @@ function atomic_assert($condition, $message) { unset($GLOBALS['__test_db_fetch_assoc'], $GLOBALS['config_before_lock']); foreach ([$stageSource, $stageTarget, $stageConfig] as $path) if (is_file($path)) unlink($path); } + // Exercise legacy background and icon relocation in a separate filesystem tree. + $repairRoot = $dir . '/repair-fixture'; + $repairPlugin = $repairRoot . '/cacti/plugins/weathermap'; + mkdir($repairPlugin . '/configs', 0777, true); mkdir($repairPlugin . '/images'); + symlink($plugin . '/lib', $repairPlugin . '/lib'); + file_put_contents($repairPlugin . '/legacy-background.png', 'Background bytes'); + file_put_contents($repairPlugin . '/legacy-icon.png', 'Icon bytes'); + $legacyFixture = "BACKGROUND legacy-background.png\nNODE Legacy\n ICON legacy-icon.png\n"; + $fixtureConfig = $repairPlugin . '/configs/legacy.conf'; + $oldConfig = $GLOBALS['config']; $oldSuppress = $GLOBALS['weathermap_error_suppress'] ?? []; + $GLOBALS['config']['base_path'] = $repairRoot . '/cacti'; + $GLOBALS['__test_db_fetch_assoc'] = fn () => [['configfile' => 'legacy.conf']]; + $GLOBALS['weathermap_error_suppress'] = ['WMPOLL09', 'WMPOLL10', 'WMPOLL11', 'WMPOLL12', 'WMPOLL13']; + try { + file_put_contents($fixtureConfig, $legacyFixture); weathermap_repair_maps(); + $repairedFixture = file_get_contents($fixtureConfig); + atomic_assert(str_contains($repairedFixture, 'BACKGROUND images/backgrounds/legacy-background.png') && str_contains($repairedFixture, 'ICON images/objects/legacy-icon.png'), 'repair creates libraries and copies both legacy asset kinds'); + atomic_assert(is_file($repairPlugin . '/legacy-background.png') && is_file($repairPlugin . '/legacy-icon.png'), 'successful repair preserves both original assets'); + file_put_contents($fixtureConfig, $legacyFixture); weathermap_repair_maps(); + atomic_assert(file_get_contents($fixtureConfig) === $repairedFixture, 'existing complete copies support path-only repair'); + weathermap_repair_maps(); atomic_assert(file_get_contents($fixtureConfig) === $repairedFixture, 'correct paths require no rewrite'); + unlink($repairPlugin . '/images/backgrounds/legacy-background.png'); unlink($repairPlugin . '/images/objects/legacy-icon.png'); + file_put_contents($fixtureConfig, $legacyFixture); $GLOBALS['config_fault'] = 'rename'; weathermap_repair_maps(); + atomic_assert(file_get_contents($fixtureConfig) === $legacyFixture && is_file($repairPlugin . '/legacy-background.png') && is_file($repairPlugin . '/legacy-icon.png'), 'failed background and icon staging preserves usable original references'); + $GLOBALS['config_fault'] = ''; file_put_contents($fixtureConfig, "BACKGROUND absent-background.png\nNODE Missing\n ICON absent-icon.png\n"); + $missingFixture = file_get_contents($fixtureConfig); weathermap_repair_maps(); + atomic_assert(file_get_contents($fixtureConfig) === $missingFixture, 'missing assets do not change config paths'); + } finally { + $GLOBALS['config'] = $oldConfig; $GLOBALS['weathermap_error_suppress'] = $oldSuppress; $GLOBALS['config_fault'] = ''; + unset($GLOBALS['__test_db_fetch_assoc']); chdir($plugin); + $iterator = new \RecursiveIteratorIterator(new \RecursiveDirectoryIterator($repairRoot, \FilesystemIterator::SKIP_DOTS), \RecursiveIteratorIterator::CHILD_FIRST); + foreach ($iterator as $entry) { if ($entry->isDir() && !$entry->isLink()) rmdir($entry->getPathname()); else unlink($entry->getPathname()); } + rmdir($repairRoot); + } $management = file_get_contents($plugin . '/weathermap-cacti-plugin-mgmt.php'); $start = strpos($management, 'function newMap('); eval(str_replace(['__DIR__', 'function newMap('], [var_export($plugin, true), 'function wm_management_newMap('], substr($management, $start))); From e59aba91e8312f47c7c554ae9ae54d833673c3e3 Mon Sep 17 00:00:00 2001 From: alcatron <27321443+alcatron@users.noreply.github.com> Date: Tue, 13 Oct 2026 13:14:38 +1030 Subject: [PATCH 20/41] Bind clone IDs in prepared registration statements --- lib/WeatherMap.class.php | 5 +++-- tests/Support/ConfigWriteAtomicRegression.php | 1 + weathermap-cacti-plugin-mgmt.php | 8 ++++---- 3 files changed, 8 insertions(+), 6 deletions(-) diff --git a/lib/WeatherMap.class.php b/lib/WeatherMap.class.php index 7bdd2db4..0e2e09f1 100644 --- a/lib/WeatherMap.class.php +++ b/lib/WeatherMap.class.php @@ -3612,9 +3612,10 @@ function WriteDataFile($filename) { * @param string $filename * @param bool $createOnly Require a new destination. * @param string|null $expectedRevision Optional request snapshot revision. + * @param resource|null $heldLock Caller-owned destination directory mutex. * @return bool */ - function WriteConfig($filename, $createOnly = false, $expectedRevision = null) { + function WriteConfig($filename, $createOnly = false, $expectedRevision = null, $heldLock = null) { require_once __DIR__ . '/WeatherMap.config.php'; $fd = fopen('php://temp', 'w+b'); @@ -3856,7 +3857,7 @@ function WriteConfig($filename, $createOnly = false, $expectedRevision = null) { $closed = fclose($fd); $sameFile = $this->configfile !== '' && $filename === $this->configfile; - $saved = $write_ok && $contents !== false && $closed && \WeathermapConfig\atomicWrite($filename, $contents, $sameFile, $expectedRevision ?? ($sameFile ? $this->config_revision : null), $createOnly); + $saved = $write_ok && $contents !== false && $closed && \WeathermapConfig\atomicWrite($filename, $contents, $sameFile, $expectedRevision ?? ($sameFile ? $this->config_revision : null), $createOnly, $heldLock); if ($saved && $sameFile) $this->config_revision = hash('sha256', $contents); return $saved; } else { diff --git a/tests/Support/ConfigWriteAtomicRegression.php b/tests/Support/ConfigWriteAtomicRegression.php index 23fe13c2..04ce4983 100644 --- a/tests/Support/ConfigWriteAtomicRegression.php +++ b/tests/Support/ConfigWriteAtomicRegression.php @@ -414,6 +414,7 @@ function atomic_assert($condition, $message) { $GLOBALS['__test_db_calls'] = []; $GLOBALS['__test_db_execute_prepared'] = function($sql, $params) use ($failure) { $GLOBALS['__test_db_calls'][] = ['sql' => $sql, 'params' => $params]; + if (str_starts_with($sql,'INSERT')) atomic_assert(!str_contains($sql, '987') && $params === [987,1], 'clone IDs are bound as prepared parameters'); if ((str_contains($sql, 'INSERT INTO weathermap_auth') && $failure === 'auth-throw') || (str_contains($sql, 'INSERT INTO weathermap_settings') && $failure === 'settings-throw')) throw new RuntimeException('insert failed'); return !((str_contains($sql, 'INSERT INTO weathermap_auth') && $failure === 'auth-false') || (str_contains($sql, 'INSERT INTO weathermap_settings') && $failure === 'settings-false')); }; diff --git a/weathermap-cacti-plugin-mgmt.php b/weathermap-cacti-plugin-mgmt.php index f6851781..acabe0f4 100644 --- a/weathermap-cacti-plugin-mgmt.php +++ b/weathermap-cacti-plugin-mgmt.php @@ -2022,14 +2022,14 @@ function map_duplicate($id, $titlecache, $configfile = null) { if (db_execute_prepared("INSERT INTO weathermap_auth (userid, mapid) - SELECT userid, '$newid' AS mapid + SELECT userid, ? AS mapid FROM weathermap_auth - WHERE mapid = ?", [$id]) === false) throw new RuntimeException(__('Could not register the new map.', 'weathermap')); + WHERE mapid = ?", [$newid, $id]) === false) throw new RuntimeException(__('Could not register the new map.', 'weathermap')); if (db_execute_prepared("INSERT INTO weathermap_settings (mapid, groupid, optname, optvalue) - SELECT '$newid' AS mapid, groupid, optname, optvalue + SELECT ? AS mapid, groupid, optname, optvalue FROM weathermap_settings - WHERE mapid = ?", [$id]) === false) throw new RuntimeException(__('Could not register the new map.', 'weathermap')); + WHERE mapid = ?", [$newid, $id]) === false) throw new RuntimeException(__('Could not register the new map.', 'weathermap')); } catch (Throwable $error) { // Only this attempt's records and successfully reserved file may be removed. try { From 35b0d2afbfde653a7b2dee29527c3ea19f5ce8ba Mon Sep 17 00:00:00 2001 From: alcatron <27321443+alcatron@users.noreply.github.com> Date: Tue, 13 Oct 2026 13:54:15 +1030 Subject: [PATCH 21/41] Render controls and revisions from one editor snapshot --- js/editor.js | 33 +++++++++---------- lib/editor.actions.php | 16 +++++---- .../Browser/EditorFilenameNavigationTest.cjs | 12 ++++--- tests/Browser/EditorSaveFailureTest.cjs | 1 + tests/Support/ConfigWriteAtomicRegression.php | 19 +++++++++++ weathermap-cacti-plugin-editor.php | 5 +-- 6 files changed, 56 insertions(+), 30 deletions(-) diff --git a/js/editor.js b/js/editor.js index 0ce9fd0e..347ec39c 100644 --- a/js/editor.js +++ b/js/editor.js @@ -871,6 +871,19 @@ function delete_link() { }); } +function wmEditorReloadUrl() { + var url = new URL(window.location.href); + url.searchParams.set('mapname', $('#mapname').val()); + url.searchParams.set('action', 'nothing'); + url.searchParams.delete('config_revision'); + url.searchParams.delete('__csrf_magic'); + return url.href; +} + +function wmReloadEditor() { + window.location.replace(wmEditorReloadUrl()); +} + function form_submit() { var data = $('input, select, textarea').serialize(); if (typeof WMConfigRevision !== 'undefined') data += '&config_revision=' + encodeURIComponent(WMConfigRevision); @@ -880,28 +893,14 @@ function form_submit() { url: editor_url, data: data, error: function(xhr) { window.alert(xhr && xhr.status === 409 ? wmEditorText.staleError : wmEditorText.saveError); }, - success: function(html) { + success: function() { hide_all_dialogs(); - - $.get('?action=load_area_data&mapname=' + encodeURIComponent($('#mapname').val()), function(data) { - $('.mapData').empty().html(data); - - $.getScript('?action=load_map_javascript&mapname=' + encodeURIComponent($('#mapname').val()), function(data) { - var date = new Date(); - - // Reload the images to update page - $('#existingdata').attr('src', $('#existingdata').attr('src') + '&date=' + date.getTime()); - $('#xycapture').attr('src', $('#xycapture').attr('src') + '&date=' + date.getTime()); - - $('#action').val(''); - - initJS(); - }); - }); + wmReloadEditor(); } }); } + function map_properties() { mapmode('existing'); diff --git a/lib/editor.actions.php b/lib/editor.actions.php index 681220e3..f2a9166a 100644 --- a/lib/editor.actions.php +++ b/lib/editor.actions.php @@ -64,6 +64,10 @@ function wm_editor_read_config($map, $mapfile) { http_response_code(422); return false; } + if (!is_string($map->config_revision)) { + http_response_code(404); + return false; + } $revision = get_nfilter_request_var('config_revision'); if ($revision !== null && $revision !== '' && (!is_string($revision) || !hash_equals($map->config_revision, $revision))) { http_response_code(409); @@ -138,12 +142,12 @@ function newMapCopy($mapfile) { * @param mixed $mapfile * @return void */ -function getMapJavaScript($mapfile) { - $map = new WeatherMap; - - $map->context = 'editor'; - - if (!wm_editor_read_config($map, $mapfile)) return; +function getMapJavaScript($mapfile, $map = null) { + if ($map === null) { + $map = new WeatherMap; + $map->context = 'editor'; + if (!wm_editor_read_config($map, $mapfile)) return; + } print $map->asJS(); print 'var WMConfigRevision = ' . json_encode($map->config_revision) . ';'; diff --git a/tests/Browser/EditorFilenameNavigationTest.cjs b/tests/Browser/EditorFilenameNavigationTest.cjs index b62ea983..a64ef372 100644 --- a/tests/Browser/EditorFilenameNavigationTest.cjs +++ b/tests/Browser/EditorFilenameNavigationTest.cjs @@ -4,7 +4,7 @@ const fs = require('node:fs'); const path = require('node:path'); const vm = require('node:vm'); const assert = require('node:assert/strict'); -const dom = new JSDOM('
', {runScripts: 'outside-only'}); +const dom = new JSDOM('
', {runScripts: 'outside-only', url:'https://example.test/editor.php?action=edit&config_revision=old&return_to=manage'}); const w = dom.window, $ = jquery(w); w.$ = $; let pickerOptions; @@ -29,13 +29,15 @@ for (const filename of ['ordinary.conf', 'A&B#C? +%é.conf']) { assert.equal(new URLSearchParams(options.data).get('mapname'), filename); options.success(''); }; - $.get = (url, callback) => { check(url); callback(''); }; - $.getScript = (url, callback) => { check(url); callback(''); }; + $.get = $.getScript = () => { throw Error('partial snapshot reload'); }; + let reloadUrl;w.wmReloadEditor=()=>reloadUrl=w.wmEditorReloadUrl(); $('#existingdata, #xycapture').attr('src', '?action=draw&mapname=' + encodeURIComponent(filename)); w.form_submit(); - assert.deepEqual(requests, ['graphs', 'load_area_data', 'load_map_javascript']); + assert.deepEqual(requests, ['graphs']); + check(reloadUrl); + const reloaded=new URL(reloadUrl);assert.equal(reloaded.searchParams.get('action'),'nothing');assert.equal(reloaded.searchParams.has('config_revision'),false);assert.equal(reloaded.searchParams.get('return_to'),'manage'); check($('#existingdata').attr('src')); check($('#xycapture').attr('src')); } w.close(); -console.log('PASS: picker, serialized save, map reload, JavaScript reload and image refresh retain ordinary and delimiter-containing filenames.'); +console.log('PASS: picker, serialized save, complete editor reload and image identity retain ordinary and delimiter-containing filenames.'); diff --git a/tests/Browser/EditorSaveFailureTest.cjs b/tests/Browser/EditorSaveFailureTest.cjs index 108b9acf..4bb4e84b 100644 --- a/tests/Browser/EditorSaveFailureTest.cjs +++ b/tests/Browser/EditorSaveFailureTest.cjs @@ -4,4 +4,5 @@ let request,message,closed=0,refreshed=0;$.ajax=opts=>request=opts;w.alert=text= const source=fs.readFileSync(path.join(__dirname,'../../js/editor.js'),'utf8');w.eval(source.slice(source.indexOf('function form_submit()'),source.indexOf('function map_properties()'))); w.form_submit();request.error();assert.equal(message,'Unable to save changes');assert.equal(closed,0);assert.equal(refreshed,0);assert.equal($('#action').val(),'set_map_properties'); w.WMConfigRevision='b'.repeat(64);w.form_submit();assert.match(request.data,/config_revision=b{64}/);request.error({status:409});assert.equal(message,'Reload the editor');assert.equal(closed,0);assert.equal(refreshed,0); +let reloads=0;w.hide_all_dialogs=()=>closed++;w.wmReloadEditor=()=>reloads++;$.get=()=>{throw Error('partial editor refresh');};request.success();assert.equal(reloads,1); w.close();console.log('PASS: save failure reports retry and preserves the form'); diff --git a/tests/Support/ConfigWriteAtomicRegression.php b/tests/Support/ConfigWriteAtomicRegression.php index 04ce4983..0fc9d026 100644 --- a/tests/Support/ConfigWriteAtomicRegression.php +++ b/tests/Support/ConfigWriteAtomicRegression.php @@ -257,6 +257,25 @@ function atomic_assert($condition, $message) { \WeathermapConfig\release($held); $wrong = \fopen($file, 'rb'); atomic_assert(!\WeathermapConfig\atomicWrite($dir . '/wrong-lock.conf', 'Wrong', false, null, true, $wrong), 'different lock file rejected'); \fclose($wrong); + // Revision metadata must describe the same map snapshot as the controls. + file_put_contents($file, $original); $snapshot = new \WeatherMap; $snapshot->ReadConfig($file); + $loadedRevision = $snapshot->config_revision; + $updatedSnapshot = str_replace('Original', 'NewSnapshot', $original); file_put_contents($file, $updatedSnapshot); + $GLOBALS['__test_nfilter_request'] = []; + ob_start(); getMapJavaScript($file, $snapshot); $javascript = ob_get_clean(); + atomic_assert(str_contains($javascript, 'Original') && !str_contains($javascript, 'NewSnapshot') && str_contains($javascript, $loadedRevision), 'rendered values and revision come from one loaded snapshot'); + $GLOBALS['__test_nfilter_request'] = ['config_revision' => $loadedRevision]; + http_response_code(200); atomic_assert(!wm_editor_read_config(new \WeatherMap, $file) && http_response_code() === 409, 'concurrent save cannot be hidden by a refreshed token'); + // Simulate deletion between the existence check and the engine file read. + class AtomicDeletedDuringReadMap extends \WeatherMap { + /** Simulate an engine load that loses its file before opening it. */ + function ReadConfig($input, $is_include = false) { unlink($input); $this->config_revision = null; return true; } + } + foreach ([[], ['config_revision' => $loadedRevision]] as $request) { + file_put_contents($file, $original); $GLOBALS['__test_nfilter_request'] = $request; + http_response_code(200); atomic_assert(!wm_editor_read_config(new AtomicDeletedDuringReadMap, $file) && http_response_code() === 404, 'deletion race cannot proceed with a null revision'); + } + $GLOBALS['__test_nfilter_request'] = []; file_put_contents($file, $original); $crlfFile = $dir . '/crlf.conf'; $crlf = "WIDTH 600\r\nHEIGHT 300\r\n"; file_put_contents($crlfFile, $crlf); $crlfMap = new \WeatherMap; $crlfMap->ReadConfig($crlfFile); diff --git a/weathermap-cacti-plugin-editor.php b/weathermap-cacti-plugin-editor.php index 46efd3c8..47a6ea18 100644 --- a/weathermap-cacti-plugin-editor.php +++ b/weathermap-cacti-plugin-editor.php @@ -243,7 +243,8 @@ } $map = new WeatherMap; -$map->ReadConfig($mapfile); +$map->context = 'editor'; +if (!wm_editor_read_config($map, $mapfile)) exit; // by here, there should be a valid $map - either a blank one, the existing one, or the existing one with requested changes wm_debug('Finished modifying'); @@ -369,7 +370,7 @@
From 07cf58b25c919d6e72545af64e31ad0827f63584 Mon Sep 17 00:00:00 2001 From: alcatron <27321443+alcatron@users.noreply.github.com> Date: Tue, 13 Oct 2026 13:58:27 +1030 Subject: [PATCH 22/41] Document the editor snapshot argument --- lib/editor.actions.php | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/lib/editor.actions.php b/lib/editor.actions.php index f2a9166a..f9386984 100644 --- a/lib/editor.actions.php +++ b/lib/editor.actions.php @@ -139,7 +139,8 @@ function newMapCopy($mapfile) { } /** - * @param mixed $mapfile + * @param mixed $mapfile Configuration path. + * @param WeatherMap|null $map Loaded control snapshot, or null for a standalone response. * @return void */ function getMapJavaScript($mapfile, $map = null) { From 8107a2251c51060c1321bfc6b0d6cf1ef56bde95 Mon Sep 17 00:00:00 2001 From: alcatron <27321443+alcatron@users.noreply.github.com> Date: Tue, 13 Oct 2026 19:33:22 +1030 Subject: [PATCH 23/41] Reject lost copy sources and finish raw saves before rendering --- lib/editor.actions.php | 5 +++- tests/Support/ConfigWriteAtomicRegression.php | 25 +++++++++++++++++++ weathermap-cacti-plugin-editor.php | 4 +-- weathermap-cacti-plugin-mgmt.php | 2 +- 4 files changed, 32 insertions(+), 4 deletions(-) diff --git a/lib/editor.actions.php b/lib/editor.actions.php index f9386984..05c61c7c 100644 --- a/lib/editor.actions.php +++ b/lib/editor.actions.php @@ -132,7 +132,10 @@ function newMapCopy($mapfile) { $sourcemap = $mapdir . '/' . $sourcemapname; if (file_exists($sourcemap) && is_readable($sourcemap)) { - $map->ReadConfig($sourcemap); + if (!$map->ReadConfig($sourcemap) || !is_string($map->config_revision)) { + http_response_code(404); + return; + } wm_editor_write_config($map, $mapfile, true); } } diff --git a/tests/Support/ConfigWriteAtomicRegression.php b/tests/Support/ConfigWriteAtomicRegression.php index 0fc9d026..f1d204b9 100644 --- a/tests/Support/ConfigWriteAtomicRegression.php +++ b/tests/Support/ConfigWriteAtomicRegression.php @@ -275,6 +275,21 @@ function ReadConfig($input, $is_include = false) { unlink($input); $this->config file_put_contents($file, $original); $GLOBALS['__test_nfilter_request'] = $request; http_response_code(200); atomic_assert(!wm_editor_read_config(new AtomicDeletedDuringReadMap, $file) && http_response_code() === 404, 'deletion race cannot proceed with a null revision'); } + + $plugin = dirname(__DIR__, 2); + // Exercise both copy entry points with an engine that loses the source during loading. + $actions = file_get_contents($plugin . '/lib/editor.actions.php'); + $copyStart = strpos($actions, 'function newMapCopy('); $copyEnd = strpos($actions, "\n/**", $copyStart); + eval(str_replace(['function newMapCopy(', 'new WeatherMap'], ['function atomicLostSourceCopy(', 'new AtomicDeletedDuringReadMap'], substr($actions, $copyStart, $copyEnd - $copyStart))); + $GLOBALS['mapdir'] = $dir; $GLOBALS['__test_request']['sourcemap'] = basename($file); $GLOBALS['__test_nfilter_request'] = ['sourcemap' => basename($file)]; + file_put_contents($file, $original); http_response_code(200); atomicLostSourceCopy($dir . '/lost-copy.conf'); + atomic_assert(http_response_code() === 404 && !is_file($dir . '/lost-copy.conf'), 'editor copy rejects source lost during load'); + $managementCopy = file_get_contents($plugin . '/weathermap-cacti-plugin-mgmt.php'); + eval(str_replace(['__DIR__', 'function newMap(', 'new WeatherMap'], [var_export($plugin, true), 'function atomicLostManagementCopy(', 'new AtomicDeletedDuringReadMap'], substr($managementCopy, strpos($managementCopy, 'function newMap(')))); + $GLOBALS['weathermap_confdir'] = $dir; file_put_contents($file, $original); + atomicLostManagementCopy('lost-management-copy.conf', basename($file)); + atomic_assert(!is_file($dir . '/lost-management-copy.conf'), 'management copy rejects source lost during load'); + unset($GLOBALS['__test_request']['sourcemap']); $GLOBALS['__test_nfilter_request'] = []; file_put_contents($file, $original); $crlfFile = $dir . '/crlf.conf'; $crlf = "WIDTH 600\r\nHEIGHT 300\r\n"; file_put_contents($crlfFile, $crlf); @@ -412,6 +427,16 @@ function ReadConfig($input, $is_include = false) { unlink($input); $this->config $GLOBALS['__test_nfilter_request'] = ['config_revision' => hash_file('sha256', $rawFile), 'node_name' => 'A', 'link_name' => 'AB', 'item_configtext' => $kind === 'node' ? "NODE A\n LABEL Updated\n POSITION 10 10\n" : "LINK AB\n NODES A B\n WIDTH 12\n"]; http_response_code(200); if ($kind === 'node') setNodeConfig($rawFile); else setLinkConfig($rawFile); atomic_assert(http_response_code() === 200 && file_get_contents($rawFile) !== $rawConfig, 'raw ' . $kind . ' edit saves without false conflict'); + file_put_contents($rawFile, $rawConfig); http_response_code(200); + $controller = file_get_contents($plugin . '/weathermap-cacti-plugin-editor.php'); + $caseStart = strpos($controller, "case 'set_" . $kind . "_config':"); + $caseEnd = strpos($controller, "case '", $caseStart + 5); + $dispatch = substr($controller, strpos($controller, ':', $caseStart) + 1, $caseEnd - strpos($controller, ':', $caseStart) - 1); + $mapfile = $rawFile; $terminated = false; + try { eval(str_replace('exit();', 'throw new \\RuntimeException("request-finished");', $dispatch)); } + catch (\RuntimeException $error) { if ($error->getMessage() !== 'request-finished') throw $error; $terminated = true; } + atomic_assert($terminated && http_response_code() === 200 && file_get_contents($rawFile) !== $rawConfig, 'controller ends successful raw ' . $kind . ' save before rendering with old token'); + } // Registration failure must retain a mutex through the callback and remove its reservation. file_put_contents($plugin . '/configs/' . $duplicateSource, "TITLE Source\n"); diff --git a/weathermap-cacti-plugin-editor.php b/weathermap-cacti-plugin-editor.php index 47a6ea18..fedc76fc 100644 --- a/weathermap-cacti-plugin-editor.php +++ b/weathermap-cacti-plugin-editor.php @@ -169,11 +169,11 @@ case 'set_link_config': setLinkConfig($mapfile); - break; + exit(); case 'set_node_config': setNodeConfig($mapfile); - break; + exit(); case 'set_node_properties': setNodeProperties($mapfile); exit; diff --git a/weathermap-cacti-plugin-mgmt.php b/weathermap-cacti-plugin-mgmt.php index acabe0f4..de7bf3fc 100644 --- a/weathermap-cacti-plugin-mgmt.php +++ b/weathermap-cacti-plugin-mgmt.php @@ -3443,7 +3443,7 @@ function newMap($mapfile, $sourcemapfile = '', $title = '') { if ($mapfile != '') { if ($sourcemapfile != '') { if (file_exists($sourcemapfile) && is_readable($sourcemapfile)) { - if (!$map->ReadConfig($sourcemapfile)) { + if (!$map->ReadConfig($sourcemapfile) || !is_string($map->config_revision)) { raise_message('map_message', __('The Source Map File name is not readable or does not exist!', 'weathermap'), MESSAGE_LEVEL_ERROR); return; } From f5b97d63a6868d4690415da72850d5b350458a0d Mon Sep 17 00:00:00 2001 From: alcatron <27321443+alcatron@users.noreply.github.com> Date: Tue, 13 Oct 2026 19:56:08 +1030 Subject: [PATCH 24/41] Keep logical configuration mutex for symlink writes --- lib/WeatherMap.config.php | 5 +++-- tests/Support/ConfigWriteAtomicRegression.php | 11 +++++++++++ 2 files changed, 14 insertions(+), 2 deletions(-) diff --git a/lib/WeatherMap.config.php b/lib/WeatherMap.config.php index 0e9377a8..87c1d1a7 100644 --- a/lib/WeatherMap.config.php +++ b/lib/WeatherMap.config.php @@ -88,15 +88,16 @@ function writeAll($stream, $contents) { */ function atomicWrite($filename, $contents, $mustExist = false, $expectedRevision = null, $createOnly = false, $heldLock = null) { if (strpos($filename, '://') !== false) return false; + $lockDirectory = dirname($filename); if (is_link($filename)) $filename = realpath($filename); if ($filename === false || !is_dir(dirname($filename)) || !is_writable(dirname($filename))) return false; if (file_exists($filename) && (!is_file($filename) || !is_writable($filename))) return false; $existed = file_exists($filename); $mustExist = $mustExist || $existed; $ownsLock = $heldLock === null; - $lockPath = realpath(dirname($filename) . '/.weathermap-config.lock'); + $lockPath = realpath($lockDirectory . '/.weathermap-config.lock'); if (!$ownsLock && ($lockPath === false || !is_resource($heldLock) || get_resource_type($heldLock) !== 'stream' || realpath(stream_get_meta_data($heldLock)['uri']) !== $lockPath)) return false; - $lock = $ownsLock ? acquire(dirname($filename)) : $heldLock; + $lock = $ownsLock ? acquire($lockDirectory) : $heldLock; if (!$lock) return false; $temp = false; $stream = false; diff --git a/tests/Support/ConfigWriteAtomicRegression.php b/tests/Support/ConfigWriteAtomicRegression.php index f1d204b9..82c7c5fe 100644 --- a/tests/Support/ConfigWriteAtomicRegression.php +++ b/tests/Support/ConfigWriteAtomicRegression.php @@ -255,6 +255,17 @@ function atomic_assert($condition, $message) { atomic_assert(pcntl_wexitstatus($status) === 0, 'atomicWrite leaves caller mutex held'); } \WeathermapConfig\release($held); + + $external = sys_get_temp_dir() . '/wm-external-' . bin2hex(random_bytes(5)); + mkdir($external); $externalFile = $external . '/map.conf'; file_put_contents($externalFile, $original); + $alias = $dir . '/external.conf'; + if (@symlink($externalFile, $alias)) { + $logicalLock = \WeathermapConfig\acquire($dir); + atomic_assert(\WeathermapConfig\atomicWrite($alias, $original, true, hash('sha256', $original), false, $logicalLock), 'external symlink accepts caller-held logical configuration mutex'); + atomic_assert(!is_file($external . '/.weathermap-config.lock'), 'symlink replacement does not choose external target mutex'); + \WeathermapConfig\release($logicalLock); unlink($alias); + } + unlink($externalFile); rmdir($external); $wrong = \fopen($file, 'rb'); atomic_assert(!\WeathermapConfig\atomicWrite($dir . '/wrong-lock.conf', 'Wrong', false, null, true, $wrong), 'different lock file rejected'); \fclose($wrong); // Revision metadata must describe the same map snapshot as the controls. From 6c0ed54060bc3453ed14248db41a89c86dbd5cb7 Mon Sep 17 00:00:00 2001 From: alcatron <27321443+alcatron@users.noreply.github.com> Date: Tue, 13 Oct 2026 20:19:05 +1030 Subject: [PATCH 25/41] Preserve selected custom images when refreshing libraries --- js/editor.js | 16 +++++++++++++++- tests/Browser/EditorImagesTest.cjs | 11 +++++++++++ tests/Browser/EditorSaveFailureTest.cjs | 2 +- 3 files changed, 27 insertions(+), 2 deletions(-) diff --git a/js/editor.js b/js/editor.js index 347ec39c..963ae13f 100644 --- a/js/editor.js +++ b/js/editor.js @@ -884,7 +884,17 @@ function wmReloadEditor() { window.location.replace(wmEditorReloadUrl()); } -function form_submit() { +function form_submit(event) { + // Property dialogs can remain open while a toolbar placement action is active. + // Their own Save buttons always dispatch the properties they are showing. + var button = event && event.currentTarget && event.currentTarget.id; + if (button === 'tb_node_submit' || button === 'tb_link_submit') { + var node = button === 'tb_node_submit'; + $('#action').val(node ? 'set_node_properties' : 'set_link_properties'); + $('#param').val($(node ? '#node_name' : '#link_name').val()); + mapmode('existing'); + } + var data = $('input, select, textarea').serialize(); if (typeof WMConfigRevision !== 'undefined') data += '&config_revision=' + encodeURIComponent(WMConfigRevision); @@ -1752,11 +1762,15 @@ function wmAssetImages(images) { var select = $('#' + spec.id), old = select.val(); var slick = select.data('ddslick'); if (slick) { old = slick.selectedData && slick.selectedData.value; select.ddslick('destroy'); select = $('#' + spec.id); } + var current = select.find('option').filter(function() { return this.value === old; }).first().clone(); select.find('option').filter(function() { return this.value.indexOf('images/' + spec.kind + '/') === 0 && /\.(png|gif|jpe?g)$/i.test(this.value); }).remove(); if (!select.find('option[value="--NONE--"]').length) select.prepend($('