Merge branch 'rmdir-rmfile' into next

RouterOS 7.18beta2 comes with some severe issues in file handling,
probably racy conditions. Let's move file (and directory) removal to
functions, so errors can be caught and ignored 🤪 from a central place.
This commit is contained in:
Christian Hesse 2025-02-07 22:00:42 +01:00
commit fefe11d1e8
9 changed files with 91 additions and 19 deletions

View file

@ -26,6 +26,7 @@
:global LogPrint; :global LogPrint;
:global MkDir; :global MkDir;
:global RandomDelay; :global RandomDelay;
:global RmDir;
:global ScriptFromTerminal; :global ScriptFromTerminal;
:global ScriptLock; :global ScriptLock;
:global SendNotification2; :global SendNotification2;
@ -97,7 +98,7 @@
$LogPrint error $ScriptName ("Failed uploading backup for " . $Identity . " to cloud!"); $LogPrint error $ScriptName ("Failed uploading backup for " . $Identity . " to cloud!");
:set PackagesUpdateBackupFailure true; :set PackagesUpdateBackupFailure true;
} }
/file/remove "tmpfs/backup-cloud"; $RmDir "tmpfs/backup-cloud";
} on-error={ } on-error={
:global ExitError; $ExitError $ExitOK [ :jobname ]; :global ExitError; $ExitError $ExitOK [ :jobname ];
} }

View file

@ -35,6 +35,8 @@
:global LogPrint; :global LogPrint;
:global MkDir; :global MkDir;
:global RandomDelay; :global RandomDelay;
:global RmDir;
:global RmFile;
:global ScriptFromTerminal; :global ScriptFromTerminal;
:global ScriptLock; :global ScriptLock;
:global SendNotification2; :global SendNotification2;
@ -99,7 +101,7 @@
:set Failed 1; :set Failed 1;
} }
/file/remove ($FilePath . ".backup"); $RmFile ($FilePath . ".backup");
} }
# create configuration export # create configuration export
@ -118,7 +120,7 @@
:set Failed 1; :set Failed 1;
} }
/file/remove ($FilePath . ".rsc"); $RmFile ($FilePath . ".rsc");
} }
# global-config-overlay # global-config-overlay
@ -139,7 +141,7 @@
:set Failed 1; :set Failed 1;
} }
/file/remove ($FilePath . ".conf"); $RmFile ($FilePath . ".conf");
} }
:local FileInfo do={ :local FileInfo do={
@ -170,7 +172,7 @@
:if ($Failed = 1) do={ :if ($Failed = 1) do={
:set PackagesUpdateBackupFailure true; :set PackagesUpdateBackupFailure true;
} }
/file/remove $DirName; $RmDir $DirName;
} on-error={ } on-error={
:global ExitError; $ExitError $ExitOK [ :jobname ]; :global ExitError; $ExitError $ExitOK [ :jobname ];
} }

View file

@ -22,6 +22,7 @@
:global DownloadPackage; :global DownloadPackage;
:global LogPrint; :global LogPrint;
:global MkDir; :global MkDir;
:global RmFile;
:global ScriptLock; :global ScriptLock;
:global WaitFullyConnected; :global WaitFullyConnected;
@ -61,7 +62,7 @@
:if ([ $DownloadPackage ($File->"package-name") $InstalledVersion \ :if ([ $DownloadPackage ($File->"package-name") $InstalledVersion \
($File->"package-architecture") $PackagePath ] = true) do={ ($File->"package-architecture") $PackagePath ] = true) do={
:set Updated true; :set Updated true;
/file/remove $Package; $RmFile $Package;
} }
} }

View file

@ -23,6 +23,7 @@
:global DownloadPackage; :global DownloadPackage;
:global LogPrint; :global LogPrint;
:global MkDir; :global MkDir;
:global RmFile;
:global ScriptLock; :global ScriptLock;
:global WaitFullyConnected; :global WaitFullyConnected;
@ -63,7 +64,7 @@
:if ([ $DownloadPackage ($File->"package-name") $InstalledVersion \ :if ([ $DownloadPackage ($File->"package-name") $InstalledVersion \
($File->"package-architecture") $PackagePath ] = true) do={ ($File->"package-architecture") $PackagePath ] = true) do={
:set Updated true; :set Updated true;
/file/remove $Package; $RmFile $Package;
} }
} }

View file

@ -22,6 +22,7 @@
:global DownloadPackage; :global DownloadPackage;
:global LogPrint; :global LogPrint;
:global MkDir; :global MkDir;
:global RmFile;
:global ScriptLock; :global ScriptLock;
:global WaitFullyConnected; :global WaitFullyConnected;
@ -61,7 +62,7 @@
:if ([ $DownloadPackage ($File->"package-name") $InstalledVersion \ :if ([ $DownloadPackage ($File->"package-name") $InstalledVersion \
($File->"package-architecture") $PackagePath ] = true) do={ ($File->"package-architecture") $PackagePath ] = true) do={
:set Updated true; :set Updated true;
/file/remove $Package; $RmFile $Package;
} }
} }

View file

