diff --git a/htdocs/admin/mails.php b/htdocs/admin/mails.php index 2b50008d58a888209631f41857a9f9ec57aa3ddd..53937687b57325d1771c634fe24b31aeb2266d2c 100644 --- a/htdocs/admin/mails.php +++ b/htdocs/admin/mails.php @@ -121,7 +121,7 @@ if (! empty($_POST['removedfile']) || ! empty($_POST['removedfilehtml'])) $pathtodelete=$listofpaths[$keytodelete]; $filetodelete=$listofnames[$keytodelete]; $result = dol_delete_file($pathtodelete,1); - if ($result >= 0) + if ($result) { setEventMessage($langs->trans("FileWasRemoved"), $filetodelete); diff --git a/htdocs/admin/tools/purge.php b/htdocs/admin/tools/purge.php index 9003a92cddf27646f129a711d79f9ed8424fdbf5..e40caa3e36e95c7cf6f92e36eadd7e48421902ae 100644 --- a/htdocs/admin/tools/purge.php +++ b/htdocs/admin/tools/purge.php @@ -89,7 +89,7 @@ if ($action=='purge' && ! preg_match('/^confirm/i',$choice) && ($choice != 'allf // If (file that is not logfile) or (if logfile with option logfile) if ($filesarray[$key]['fullname'] != $filelog || $choice=='logfile') { - $count+=dol_delete_file($filesarray[$key]['fullname']); + $count+=(dol_delete_file($filesarray[$key]['fullname'])?1:0); } } } diff --git a/htdocs/compta/facture.php b/htdocs/compta/facture.php index 88d6bb587e4eb1950658bfb045fab6ce99e8ad4d..f765284f27ee6f5c65741be3ec2171544bbf7bef 100644 --- a/htdocs/compta/facture.php +++ b/htdocs/compta/facture.php @@ -1759,10 +1759,8 @@ if (empty($reshook)) $upload_dir = $conf->facture->dir_output; $file = $upload_dir . '/' . GETPOST('file'); $ret = dol_delete_file($file, 0, 0, 0, $object); - if ($ret) - setEventMessage($langs->trans("FileWasRemoved", GETPOST('urlfile'))); - else - setEventMessage($langs->trans("ErrorFailToDeleteFile", GETPOST('urlfile')), 'errors'); + if ($ret) setEventMessage($langs->trans("FileWasRemoved", GETPOST('urlfile'))); + else setEventMessage($langs->trans("ErrorFailToDeleteFile", GETPOST('urlfile')), 'errors'); $action = ''; } } diff --git a/htdocs/contrat/card.php b/htdocs/contrat/card.php index dd48b27684f4848e1e3cc3b94664daa027ffd8f9..192a2d16b1785823fa41a8a1c0fc1fe0ea028c5f 100644 --- a/htdocs/contrat/card.php +++ b/htdocs/contrat/card.php @@ -823,10 +823,8 @@ else if ($action == 'remove_file' && $user->rights->contrat->creer) { $upload_dir = $conf->contrat->dir_output; $file = $upload_dir . '/' . GETPOST('file'); $ret = dol_delete_file($file, 0, 0, 0, $object); - if ($ret) - setEventMessage($langs->trans("FileWasRemoved", GETPOST('file'))); - else - setEventMessage($langs->trans("ErrorFailToDeleteFile", GETPOST('file')), 'errors'); + if ($ret) setEventMessage($langs->trans("FileWasRemoved", GETPOST('file'))); + else setEventMessage($langs->trans("ErrorFailToDeleteFile", GETPOST('file')), 'errors'); } } diff --git a/htdocs/core/class/hookmanager.class.php b/htdocs/core/class/hookmanager.class.php index 47810e204bdf995685dd80b64785160690f4cbb2..ee7742660c171697d0636f23fd3717a54787f5b7 100644 --- a/htdocs/core/class/hookmanager.class.php +++ b/htdocs/core/class/hookmanager.class.php @@ -116,9 +116,9 @@ class HookManager * @param array $parameters Array of parameters * @param Object $object Object to use hooks on * @param string $action Action code on calling page ('create', 'edit', 'view', 'add', 'update', 'delete'...) - * @return mixed For doActions,formObjectOptions,pdf_xxx: Return 0 if we want to keep standard actions, >0 if we want to stop standard actions, <0 means KO. - * For printSearchForm,printLeftBlock,printTopRightMenu,formAddObjectLine,...: Return HTML string. TODO Deprecated. Must always return an int and things to print into ->resprints. - * Can also return some values into an array ->results. + * @return mixed For 'addreplace hooks (doActions,formObjectOptions,pdf_xxx,...): Return 0 if we want to keep standard actions, >0 if we want to stop standard actions, <0 if KO. + * For 'output' hooks (printLeftBlock, formAddObjectLine, formBuilddocOptions, ...): Return 0, <0 if KO. Things to print are returned into ->resprints and set into ->resPrint. + * All types can also return some values into an array ->results. * $this->error or this->errors are also defined by class called by this function if error. */ function executeHooks($method, $parameters=false, &$object='', &$action='') @@ -135,7 +135,8 @@ class HookManager array( 'addMoreActionsButtons', 'addStatisticLine', - 'doActions', + 'deleteFile', + 'doActions', 'formCreateThirdpartyOptions', 'formObjectOptions', 'formattachOptions', @@ -147,7 +148,7 @@ class HookManager 'formatEvent' ) )) $hooktype='addreplace'; - // Deprecated hook types + // Deprecated hook types ('returnvalue') if (preg_match('/^pdf_/',$method) && $method != 'pdf_writelinedesc') $hooktype='returnvalue'; // pdf_xxx except pdf_writelinedesc are 'returnvalue' hooks. When there is 2 hooks of this type, only last one win. TODO Move them into 'output' or 'addreplace' hooks. if ($method == 'insertExtraFields') { @@ -195,7 +196,7 @@ class HookManager if (isset($actionclassinstance->results) && is_array($actionclassinstance->results)) $this->resArray =array_merge($this->resArray, $actionclassinstance->results); if (! empty($actionclassinstance->resprints)) $this->resPrint.=$actionclassinstance->resprints; } - // Generic hooks that return a string or array (printSearchForm, printLeftBlock, formAddObjectLine, formBuilddocOptions, ...) + // Generic hooks that return a string or array (printLeftBlock, formAddObjectLine, formBuilddocOptions, ...) else { // TODO. this should be done into the method of hook by returning nothing diff --git a/htdocs/core/lib/files.lib.php b/htdocs/core/lib/files.lib.php index daae392cd393af2a443b385d43d713bb82e65e51..05fdcc96624f291e83687b084472c261abde6434 100644 --- a/htdocs/core/lib/files.lib.php +++ b/htdocs/core/lib/files.lib.php @@ -793,12 +793,12 @@ function dol_move_uploaded_file($src_file, $dest_file, $allowoverwrite, $disable /** * Remove a file or several files with a mask * - * @param string $file File to delete or mask of file to delete - * @param int $disableglob Disable usage of glob like * + * @param string $file File to delete or mask of files to delete + * @param int $disableglob Disable usage of glob like * so function is an exact delete function that will return error if no file found * @param int $nophperrors Disable all PHP output errors * @param int $nohook Disable all hooks * @param object $object Current object in use - * @return boolean True if file is deleted (or if glob is used and there's nothing to delete), False if error + * @return boolean True if no error (file is deleted or if glob is used and there's nothing to delete), False if error */ function dol_delete_file($file,$disableglob=0,$nophperrors=0,$nohook=0,$object=null) { @@ -823,19 +823,20 @@ function dol_delete_file($file,$disableglob=0,$nophperrors=0,$nohook=0,$object=n $reshook=$hookmanager->executeHooks('deleteFile', $parameters, $object); } - if (empty($nohook) && isset($reshook) && $reshook != '') // 0:not deleted, 1:deleted, null or '' for bypass + if (empty($nohook) && $reshook != 0) // reshook = 0 to do standard actions, 1 = ok, -1 = ko { - return $reshook; + if ($reshook < 0) return false; + return true; } else { $error=0; //print "x".$file." ".$disableglob;exit; - $ok=true; $file_osencoded=dol_osencode($file); // New filename encoded in OS filesystem encoding charset if (empty($disableglob) && ! empty($file_osencoded)) { + $ok=true; $globencoded=str_replace('[','\[',$file_osencoded); $globencoded=str_replace(']','\]',$globencoded); $listofdir=glob($globencoded); @@ -853,6 +854,7 @@ function dol_delete_file($file,$disableglob=0,$nophperrors=0,$nohook=0,$object=n } else { + $ok=false; if ($nophperrors) $ok=@unlink($file_osencoded); else $ok=unlink($file_osencoded); if ($ok) dol_syslog("Removed file ".$file_osencoded, LOG_DEBUG); @@ -958,9 +960,9 @@ function dol_delete_preview($object) if (file_exists($file) && is_writable($file)) { - if ( ! dol_delete_file($file,1) ) + if (! dol_delete_file($file,1)) { - $object->error=$langs->trans("ErrorFailedToOpenFile",$file); + $object->error=$langs->trans("ErrorFailedToDeleteFile",$file); return 0; } } diff --git a/htdocs/core/tpl/document_actions_pre_headers.tpl.php b/htdocs/core/tpl/document_actions_pre_headers.tpl.php index 36b6ce75fe1ac4d5aea8d2ae2428d343d2371ef6..40bd0a5368e86a1aada3d2eaf8b9e4908b1891fb 100644 --- a/htdocs/core/tpl/document_actions_pre_headers.tpl.php +++ b/htdocs/core/tpl/document_actions_pre_headers.tpl.php @@ -59,11 +59,8 @@ if ($action == 'confirm_deletefile' && $confirm == 'yes') if ($urlfile) { $ret = dol_delete_file($file, 0, 0, 0, $object); - if ($ret) { - setEventMessage($langs->trans("FileWasRemoved", $urlfile)); - } else { - setEventMessage($langs->trans("ErrorFailToDeleteFile", $urlfile), 'errors'); - } + if ($ret) setEventMessage($langs->trans("FileWasRemoved", $urlfile)); + else setEventMessage($langs->trans("ErrorFailToDeleteFile", $urlfile), 'errors'); } elseif ($linkid) { diff --git a/htdocs/install/upgrade2.php b/htdocs/install/upgrade2.php index 70ebd8550c5da4416a186924d47c72f57ceab4e9..913b21bdd14dc0f9b520a6252ff220bcec194d17 100644 --- a/htdocs/install/upgrade2.php +++ b/htdocs/install/upgrade2.php @@ -3680,7 +3680,7 @@ function migrate_delete_old_files($db,$langs,$conf) print ' '.$langs->trans("RemoveItManuallyAndPressF5ToContinue").'</div>'; } else - { + { //print $langs->trans("FileWasRemoved",$filetodelete); } } diff --git a/test/phpunit/FilesLibTest.php b/test/phpunit/FilesLibTest.php index 4477784ae7d69922da42beabed53e06f0d03ef01..e9fb713b857c8de025e1f95a06bc99f2db77273a 100644 --- a/test/phpunit/FilesLibTest.php +++ b/test/phpunit/FilesLibTest.php @@ -339,10 +339,15 @@ class FilesLibTest extends PHPUnit_Framework_TestCase print __METHOD__." result=".$result."\n"; $this->assertTrue($result,'delete file'); - // Again to test no error when deleteing a non existing file - $result=dol_delete_file($conf->admin->dir_temp.'/file2.csv'); + // Again to test there is error when deleting a non existing file with option disableglob + $result=dol_delete_file($conf->admin->dir_temp.'/file2.csv',1,1); + print __METHOD__." result=".$result."\n"; + $this->assertFalse($result,'delete file that does not exists with disableglo must return ko'); + + // Again to test there is no error when deleting a non existing file without option disableglob + $result=dol_delete_file($conf->admin->dir_temp.'/file2.csv',0,1); print __METHOD__." result=".$result."\n"; - $this->assertTrue($result,'delete file that does not exists'); + $this->assertTrue($result,'delete file that does not exists without disabling glob must return ok'); // Test copy with special char / delete with blob $result=dol_copy($file, $conf->admin->dir_temp.'/file with [x] and é.csv',0,1);