@ -44,6 +44,7 @@
:global EscapeForRegEx; :global EscapeForRegEx;
:global FetchUserAgentStr; :global FetchUserAgentStr;
:global LogPrint; :global LogPrint;
:global RmFile;
:global UrlEncode; :global UrlEncode;
:global WaitForFile; :global WaitForFile;
@ -63,7 +64,7 @@
:set DecryptionFailed false; :set DecryptionFailed false;
} }
} }
/file/remove [ find where name=$CertFileName ]; $RmFile $CertFileName;
:if ($DecryptionFailed = true) do={ :if ($DecryptionFailed = true) do={
$LogPrint warning $ScriptName ("Decryption failed for certificate file '" . $CertFileName . "'."); $LogPrint warning $ScriptName ("Decryption failed for certificate file '" . $CertFileName . "'.");

View file

@ -63,6 +63,8 @@
:global ProtocolStrip; :global ProtocolStrip;
:global RandomDelay; :global RandomDelay;
:global RequiredRouterOS; :global RequiredRouterOS;
:global RmDir;
:global RmFile;
:global ScriptFromTerminal; :global ScriptFromTerminal;
:global ScriptInstallUpdate; :global ScriptInstallUpdate;
:global ScriptLock; :global ScriptLock;
@ -147,6 +149,7 @@
:global CleanName; :global CleanName;
:global FetchUserAgentStr; :global FetchUserAgentStr;
:global LogPrint; :global LogPrint;
:global RmFile;
:global WaitForFile; :global WaitForFile;
$LogPrint info $0 ("Downloading and importing certificate with " . \ $LogPrint info $0 ("Downloading and importing certificate with " . \
@ -170,7 +173,7 @@
dst-path=$FileName as-value; dst-path=$FileName as-value;
$WaitForFile $FileName; $WaitForFile $FileName;
:if ([ /file/get $FileName size ] = 0) do={ :if ([ /file/get $FileName size ] = 0) do={
/file/remove $FileName; $RmFile $FileName;
:error false; :error false;
} }
} on-error={ } on-error={
@ -181,7 +184,7 @@
/certificate/import file-name=$FileName passphrase="" as-value; /certificate/import file-name=$FileName passphrase="" as-value;
:delay 1s; :delay 1s;
/file/remove [ find where name=$FileName ]; $RmFile $FileName;
:if ([ :len [ /certificate/find where common-name=$CommonName ] ] = 0) do={ :if ([ :len [ /certificate/find where common-name=$CommonName ] ] = 0) do={
/certificate/remove [ find where name~("^" . $FileName . "_[0-9]+\$") ]; /certificate/remove [ find where name~("^" . $FileName . "_[0-9]+\$") ];
@ -340,6 +343,7 @@
:global CleanFilePath; :global CleanFilePath;
:global LogPrint; :global LogPrint;
:global MkDir; :global MkDir;
:global RmFile;
:global WaitForFile; :global WaitForFile;
:if ([ :len $PkgName ] = 0) do={ :return false; } :if ([ :len $PkgName ] = 0) do={ :return false; }
@ -383,7 +387,7 @@
$LogPrint debug $0 ("Downloading package file failed."); $LogPrint debug $0 ("Downloading package file failed.");
} }
/file/remove [ find where name=$PkgDest ]; $RmFile $PkgDest;
:set Retry ($Retry - 1); :set Retry ($Retry - 1);
} }
@ -453,6 +457,8 @@
:global IfThenElse; :global IfThenElse;
:global LogPrint; :global LogPrint;
:global MkDir; :global MkDir;
:global RmDir;
:global RmFile;
:global WaitForFile; :global WaitForFile;
:set CheckCert [ $IfThenElse ($CheckCert = "false") "no" "yes-without-crl" ]; :set CheckCert [ $IfThenElse ($CheckCert = "false") "no" "yes-without-crl" ];
@ -469,10 +475,10 @@
http-header-field=({ [ $FetchUserAgentStr $ScriptName ] }) as-value; http-header-field=({ [ $FetchUserAgentStr $ScriptName ] }) as-value;
} on-error={ } on-error={
:if ([ $WaitForFile $FileName 500ms ] = true) do={ :if ([ $WaitForFile $FileName 500ms ] = true) do={
/file/remove $FileName; $RmFile $FileName;
} }
$LogPrint debug $0 ("Failed downloading from: " . $Url); $LogPrint debug $0 ("Failed downloading from: " . $Url);
/file/remove $DirName; $RmDir $DirName;
:return false; :return false;
} }
$WaitForFile $FileName; $WaitForFile $FileName;
@ -488,7 +494,7 @@
:delay 100ms; :delay 100ms;
} }
} }
/file/remove $DirName; $RmDir $DirName;
:return $Return; :return $Return;
} }
@ -851,6 +857,7 @@
:global CleanFilePath; :global CleanFilePath;
:global LogPrint; :global LogPrint;
:global RmDir;
:global WaitForFile; :global WaitForFile;
:local MkTmpfs do={ :local MkTmpfs do={
@ -867,7 +874,7 @@
} }
$LogPrint info $0 ("Creating disk of type tmpfs."); $LogPrint info $0 ("Creating disk of type tmpfs.");
/file/remove [ find where name="tmpfs" type="directory" ]; $RmDir "tmpfs";
:do { :do {
/disk/add slot=tmpfs type=tmpfs tmpfs-max-size=([ /system/resource/get total-memory ] / 3); /disk/add slot=tmpfs type=tmpfs tmpfs-max-size=([ /system/resource/get total-memory ] / 3);
$WaitForFile "tmpfs"; $WaitForFile "tmpfs";
@ -1004,6 +1011,62 @@
:return true; :return true;
} }
# remove directory
:set RmDir do={
:local DirName [ :tostr $1 ];
:global LogPrint;
$LogPrint debug $0 ("Removing directory: ". $DirName);
:if ([ :len [ /file/find where name=$DirName type!=directory ] ] > 0) do={
$LogPrint error $0 ("Directory '" . $DirName . "' is not a directory.");
:return false;
}
:local Dir [ /file/find where name=$DirName type=directory ];
:if ([ :len $Dir ] = 0) do={
$LogPrint debug $0 ("... which does not exist.");
:return true;
}
:do {
/file/remove $Dir;
} on-error={
$LogPrint error $0 ("Removing directory '" . $DirName . "' (" . $Dir . ") failed.");
:return false;
}
:return true;
}
# remove file
:set RmFile do={
:local FileName [ :tostr $1 ];
:global LogPrint;
$LogPrint debug $0 ("Removing file: ". $FileName);
:if ([ :len [ /file/find where name=$FileName type!=file ] ] > 0) do={
$LogPrint error $0 ("File '" . $FileName . "' is not a file.");
:return false;
}
:local File [ /file/find where name=$FileName type=file ];
:if ([ :len $File ] = 0) do={
$LogPrint debug $0 ("... which does not exist.");
:return true;
}
:do {
/file/remove $File;
} on-error={
$LogPrint error $0 ("Removing file '" . $FileName . "' (" . $File . ") failed.");
:return false;
}
:return true;
}
# check if script is run from terminal # check if script is run from terminal
:set ScriptFromTerminal do={ :set ScriptFromTerminal do={
:local Script [ :tostr $1 ]; :local Script [ :tostr $1 ];

View file

@ -19,6 +19,7 @@
:global GetRandom20CharAlNum; :global GetRandom20CharAlNum;
:global LogPrint; :global LogPrint;
:global MkDir; :global MkDir;
:global RmDir;
:global WaitForFile; :global WaitForFile;
:if ([ :len $Key ] = 0 || [ :len $User ] = 0) do={ :if ([ :len $Key ] = 0 || [ :len $User ] = 0) do={
@ -58,10 +59,10 @@
/user/ssh-keys/import public-key-file=$FileName user=$User; /user/ssh-keys/import public-key-file=$FileName user=$User;
$LogPrint info $0 ("Imported ssh public key (" . $KeyVal->2 . ", " . $KeyVal->0 . ", " . \ $LogPrint info $0 ("Imported ssh public key (" . $KeyVal->2 . ", " . $KeyVal->0 . ", " . \
"MD5:" . $FingerPrintMD5 . ") for user '" . $User . "'."); "MD5:" . $FingerPrintMD5 . ") for user '" . $User . "'.");
/file/remove "tmpfs/ssh-keys-import"; $RmDir "tmpfs/ssh-keys-import";
} on-error={ } on-error={
$LogPrint warning $0 ("Failed importing key."); $LogPrint warning $0 ("Failed importing key.");
/file/remove "tmpfs/ssh-keys-import"; $RmDir "tmpfs/ssh-keys-import";
:return false; :return false;
} }
} on-error={ } on-error={

View file

@ -37,6 +37,7 @@
:global MIN; :global MIN;
:global MkDir; :global MkDir;
:global RandomDelay; :global RandomDelay;
:global RmDir;
:global ScriptLock; :global ScriptLock;
:global SendTelegram2; :global SendTelegram2;
:global SymbolForNotification; :global SymbolForNotification;
@ -154,7 +155,7 @@
$State . [ $IfThenElse ([ :len $Content ] > 0) \ $State . [ $IfThenElse ([ :len $Content ] > 0) \
([ $SymbolForNotification "memo" ] . "Output:\n" . $Content) \ ([ $SymbolForNotification "memo" ] . "Output:\n" . $Content) \
([ $SymbolForNotification "memo" ] . "No output.") ]) }); ([ $SymbolForNotification "memo" ] . "No output.") ]) });
/file/remove "tmpfs/telegram-chat"; $RmDir "tmpfs/telegram-chat";
} else={ } else={
$LogPrint info $ScriptName ("The command from update " . $UpdateID . " failed syntax validation!"); $LogPrint info $ScriptName ("The command from update " . $UpdateID . " failed syntax validation!");
$SendTelegram2 ({ origin=$ScriptName; chatid=($Chat->"id"); silent=false; replyto=($Message->"message_id"); \ $SendTelegram2 ({ origin=$ScriptName; chatid=($Chat->"id"); silent=false; replyto=($Message->"message_id"); \