diff --git a/htdocs/accountancy/admin/fiscalyear_card.php b/htdocs/accountancy/admin/fiscalyear_card.php index 6de0f000241..8c587b143c1 100644 --- a/htdocs/accountancy/admin/fiscalyear_card.php +++ b/htdocs/accountancy/admin/fiscalyear_card.php @@ -321,7 +321,7 @@ if (($id || $ref) && $action == 'edit') { if ($object->id > 0 && (empty($action) || ($action != 'edit' && $action != 'create'))) { $head = fiscalyear_prepare_head($object); - print dol_get_fiche_head($head, 'card', $langs->trans("Fiscalyear"), -1, $object->picto, 0, '', '', 0, '', 1); + print dol_get_fiche_head($head, 'card', $langs->trans("Fiscalyear"), -1, $object->picto); $morehtmlref = ''; //$morehtmlref .= '
'; diff --git a/htdocs/accountancy/admin/fiscalyear_info.php b/htdocs/accountancy/admin/fiscalyear_info.php index 54217511698..2ebd074e995 100644 --- a/htdocs/accountancy/admin/fiscalyear_info.php +++ b/htdocs/accountancy/admin/fiscalyear_info.php @@ -78,7 +78,7 @@ if ($id) { $head = fiscalyear_prepare_head($object); - print dol_get_fiche_head($head, 'info', $langs->trans("Fiscalyear"), -1, $object->picto, 0, '', '', 0, '', 1); + print dol_get_fiche_head($head, 'info', $langs->trans("Fiscalyear"), -1, $object->picto); $linkback = ''.$langs->trans("BackToList").''; diff --git a/htdocs/accountancy/admin/template/card.php b/htdocs/accountancy/admin/template/card.php index dfc26e15bd5..74d64c5c7db 100644 --- a/htdocs/accountancy/admin/template/card.php +++ b/htdocs/accountancy/admin/template/card.php @@ -397,7 +397,7 @@ if (($id || $code) && $action == 'edit') { if ($object->id > 0 && (empty($action) || ($action != 'edit' && $action != 'create'))) { $head = accountingTransactionTemplatePrepareHead($object); - print dol_get_fiche_head($head, 'card', $langs->trans("BookkeepingTemplate"), -1, $object->picto, 0, '', '', 0, '', 1); + print dol_get_fiche_head($head, 'card', $langs->trans("BookkeepingTemplate"), -1, $object->picto); $formconfirm = ''; diff --git a/htdocs/asset/card.php b/htdocs/asset/card.php index fc6405f5453..8fcf3591850 100644 --- a/htdocs/asset/card.php +++ b/htdocs/asset/card.php @@ -263,7 +263,7 @@ if ($object->id > 0 && (empty($action) || ($action != 'edit' && $action != 'crea $res = $object->fetch_optionals(); $head = assetPrepareHead($object); - print dol_get_fiche_head($head, 'card', $langs->trans("Asset"), -1, $object->picto); + print dol_get_fiche_head($head, 'card', $langs->trans("Asset"), -1, $object->picto, 0, '', '', 0, '', 1); $formconfirm = ''; diff --git a/htdocs/bookcal/booking_list.php b/htdocs/bookcal/booking_list.php index f1ccc9ea740..8dd2d2b5980 100644 --- a/htdocs/bookcal/booking_list.php +++ b/htdocs/bookcal/booking_list.php @@ -126,7 +126,7 @@ llxHeader('', $title, $helpurl, '', 0, 0, '', '', '', 'mod-bookcal page-list'); if ($object->id > 0) { $head = calendarPrepareHead($object); - print dol_get_fiche_head($head, 'booking', $langs->trans("Calendar"), -1, $object->picto, 0, '', '', 0, '', 1); + print dol_get_fiche_head($head, 'booking', $langs->trans("Calendar"), -1, $object->picto); $formconfirm = ''; diff --git a/htdocs/bookcal/calendar_card.php b/htdocs/bookcal/calendar_card.php index e320b75c18f..2344fd983db 100644 --- a/htdocs/bookcal/calendar_card.php +++ b/htdocs/bookcal/calendar_card.php @@ -287,7 +287,7 @@ if (($id || $ref) && $action == 'edit') { if ($object->id > 0 && (empty($action) || ($action != 'edit' && $action != 'create'))) { $head = calendarPrepareHead($object); - print dol_get_fiche_head($head, 'card', $langs->trans("Calendar"), -1, $object->picto, 0, '', '', 0, '', 1); + print dol_get_fiche_head($head, 'card', $langs->trans("Calendar"), -1, $object->picto); $formconfirm = ''; diff --git a/htdocs/comm/action/card.php b/htdocs/comm/action/card.php index f891f4c38d9..954cbd10d3c 100644 --- a/htdocs/comm/action/card.php +++ b/htdocs/comm/action/card.php @@ -2659,7 +2659,7 @@ if ($id > 0 && $action != 'create') { print ''; } else { - print dol_get_fiche_head($head, 'card', $langs->trans("Action"), -1, 'action'); + print dol_get_fiche_head($head, 'card', $langs->trans("Action"), -1, 'action', 0, '', '', 0, '', 1); $formconfirm = ''; diff --git a/htdocs/compta/bank/various_payment/card.php b/htdocs/compta/bank/various_payment/card.php index ebdd2d23f8f..bb4aa5f009f 100644 --- a/htdocs/compta/bank/various_payment/card.php +++ b/htdocs/compta/bank/various_payment/card.php @@ -639,7 +639,7 @@ if ($id) { print $form->formconfirm(dolBuildUrl($_SERVER["PHP_SELF"], ['id' => $object->id]), $langs->trans('DeleteVariousPayment'), $text, 'confirm_delete', '', '', 2); } - print dol_get_fiche_head($head, 'card', $langs->trans("VariousPayment"), -1, $object->picto); + print dol_get_fiche_head($head, 'card', $langs->trans("VariousPayment"), -1, $object->picto, 0, '', '', 0, '', 1); $morehtmlref = '
'; // Project diff --git a/htdocs/compta/paiement/card.php b/htdocs/compta/paiement/card.php index d72b08f95d2..2d0dfb10320 100644 --- a/htdocs/compta/paiement/card.php +++ b/htdocs/compta/paiement/card.php @@ -291,7 +291,7 @@ $form = new Form($db); $head = payment_prepare_head($object); -print dol_get_fiche_head($head, 'payment', $langs->trans("PaymentCustomerInvoice"), -1, 'payment'); +print dol_get_fiche_head($head, 'payment', $langs->trans("PaymentCustomerInvoice"), -1, 'payment', 0, '', '', 0, '', 1); // Confirmation of payment delete if ($action == 'delete') { diff --git a/htdocs/compta/sociales/card.php b/htdocs/compta/sociales/card.php index 141a3e85cef..5aeb7a4a758 100644 --- a/htdocs/compta/sociales/card.php +++ b/htdocs/compta/sociales/card.php @@ -513,7 +513,7 @@ if ($id > 0) { } - print dol_get_fiche_head($head, 'card', $langs->trans("SocialContribution"), -1, $object->picto, 0, '', '', 0, '', 1); + print dol_get_fiche_head($head, 'card', $langs->trans("SocialContribution"), -1, $object->picto, 0, '', '', 0, '', ($action == 'edit' ? 0 : 1)); // Print form confirm print $formconfirm; diff --git a/htdocs/compta/tva/card.php b/htdocs/compta/tva/card.php index bade10f8669..ae9450594c1 100644 --- a/htdocs/compta/tva/card.php +++ b/htdocs/compta/tva/card.php @@ -583,7 +583,7 @@ if ($id > 0) { $formconfirm = $hookmanager->resPrint; } - print dol_get_fiche_head($head, 'card', $langs->trans("VATPayment"), -1, 'payment', 0, '', '', 0, '', 1); + print dol_get_fiche_head($head, 'card', $langs->trans("VATPayment"), -1, 'payment', 0, '', '', 0, '', ($action == 'edit' ? 0 : 1)); // Print form confirm print $formconfirm; diff --git a/htdocs/contact/card.php b/htdocs/contact/card.php index 9fe9199906a..8a2e7ac1633 100644 --- a/htdocs/contact/card.php +++ b/htdocs/contact/card.php @@ -1355,7 +1355,7 @@ if (is_object($objcanvas) && $objcanvas->displayCanvasExists($action)) { // Show errors dol_htmloutput_errors(is_numeric($error) ? '' : $error, $errors); - print dol_get_fiche_head($head, 'card', $title, -1, 'contact'); + print dol_get_fiche_head($head, 'card', $title, -1, 'contact', 0, '', '', 0, '', 1); if ($action == 'create_user') { // Full firstname and lastname separated with a dot : firstname.lastname diff --git a/htdocs/core/ajax/fileupload.php b/htdocs/core/ajax/fileupload.php index 353bab4111c..c8e64b51501 100644 --- a/htdocs/core/ajax/fileupload.php +++ b/htdocs/core/ajax/fileupload.php @@ -56,6 +56,17 @@ $elementupload = $element; // Load object according to $id and $element $object = fetchObjectByElement($id, $element); +// fetchObjectByElement() returns an object even when the record was not found, and it returns a +// GenericObject with no module when the element is unknown. In both cases restrictedArea() is then called +// with an empty feature, and it grants the access without checking any permission, so we must stop here. +// Note: fetchObjectByElement() may also return an int instead of an object when the module is disabled. +// We answer the same http code and the same message than a refusal by restrictedArea() below, so that a +// user can't tell an object that exists but is not allowed from an object that does not exist. +if (!is_object($object) || empty($object->id) || empty($object->module)) { + dol_syslog("fileupload.php object ".$element." with id ".$id." was not found or its element is not supported", LOG_WARNING); + httponly_accessforbidden('Not allowed'); +} + $module = $object->module; $element = $object->element; @@ -69,14 +80,21 @@ if ($usesublevelpermission && !$user->hasRight($module, $element)) { // There is // Security check if (!empty($user->socid)) { $socid = $user->socid; + // socid is not declared on CommonObject, which is the type fetchObjectByElement() answers, and an object + // that has no third party does not own the property at all. if (property_exists($object, 'socid') && !empty($object->socid) && $socid != $object->socid) { // @phan-suppress-current-line PhanUndeclaredProperty - httponly_accessforbidden("Access on object not allowed for this external user."); // This includes the exit. + // Same message than every other refusal of this page: a distinct one tells an external user that the + // object exists but belongs to another third party, which lets him enumerate the records of the others. + dol_syslog("fileupload.php object ".$element." with id ".$id." belongs to another third party than the external user", LOG_WARNING); + httponly_accessforbidden('Not allowed'); // This includes the exit. } } $result = restrictedArea($user, $object->module, $object, $object->table_element, $usesublevelpermission, 'fk_soc', 'rowid', 0, 1); // Call with mode return if (!$result) { - httponly_accessforbidden('Not allowed by restrictArea (module='.$object->module.' table_element='.$object->table_element.')'); + // The module and the table are reported into the log only, they must not be disclosed to the caller + dol_syslog("fileupload.php not allowed by restrictedArea (module=".$object->module." table_element=".$object->table_element.")", LOG_WARNING); + httponly_accessforbidden('Not allowed'); } @@ -103,7 +121,24 @@ switch ($_SERVER['REQUEST_METHOD']) { break; */ case 'POST': - $upload_handler = new FileUpload(null, $id, $elementupload); + // The constructor throws an exception when the element does not support file uploading, or when the + // object was not found. Answer with the same json contract than post() so the caller can show the + // error, instead of letting a fatal error return an http code 500 with no usable content. + try { + $upload_handler = new FileUpload(null, $id, $elementupload); + } catch (Exception $e) { + // Same http code 200 and same content type negotiation than post(), the error is reported into the + // json content as the caller expects. Forcing 'application/json' would make jQuery parse the content + // by itself, and the JSON.parse() of the caller would then fail on an already parsed array. + if (isset($_SERVER['HTTP_ACCEPT']) && (strpos($_SERVER['HTTP_ACCEPT'], 'application/json') !== false)) { + header('Content-type: application/json'); + } else { + header('Content-type: text/plain'); + } + echo json_encode(array(array('name' => '', 'error' => $e->getMessage()))); + $db->close(); + exit; + } /*if (isset($_REQUEST['_method']) && $_REQUEST['_method'] === 'DELETE') { $file = GETPOST('file'); diff --git a/htdocs/core/class/fileupload.class.php b/htdocs/core/class/fileupload.class.php index 7e2b2f7f387..938fdf7ab42 100644 --- a/htdocs/core/class/fileupload.class.php +++ b/htdocs/core/class/fileupload.class.php @@ -56,10 +56,11 @@ class FileUpload * @param ?array{script_url?:string,upload_dir?:string,upload_url?:string,param_name?:string,delete_type?:string,max_file_size?:?int,min_file_size?:int,accept_file_types?:string,max_number_of_files?:?int,max_width?:?int,max_height?:?int,min_width?:int,min_height?:int,discard_aborted_uploads?:bool,image_versions?:array} $options Options array * @param int $fk_element ID of element * @param string $element Code of element + * @throws Exception If the object was not found, if the element does not support file + * uploading, or if the upload directory is missing or not writable */ public function __construct($options = null, $fk_element = null, $element = null) { - global $db; global $hookmanager; $hookmanager->initHooks(array('fileupload')); @@ -77,11 +78,6 @@ class FileUpload //print 'fileupload.class.php: element='.$element.' pathname='.$pathname.' filename='.$filename.' dir_output='.$dir_output."\n"; - if (empty($dir_output)) { - setEventMessage('The element '.$element.' is not supported for uploading file. dir_output is unknown.', 'errors'); - throw new Exception('The element '.$element.' is not supported for uploading file. dir_output is unknown.'); - } - $object_ref = 'UndefinedReference'; $object = null; // If pathname and filename are null then we can still upload files if we have specified upload_dir on $options @@ -89,36 +85,62 @@ class FileUpload // Get object from its id and type $object = fetchObjectByElement($fk_element, $element); - $object_ref = dol_sanitizeFileName($object->ref); + // fetchObjectByElement() also returns an object when the record was not found (fetch() returning 0), + // so we must check the object was really loaded. Without this, files would be stored at the root of + // the module directory, out of any object and out of any permission check on the object. + if (!is_object($object) || empty($object->id)) { + dol_syslog(get_class($this)."::__construct object ".$element." with id ".((int) $fk_element)." was not found", LOG_WARNING); + throw new Exception('objectnotfound'); + } + + // Directory of the module, including the sub directory used by some elements (/sending for a shipment, + // /commande for a supplier order, / for a task, ...). We must use the same directory than + // the one used by the "Attached files" tab of the object, otherwise the uploaded file is stored but + // never shown to the user. + // Note: getMultidirOutput() only knows the elements of its own switch, that is a minority of them. For + // all the others it does not return an empty string but the string + // 'error-diroutput-not-defined-for-this-object=x', and keeping the directory of getElementProperties() + // is then the nominal case, not a degraded one. So we only accept an absolute path: that string is a + // relative path, and writing into it would create the files under the web root. + $tmpdir = getMultidirOutput($object, $element); + if (!empty($tmpdir) && preg_match('/^([a-z]:)?[\\\\\/]/i', $tmpdir)) { + $dir_output = dol_sanitizePathName($tmpdir); + } // Add object reference as file name prefix if const MAIN_DISABLE_SUGGEST_REF_AS_PREFIX is not enabled if (!getDolGlobalInt('MAIN_DISABLE_SUGGEST_REF_AS_PREFIX')) { - $savingDocMask = $object_ref . '-__file__'; + $savingDocMask = dol_sanitizeFileName($object->ref).'-__file__'; } - // Special cases to forge $object_ref used to forge $upload_dir - if ($element == 'invoice_supplier') { - $object_ref = get_exdir($object->id, 2, 0, 0, $object, 'invoice_supplier').$object_ref; - } elseif ($element == 'project_task') { - $parentForeignKey = 'fk_project'; - $parentClass = 'Project'; - $parentElement = 'projet'; - $parentObject = 'project'; + // get_exdir() forges the directory of an object the way the "Attached files" tabs do: it always + // uses the id for a thirdparty (a thirdparty ref is a company name, so it is not unique), and it + // falls back on the id when the ref is empty. Using anything else here would store the file into + // a directory the tab never reads. + // Note that a few tabs sanitize the ref themselves instead of calling this function, so they have + // no fallback: on an object whose ref is empty in database, which the interface does not produce + // but old records may hold, they read the root of the directory of the module while we store + // under the id. Storing at the root would mix the files of every object of the module, so the + // fallback is kept and those tabs are the ones that should be fixed. + $object_ref = get_exdir(0, 0, 0, 1, $object, $element); - dol_include_once('/'.$parentElement.'/class/'.$parentObject.'.class.php'); - $parent = new $parentClass($db); - if ($object->$parentForeignKey !== null) { - $parent->fetch((int) $object->$parentForeignKey); - if (!empty($parent->socid)) { - $parent->fetch_thirdparty(); - } - $object->$parentObject = clone $parent; - } - - $object_ref = dol_sanitizeFileName($object->project->ref).'/'.$object_ref; + // For the modules storing their documents on several levels, get_exdir() returned the level + // directories only, so we must append the directory of the object itself. + if (in_array($element, array('invoice_supplier', 'supplier_invoice'))) { + $object_ref .= '/'.dol_sanitizeFileName($object->ref); } } + // Tested after the call to getMultidirOutput(), because some elements have no 'dir_output' returned by + // getElementProperties() while getMultidirOutput() is still able to resolve their output directory. + if (empty($dir_output)) { + dol_syslog(get_class($this)."::__construct element ".$element." is not supported for uploading file, dir_output is unknown", LOG_WARNING); + throw new Exception('elementnotsupported'); + } + + // Note: 'upload_url' is not always the url of the file stored into 'upload_dir', because document.php + // forges the path of the file with its own rules for each value of modulepart. It is currently not a + // problem because the only caller of this class (the drag and drop of a file on a card) does not use + // the url returned into the json. $this->options = array( 'script_url' => $_SERVER['PHP_SELF'], 'upload_dir' => $dir_output.'/'.$object_ref.'/', @@ -458,6 +480,20 @@ class FileUpload $file->name = $fileNameWithPrefix; } + // trimFileName() checked the name is not already used, but it did it before the reference of the + // object was added as a prefix above, so it compared a name that is not the one we store. We must + // check it again on the final name, otherwise uploading twice the same file silently overwrites + // the first one, because dol_move_uploaded_file() is called below with $allowoverwrite = 1. + // The .noexe suffix is appended by dol_move_uploaded_file() on an executable file, so we must also + // look for the suffixed name, otherwise such a file is never seen as already existing and it is + // overwritten at each upload. + if ($this->options['discard_aborted_uploads']) { + $tmppath = dol_sanitizePathName($this->options['upload_dir']); + while (dol_is_file($tmppath.dol_sanitizeFileName($file->name)) || dol_is_file($tmppath.dol_sanitizeFileName($file->name).'.noexe')) { + $file->name = $this->upcountName($file->name); + } + } + $file_path = dol_sanitizePathName($this->options['upload_dir']).dol_sanitizeFileName($file->name); $append_file = !$this->options['discard_aborted_uploads'] && dol_is_file($file_path) && $file->size > dol_filesize($file_path); @@ -470,6 +506,14 @@ class FileUpload } else { // TODO Replace this with a call of dol_add_file_process(... $mode=1) $result = dol_move_uploaded_file($uploaded_file, $file_path, 1, 0, 0, 0, 'userfile'); + + // A return of 2 means the file was stored with a .noexe suffix appended on its name. + // We must follow that renaming, otherwise the size check below is done on a file that + // does not exist, and we report an error on a file that was correctly stored. + if ($result == 2) { + $file->name .= '.noexe'; + $file_path .= '.noexe'; + } } } else { // Non-multipart uploads (PUT method support) diff --git a/htdocs/core/lib/files.lib.php b/htdocs/core/lib/files.lib.php index 42c914411d7..5be5993d72c 100644 --- a/htdocs/core/lib/files.lib.php +++ b/htdocs/core/lib/files.lib.php @@ -4048,6 +4048,14 @@ function dragAndDropFileUpload($htmlname) { global $object, $langs; + // Every generated javascript string that carries an interpolated value is delimited by a single quote, so + // dol_escape_js() is called with the mode 1 everywhere below: it escapes a single quote and leaves a double + // quote alone. The default mode would rewrite a double quote into an escaped single quote, which is safe + // inside a '...' string but silently alters the value. + // dol_escape_js() escapes the quotes but not ''.img_picto("", 'download').'
'.$langs->trans("DropFileToAddItToObject").'
'; $out .= "\n\n"; @@ -4056,7 +4064,7 @@ function dragAndDropFileUpload($htmlname) jQuery(document).ready(function() { var enterTargetDragDrop = null; - $("#'.$htmlname.'").addClass("cssDragDropArea"); + $(\'#'.$htmlname.'\').addClass(\'cssDragDropArea\'); $(".cssDragDropArea").on("dragenter", function(ev, ui) { var dataTransfer = ev.originalEvent.dataTransfer; @@ -4074,7 +4082,7 @@ function dragAndDropFileUpload($htmlname) console.log("dragAndDropFileUpload: We add class highlightDragDropArea") enterTargetDragDrop = ev.target; $(this).addClass("highlightDragDropArea"); - $("#'.$htmlname.'Message").removeClass("hidden"); + $(\'#'.$htmlname.'Message\').removeClass(\'hidden\'); ev.preventDefault(); }); @@ -4082,7 +4090,7 @@ function dragAndDropFileUpload($htmlname) // Going out of drop area. Remove Highlight if (enterTargetDragDrop == ev.target){ console.log("dragAndDropFileUpload: We remove class highlightDragDropArea") - $("#'.$htmlname.'Message").addClass("hidden"); + $(\'#'.$htmlname.'Message\').addClass(\'hidden\'); $(this).removeClass("highlightDragDropArea"); } }); @@ -4093,12 +4101,12 @@ function dragAndDropFileUpload($htmlname) }); $(".cssDragDropArea").on("drop", function(e) { - console.log("Trigger event file dropped. fk_element='.dol_escape_js((string) $object->id).' element='.dol_escape_js($object->element).'"); + console.log(\'Trigger event file dropped. fk_element='.dol_escape_js((string) $object->id, 1).' element='.dol_escape_js($object->element, 1).'\'); e.preventDefault(); fd = new FormData(); - fd.append("fk_element", "'.dol_escape_js((string) $object->id).'"); - fd.append("element", "'.dol_escape_js($object->element).'"); - fd.append("token", "'.currentToken().'"); + fd.append(\'fk_element\', \''.dol_escape_js((string) $object->id, 1).'\'); + fd.append(\'element\', \''.dol_escape_js($object->element, 1).'\'); + fd.append(\'token\', \''.currentToken().'\'); fd.append("action", "linkit"); var dataTransfer = e.originalEvent.dataTransfer; @@ -4112,7 +4120,7 @@ function dragAndDropFileUpload($htmlname) $(".cssDragDropArea").removeClass("highlightDragDropArea"); counterdragdrop = 0; $.ajax({ - url: "'.DOL_URL_ROOT.'/core/ajax/fileupload.php", + url: \''.DOL_URL_ROOT.'/core/ajax/fileupload.php\', type: "POST", processData: false, contentType: false, @@ -4121,7 +4129,16 @@ function dragAndDropFileUpload($htmlname) console.log("Uploaded.", arguments); /* arguments[0] is the json string of files */ /* arguments[1] is the value for variable "success", can be 0 or 1 */ - let listoffiles = JSON.parse(arguments[0]); + let listoffiles = []; + /* The answer is not the expected json when php stopped before answering, for example when + post_max_size was reached. Without this, the exception of JSON.parse() would leave the + user on a page with no message at all, thinking the file was added. */ + try { + listoffiles = JSON.parse(arguments[0]); + } catch (e) { + window.location.href = \''.$pageurl.'?id='.dol_escape_js((string) $object->id, 1).'&seteventmessages=ErrorUploadFileDragDrop:errors\'; + return; + } console.log(listoffiles); let nboferror = 0; for (let i = 0; i < listoffiles.length; i++) { @@ -4131,18 +4148,23 @@ function dragAndDropFileUpload($htmlname) } } console.log(nboferror); - if (nboferror > 0) { - window.location.href = "'.$_SERVER["PHP_SELF"].'?id='.dol_escape_js((string) $object->id).'&seteventmessages=ErrorOnAtLeastOneFileUpload:warnings"; + /* An empty list means no file was stored at all, so it is an error and not a success: + php empties $_FILES when post_max_size is reached. */ + if (listoffiles.length == 0) { + window.location.href = \''.$pageurl.'?id='.dol_escape_js((string) $object->id, 1).'&seteventmessages=ErrorUploadFileDragDrop:errors\'; + } else if (nboferror > 0) { + window.location.href = \''.$pageurl.'?id='.dol_escape_js((string) $object->id, 1).'&seteventmessages=ErrorOnAtLeastOneFileUpload:warnings\'; } else { - window.location.href = "'.$_SERVER["PHP_SELF"].'?id='.dol_escape_js((string) $object->id).'&seteventmessages=UploadFileDragDropSuccess:mesgs"; + window.location.href = \''.$pageurl.'?id='.dol_escape_js((string) $object->id, 1).'&seteventmessages=UploadFileDragDropSuccess:mesgs\'; } }, - error:function() { + error:function(jqXHR) { console.log("Error Uploading.", arguments) - if (arguments[0].status == 403) { - window.location.href = "'.$_SERVER["PHP_SELF"].'?id='.dol_escape_js((string) $object->id).'&seteventmessages=ErrorUploadPermissionDenied:errors"; + if (jqXHR.status == 403) { + window.location.href = \''.$pageurl.'?id='.dol_escape_js((string) $object->id, 1).'&seteventmessages=ErrorUploadFileDragDropPermissionDenied:errors\'; + } else { + window.location.href = \''.$pageurl.'?id='.dol_escape_js((string) $object->id, 1).'&seteventmessages=ErrorUploadFileDragDrop:errors\'; } - window.location.href = "'.$_SERVER["PHP_SELF"].'?id='.dol_escape_js((string) $object->id).'&seteventmessages=ErrorUploadFileDragDropPermissionDenied:errors"; }, }) }); diff --git a/htdocs/core/lib/functions.lib.php b/htdocs/core/lib/functions.lib.php index cbbd89b7e46..d8a62613fa4 100644 --- a/htdocs/core/lib/functions.lib.php +++ b/htdocs/core/lib/functions.lib.php @@ -187,6 +187,12 @@ function getMultidirOutput($object, $module = '', $forobject = 0, $mode = 'outpu $module = 'knowledgemanagement'; $subdirectory = '/knowledgerecord'; break; + case 'partnership': + $subdirectory = '/partnership'; + break; + case 'stocktransfer': + $subdirectory = '/stocktransfer'; + break; case 'commande_fournisseur': $module = 'fournisseur'; $subdirectory = '/commande'; @@ -207,10 +213,20 @@ function getMultidirOutput($object, $module = '', $forobject = 0, $mode = 'outpu case 'project_task': $module = 'projet'; - // Fetch the project to build the correct path - $object->fetchProject(); + // Fetch the project to build the correct path. The signature of this function accepts an object + // that is not a CommonObject, and even a null when a module is given, so we must not call a method + // that only a CommonObject owns without testing it exists. + if (is_object($object) && method_exists($object, 'fetchProject')) { + $object->fetchProject(); + } - $subdirectory = '/'.$object->project->ref; + // The ref must be sanitized with dol_sanitizeFileName() and not only with dol_sanitizePathName() + // done at the end of this function, because a project ref is a user input that may contain a '/', + // a ':' or an accented char. dol_sanitizePathName() keeps them, so we would not return the + // directory used by projet/tasks/document.php, that sanitizes the ref with dol_sanitizeFileName(). + if (!empty($object->project->ref)) { + $subdirectory = '/'.dol_sanitizeFileName($object->project->ref); + } break; case 'action': case 'actioncomm': @@ -226,7 +242,13 @@ function getMultidirOutput($object, $module = '', $forobject = 0, $mode = 'outpu if (isset($conf->$module) && property_exists($conf->$module, 'multidir_output')) { $s = ''; if ($mode != 'outputrel') { - $s = $conf->$module->multidir_output[(empty($object->entity) ? $conf->entity : $object->entity)] . $subdirectory; + // An entity with no directory declared used to return an undefined index, so a relative path + // that made the caller read or write under the web root. Answer the error instead. + $entity = (int) (empty($object->entity) ? $conf->entity : $object->entity); + if (!isset($conf->$module->multidir_output[$entity])) { + return 'error-diroutput-not-defined-for-this-object='.$module; + } + $s = $conf->$module->multidir_output[$entity].$subdirectory; } if ($forobject && $object->id > 0) { $s .= ($mode != 'outputrel' ? '/' : '') . get_exdir(0, 0, 0, 0, $object); @@ -246,7 +268,12 @@ function getMultidirOutput($object, $module = '', $forobject = 0, $mode = 'outpu } } elseif ($mode == 'temp') { if (isset($conf->$module) && property_exists($conf->$module, 'multidir_temp')) { - return dol_sanitizePathName($conf->$module->multidir_temp[(empty($object->entity) ? $conf->entity : $object->entity)]); + // Same guard as the 'output' mode above, see the comment there + $entity = (int) (empty($object->entity) ? $conf->entity : $object->entity); + if (!isset($conf->$module->multidir_temp[$entity])) { + return 'error-dirtemp-not-defined-for-this-object='.$module; + } + return dol_sanitizePathName($conf->$module->multidir_temp[$entity]); } elseif (isset($conf->$module) && property_exists($conf->$module, 'dir_temp')) { return dol_sanitizePathName($conf->$module->dir_temp); } else { @@ -15243,6 +15270,7 @@ function getElementProperties($elementType) $module = 'societe'; $subelement = 'contact'; $table_element = 'socpeople'; + $subdir = '/contact'; } elseif ($elementType == 'inventory') { $module = 'product'; $classpath = 'product/inventory/class'; @@ -15458,6 +15486,45 @@ function getElementProperties($elementType) $classfile = 'paymentsalary'; $classname = 'PaymentSalary'; $module = 'salaries'; + } elseif ($elementType == 'payment') { + $classpath = 'compta/paiement/class'; + $classfile = 'paiement'; + $classname = 'Paiement'; + $module = 'facture'; // A customer payment belongs to the invoice module, there is no 'compta' module + $element = 'payment'; + $subelement = 'payment'; + $table_element = 'paiement'; + } elseif ($elementType == 'payment_supplier') { + $classpath = 'fourn/class'; + $classfile = 'paiementfourn'; + $classname = 'PaiementFourn'; + $module = 'fournisseur'; + $element = 'payment_supplier'; + $subelement = 'payment_supplier'; + $table_element = 'paiementfourn'; + } elseif ($elementType == 'payment_various') { + $classpath = 'compta/bank/class'; + $classfile = 'paymentvarious'; + $classname = 'PaymentVarious'; + $module = 'bank'; // We need $conf->bank->dir_output and not $conf->banque->dir_output + $element = 'payment_various'; + $subelement = 'payment_various'; + $table_element = 'payment_various'; + } elseif ($elementType == 'stocktransfer') { + $classpath = 'product/stock/stocktransfer/class'; + $classfile = 'stocktransfer'; + $classname = 'StockTransfer'; // Not the ucfirst() of the element, so it must be set explicitly + $module = 'stocktransfer'; + $subelement = 'stocktransfer'; + $table_element = 'stocktransfer_stocktransfer'; + } elseif ($elementType == 'job' || $elementType == 'position' || $elementType == 'skill' || $elementType == 'evaluation') { + $classpath = 'hrm/class'; + $classfile = $elementType; + $classname = ucfirst($elementType); + $module = 'hrm'; + $subelement = $elementType; + $table_element = ($elementType == 'position' ? 'hrm_job_user' : 'hrm_'.$elementType); + $subdir = '/'.$elementType; } elseif ($elementType == 'productlot') { $module = 'productbatch'; $classpath = 'product/stock/class'; @@ -15508,6 +15575,7 @@ function getElementProperties($elementType) $classfile = 'conferenceorbooth'; $classname = 'ConferenceOrBooth'; $module = 'eventorganization'; + $subdir = '/conferenceorbooth'; } elseif ($elementType == 'ccountry') { $module = ''; $classpath = 'core/class'; @@ -15597,9 +15665,24 @@ function getElementProperties($elementType) } elseif ($element == 'invoice_supplier' && isModEnabled('fournisseur')) { $dir_output = $conf->fournisseur->facture->dir_output; $dir_temp = $conf->fournisseur->facture->dir_temp; + } elseif ($elementType == 'payment' && isModEnabled('invoice') && isset($conf->compta->payment)) { + // A customer payment is stored into a sub object of $conf, not handled by the generic case. + // Note: we must test $elementType and not $element, because the 'myobject_mysubobject' rule above + // rewrites $element to 'payment' for the element 'payment_salary' too, which is stored elsewhere. + $dir_output = $conf->compta->payment->dir_output; + $dir_temp = $conf->compta->payment->dir_temp; + } elseif ($elementType == 'payment_supplier' && isModEnabled('fournisseur') && isset($conf->fournisseur->payment)) { + $dir_output = $conf->fournisseur->payment->dir_output; + $dir_temp = $conf->fournisseur->payment->dir_temp; + } + // The sub directory must not be appended when the module is disabled, because $dir_output is then empty + // and we would return a path at the root of the file system instead of an empty string. + if (!empty($dir_output)) { + $dir_output .= $subdir; + } + if (!empty($dir_temp)) { + $dir_temp .= $subdir; } - $dir_output .= $subdir; - $dir_temp .= $subdir; $elementProperties = array( 'module' => $module, diff --git a/htdocs/core/lib/security.lib.php b/htdocs/core/lib/security.lib.php index 803beaba888..4de3acb9e5f 100644 --- a/htdocs/core/lib/security.lib.php +++ b/htdocs/core/lib/security.lib.php @@ -227,6 +227,24 @@ function restrictedArea(User $user, $features, $object = 0, $tableandshare = '', $parentfortableentity = 'fk_website@website'; } elseif ($features == 'project') { $features = 'projet'; + } elseif ($features == 'eventorganization' && is_object($object) && $object->element == 'conferenceorbooth') { + // The module of an event organization declares no permission of its own, on purpose, so a check on + // 'eventorganization' is refused to everyone, an administrator included. Check the parent project + // instead, which is what the card of the object does itself. + // The card refuses an external user before that check, and fk_project is nullable, so we must refuse + // both cases here too: with no parent project there is nothing left to check the access on, and + // granting it would be an access with no check at all. + if (!empty($user->socid) || empty($object->fk_project)) { + if ($mode) { + return 0; + } else { + accessforbidden(); + } + } + $features = 'projet'; + $tableandshare = 'projet&project'; + $objectid = (int) $object->fk_project; + $object = $objectid; } elseif ($features == 'product') { $features = 'produit'; } elseif ($features == 'productbatch') { @@ -235,6 +253,10 @@ function restrictedArea(User $user, $features, $object = 0, $tableandshare = '', $feature2 = 'charges'; } elseif ($features == 'workstation') { $feature2 = 'workstation'; + } elseif ($features == 'hrm' && is_object($object) && in_array($object->element, array('job', 'position', 'skill'))) { + $feature2 = 'all'; // These 3 objects have no permission of their own, they share the level "all" + } elseif ($features == 'stocktransfer' && is_object($object) && $object->element == 'stocktransfer') { + $feature2 = 'stocktransfer'; // This module declares no permission at its first level, only this one } elseif ($features == 'fournisseur') { // When vendor invoice and purchase order are into module 'fournisseur' if (is_object($object) && $object->element == 'invoice_supplier') { $feature2 = 'facture'; @@ -783,6 +805,30 @@ function checkUserAccessToObject($user, array $featuresarray, $object = 0, $tabl $sharedelement = (!empty($params[1]) ? $params[1] : $dbtablename); // We change dbtablename, so we set sharedelement too. } + // The default rule reads the columns entity and $dbt_keyfield of the table, but some tables own neither of + // them. The sql was then built on columns that do not exist, so it always failed and the access was refused + // to the users that this rule applies to. + // The rule is selected on the table and not on the element of the object, because $object is an id and not + // an object for most of the callers, the card of an asset and the card of a workstation included. + if (!empty($objectid) && in_array($dbtablename, array('asset', 'paiement', 'paiementfourn', 'workstation_workstation', 'hrm_job', 'hrm_job_user', 'hrm_skill'))) { + // None of these objects is linked to a third party, so an external user can own none of them. The + // default rule refused him through a link that does not exist, we must refuse him explicitly instead, + // otherwise the rules below, which do not look at the third party of the user at all, would grant it. + if (!empty($user->socid)) { + return false; + } + if (in_array($dbtablename, array('hrm_job', 'hrm_job_user', 'hrm_skill'))) { + // These 3 tables have no entity column either, so no rule that reads the table can be run on them. + // The permission is still checked by restrictedArea(), and the $checkhierarchy rule below still runs. + // Note that these 3 objects are therefore not partitioned between entities at all, in the database + // itself: their cards already answer to a user of another entity, and their lists already show the + // records of all of them. This rule does not widen that, it aligns with it. + $nocheck[] = $feature; + } else { + $check[] = $feature; // Test on the entity only, there is no third party to restrict on + } + } + // $objectid was already sanitized at begin of this method (can be an int or a list of int separated by comma). // To avoid an access forbidden with a numeric ref if ($dbt_select != 'rowid' && $dbt_select != 'id') { diff --git a/htdocs/don/card.php b/htdocs/don/card.php index 61c80893a9c..9ad1ed259d8 100644 --- a/htdocs/don/card.php +++ b/htdocs/don/card.php @@ -701,7 +701,7 @@ if (!empty($id) && $action != 'edit') { $hselected = 'card'; $head = donation_prepare_head($object); - print dol_get_fiche_head($head, $hselected, $langs->trans("Donation"), -1, 'donation'); + print dol_get_fiche_head($head, $hselected, $langs->trans("Donation"), -1, 'donation', 0, '', '', 0, '', 1); // Print form confirm print $formconfirm; diff --git a/htdocs/eventorganization/conferenceorbooth_card.php b/htdocs/eventorganization/conferenceorbooth_card.php index a1a6adbe8bf..e80d786139b 100644 --- a/htdocs/eventorganization/conferenceorbooth_card.php +++ b/htdocs/eventorganization/conferenceorbooth_card.php @@ -534,7 +534,7 @@ if ($object->id > 0 && (empty($action) || ($action != 'edit' && $action != 'crea $head = conferenceorboothPrepareHead($object, $withproject); - print dol_get_fiche_head($head, 'card', $langs->trans("ConferenceOrBooth"), -1, $object->picto); + print dol_get_fiche_head($head, 'card', $langs->trans("ConferenceOrBooth"), -1, $object->picto, 0, '', '', 0, '', 1); $formconfirm = ''; diff --git a/htdocs/expedition/card.php b/htdocs/expedition/card.php index 56bbe562258..c4931b3a7b3 100644 --- a/htdocs/expedition/card.php +++ b/htdocs/expedition/card.php @@ -2687,7 +2687,7 @@ if ($action == 'create' && $usercancreate) { $res = $object->fetch_optionals(); $head = shipping_prepare_head($object); - print dol_get_fiche_head($head, 'shipping', $langs->trans("Shipment"), -1, $object->picto); + print dol_get_fiche_head($head, 'shipping', $langs->trans("Shipment"), -1, $object->picto, 0, '', '', 0, '', 1); $formconfirm = ''; diff --git a/htdocs/expensereport/card.php b/htdocs/expensereport/card.php index 91916cadd9a..1d18ec5d37a 100644 --- a/htdocs/expensereport/card.php +++ b/htdocs/expensereport/card.php @@ -1621,7 +1621,7 @@ if ($action == 'create') { print ''; print ''; - print dol_get_fiche_head($head, 'card', $langs->trans("ExpenseReport"), 0, 'trip', 0, '', '', 0, '', 1); + print dol_get_fiche_head($head, 'card', $langs->trans("ExpenseReport"), 0, 'trip', 0, '', '', 0, '', 0); // No drag and drop on the edit form, dropping a file reloads the page and discards it if ($object->status == ExpenseReport::STATUS_REFUSED) { print ''; diff --git a/htdocs/fourn/paiement/card.php b/htdocs/fourn/paiement/card.php index e21d842c69f..fb19a1d9d91 100644 --- a/htdocs/fourn/paiement/card.php +++ b/htdocs/fourn/paiement/card.php @@ -174,7 +174,9 @@ $formfile = new FormFile($db); $head = payment_supplier_prepare_head($object); -print dol_get_fiche_head($head, 'payment', $langs->trans('SupplierPayment'), -1, 'payment'); +// The tabs are printed before the result of the fetch is checked below, so we must not offer a drop area +// when the object was not loaded: the upload could only fail. +print dol_get_fiche_head($head, 'payment', $langs->trans('SupplierPayment'), -1, 'payment', 0, '', '', 0, '', ($result > 0 ? 1 : 0)); if ($result > 0) { /* diff --git a/htdocs/holiday/card.php b/htdocs/holiday/card.php index 7bbdeaa4307..d3feb4c19c2 100644 --- a/htdocs/holiday/card.php +++ b/htdocs/holiday/card.php @@ -1334,7 +1334,9 @@ if ((empty($id) && empty($ref)) || $action == 'create' || $action == 'add') { if ($canread) { $head = holiday_prepare_head($object); - if (($action == 'edit' && $object->status == Holiday::STATUS_DRAFT) || ($action == 'editvalidator')) { + $editmode = (($action == 'edit' && $object->status == Holiday::STATUS_DRAFT) || ($action == 'editvalidator')); + + if ($editmode) { if ($action == 'edit' && $object->status == Holiday::STATUS_DRAFT) { $edit = true; } @@ -1345,7 +1347,9 @@ if ((empty($id) && empty($ref)) || $action == 'create' || $action == 'add') { print ''."\n"; } - print dol_get_fiche_head($head, 'card', $langs->trans("CPTitreMenu"), -1, 'holiday'); + // No drop area in edit mode: these tabs are printed inside the edit form, and dropping a file + // reloads the page, which would discard what the user is typing. + print dol_get_fiche_head($head, 'card', $langs->trans("CPTitreMenu"), -1, 'holiday', 0, '', '', 0, '', ($editmode ? 0 : 1)); $linkback = ''.$langs->trans("BackToList").''; diff --git a/htdocs/hrm/evaluation_card.php b/htdocs/hrm/evaluation_card.php index 52c7193c17f..7450064fa74 100644 --- a/htdocs/hrm/evaluation_card.php +++ b/htdocs/hrm/evaluation_card.php @@ -358,7 +358,7 @@ if ($object->id > 0 && (empty($action) || ($action != 'edit' && $action != 'crea $res = $object->fetch_optionals(); $head = evaluationPrepareHead($object); - print dol_get_fiche_head($head, 'card', $langs->trans("Workstation"), -1, $object->picto); + print dol_get_fiche_head($head, 'card', $langs->trans("Workstation"), -1, $object->picto, 0, '', '', 0, '', 1); $formconfirm = ''; diff --git a/htdocs/hrm/job_card.php b/htdocs/hrm/job_card.php index 59fc1aa5c20..785a06744b4 100644 --- a/htdocs/hrm/job_card.php +++ b/htdocs/hrm/job_card.php @@ -314,7 +314,7 @@ if ($object->id > 0 && (empty($action) || ($action != 'edit' && $action != 'crea $head = jobPrepareHead($object); $picto = 'company.png'; - print dol_get_fiche_head($head, 'job_card', $langs->trans("Workstation"), -1, $object->picto); + print dol_get_fiche_head($head, 'job_card', $langs->trans("Workstation"), -1, $object->picto, 0, '', '', 0, '', 1); $formconfirm = ''; diff --git a/htdocs/hrm/position_card.php b/htdocs/hrm/position_card.php index a171fa9a8a9..1285a99db31 100644 --- a/htdocs/hrm/position_card.php +++ b/htdocs/hrm/position_card.php @@ -271,7 +271,7 @@ function displayPositionCard(&$object) $head = positionCardPrepareHead($object); - print dol_get_fiche_head($head, 'position', $langs->trans("Workstation"), -1, $object->picto); + print dol_get_fiche_head($head, 'position', $langs->trans("Workstation"), -1, $object->picto, 0, '', '', 0, '', 1); $formconfirm = ''; diff --git a/htdocs/hrm/skill_card.php b/htdocs/hrm/skill_card.php index 106b1e6072f..3b5c6dad78a 100644 --- a/htdocs/hrm/skill_card.php +++ b/htdocs/hrm/skill_card.php @@ -430,7 +430,7 @@ if ($object->id > 0 && (empty($action) || ($action != 'edit' && $action != 'crea $res = $object->fetch_optionals(); $head = skillPrepareHead($object); - print dol_get_fiche_head($head, 'card', $langs->trans("Workstation"), -1, $object->picto); + print dol_get_fiche_head($head, 'card', $langs->trans("Workstation"), -1, $object->picto, 0, '', '', 0, '', 1); $formconfirm = ''; diff --git a/htdocs/intracommreport/card.php b/htdocs/intracommreport/card.php index c6288c30ab1..748325c23a2 100644 --- a/htdocs/intracommreport/card.php +++ b/htdocs/intracommreport/card.php @@ -284,7 +284,7 @@ if (($id || $ref) && $action == 'edit') { if ($object->id > 0 && (empty($action) || ($action != 'edit' && $action != 'create'))) { $head = intracommreportPrepareHead($object); - print dol_get_fiche_head($head, 'card', $langs->trans("IntraCommReport"), -1, $object->picto, 0, '', '', 0, '', 1); + print dol_get_fiche_head($head, 'card', $langs->trans("IntraCommReport"), -1, $object->picto); $formconfirm = ''; diff --git a/htdocs/knowledgemanagement/knowledgerecord_card.php b/htdocs/knowledgemanagement/knowledgerecord_card.php index 6fbb511b928..3f10add8e04 100644 --- a/htdocs/knowledgemanagement/knowledgerecord_card.php +++ b/htdocs/knowledgemanagement/knowledgerecord_card.php @@ -300,7 +300,7 @@ if ($object->id > 0 && (empty($action) || ($action != 'edit' && $action != 'crea $res = $object->fetch_optionals(); $head = knowledgerecordPrepareHead($object); - print dol_get_fiche_head($head, 'card', $langs->trans("KnowledgeRecord"), -1, $object->picto); + print dol_get_fiche_head($head, 'card', $langs->trans("KnowledgeRecord"), -1, $object->picto, 0, '', '', 0, '', 1); $formconfirm = ''; diff --git a/htdocs/langs/en_US/errors.lang b/htdocs/langs/en_US/errors.lang index b5ac29a3140..412d290fc3a 100644 --- a/htdocs/langs/en_US/errors.lang +++ b/htdocs/langs/en_US/errors.lang @@ -334,6 +334,7 @@ ErrorFieldValue=Value for %s is incorrect ErrorCoherenceMenu=%s is required when %s is 'left' ErrorUploadFileDragDrop=There was an error while the file(s) upload ErrorUploadFileDragDropPermissionDenied=There was an error while the file(s) upload : Permission denied +ErrorOnAtLeastOneFileUpload=There was an error while uploading at least one file ErrorFixThisHere=Fix this here ErrorTheUrlOfYourDolInstanceDoesNotMatchURLIntoOAuthSetup=Error: The URL of you current instance (%s) does not match the URL defined into your OAuth2 login setup (%s). Doing OAuth2 login in such a configuration is not allowed. ErrorMenuExistValue=A Menu already exist with this Title or URL diff --git a/htdocs/loan/card.php b/htdocs/loan/card.php index 70e81f7bb42..6a5c407e574 100644 --- a/htdocs/loan/card.php +++ b/htdocs/loan/card.php @@ -442,7 +442,7 @@ if ($id > 0) { print ''; } - print dol_get_fiche_head($head, 'card', $langs->trans("Loan"), -1, 'money-bill-alt', 0, '', '', 0, '', 1); + print dol_get_fiche_head($head, 'card', $langs->trans("Loan"), -1, 'money-bill-alt', 0, '', '', 0, '', ($action == 'edit' ? 0 : 1)); // Loan card $linkback = ''.$langs->trans("BackToList").''; diff --git a/htdocs/mrp/mo_card.php b/htdocs/mrp/mo_card.php index 76b012b242e..04791cb8a68 100644 --- a/htdocs/mrp/mo_card.php +++ b/htdocs/mrp/mo_card.php @@ -584,7 +584,7 @@ if ($object->id > 0 && (empty($action) || ($action != 'edit' && $action != 'crea $head = moPrepareHead($object); - print dol_get_fiche_head($head, 'card', $langs->trans("ManufacturingOrder"), -1, $object->picto); + print dol_get_fiche_head($head, 'card', $langs->trans("ManufacturingOrder"), -1, $object->picto, 0, '', '', 0, '', 1); $formconfirm = ''; diff --git a/htdocs/partnership/partnership_card.php b/htdocs/partnership/partnership_card.php index 9bb895dba7a..926ff24fff4 100644 --- a/htdocs/partnership/partnership_card.php +++ b/htdocs/partnership/partnership_card.php @@ -357,7 +357,7 @@ if (($id || $ref) && $action == 'edit') { if ($object->id > 0 && (empty($action) || ($action != 'edit' && $action != 'create'))) { $head = partnershipPrepareHead($object); - print dol_get_fiche_head($head, 'card', $langs->trans("Partnership"), -1, $object->picto); + print dol_get_fiche_head($head, 'card', $langs->trans("Partnership"), -1, $object->picto, 0, '', '', 0, '', 1); $formconfirm = ''; diff --git a/htdocs/product/card.php b/htdocs/product/card.php index 407d70528bc..393c9278d17 100644 --- a/htdocs/product/card.php +++ b/htdocs/product/card.php @@ -2075,7 +2075,7 @@ if (is_object($objcanvas) && $objcanvas->displayCanvasExists($canvasdisplayactio $head = product_prepare_head($object); $titre = $langs->trans("CardProduct".$object->type); $picto = ($object->type == Product::TYPE_SERVICE ? 'service' : 'product'); - print dol_get_fiche_head($head, 'card', $titre, 0, $picto, 0, '', '', 0, '', 1); + print dol_get_fiche_head($head, 'card', $titre, 0, $picto, 0, '', '', 0, '', 0); // No drag and drop on the edit form, dropping a file reloads the page and discards it // Call Hook tabContentEditProduct $parameters = array(); diff --git a/htdocs/product/stock/productlot_card.php b/htdocs/product/stock/productlot_card.php index 29b3238dadc..14426d4fe91 100644 --- a/htdocs/product/stock/productlot_card.php +++ b/htdocs/product/stock/productlot_card.php @@ -344,7 +344,7 @@ if ($object->id > 0 && (empty($action) || ($action != 'edit' && $action != 'crea $res = $object->fetch_optionals(); $head = productlot_prepare_head($object); - print dol_get_fiche_head($head, 'card', $langs->trans("Batch"), -1, $object->picto); + print dol_get_fiche_head($head, 'card', $langs->trans("Batch"), -1, $object->picto, 0, '', '', 0, '', 1); $formconfirm = ''; diff --git a/htdocs/product/stock/stocktransfer/stocktransfer_card.php b/htdocs/product/stock/stocktransfer/stocktransfer_card.php index 744d2a75bc1..a68cdf027ff 100644 --- a/htdocs/product/stock/stocktransfer/stocktransfer_card.php +++ b/htdocs/product/stock/stocktransfer/stocktransfer_card.php @@ -548,7 +548,7 @@ if ($object->id > 0 && (empty($action) || ($action != 'edit' && $action != 'crea $head = stocktransferPrepareHead($object); - print dol_get_fiche_head($head, 'card', $langs->trans("StockTransfer"), -1, $object->picto); + print dol_get_fiche_head($head, 'card', $langs->trans("StockTransfer"), -1, $object->picto, 0, '', '', 0, '', 1); $formconfirm = ''; diff --git a/htdocs/projet/tasks/task.php b/htdocs/projet/tasks/task.php index 3be6271041c..2bc963842bf 100644 --- a/htdocs/projet/tasks/task.php +++ b/htdocs/projet/tasks/task.php @@ -598,7 +598,7 @@ if ($id > 0 || !empty($ref)) { $param = ($withproject ? '&withproject=1' : ''); $linkback = $withproject ? ''.$langs->trans("BackToList").'' : ''; - print dol_get_fiche_head($head, 'task_task', $langs->trans("Task"), -1, 'projecttask', 0, '', 'reposition'); + print dol_get_fiche_head($head, 'task_task', $langs->trans("Task"), -1, 'projecttask', 0, '', 'reposition', 0, '', 1); if ($action == 'clone') { $formquestion = array( diff --git a/htdocs/reception/card.php b/htdocs/reception/card.php index a989e9ec06c..c3b4ccc3c24 100644 --- a/htdocs/reception/card.php +++ b/htdocs/reception/card.php @@ -2011,7 +2011,7 @@ if ($action == 'create' && $permissiontoadd) { $res = $object->fetch_optionals(); $head = reception_prepare_head($object); - print dol_get_fiche_head($head, 'reception', $langs->trans("Reception"), -1, 'dollyrevert'); + print dol_get_fiche_head($head, 'reception', $langs->trans("Reception"), -1, 'dollyrevert', 0, '', '', 0, '', 1); $formconfirm = ''; diff --git a/htdocs/resource/card.php b/htdocs/resource/card.php index 949ecc18a24..e202b36a827 100644 --- a/htdocs/resource/card.php +++ b/htdocs/resource/card.php @@ -250,7 +250,7 @@ if ($action == 'create' || $object->fetch($id, $ref) > 0) { print dol_get_fiche_head(); } else { $head = resource_prepare_head($object); - print dol_get_fiche_head($head, 'resource', $title, -1, 'resource'); + print dol_get_fiche_head($head, 'resource', $title, -1, 'resource', 0, '', '', 0, '', ($action == 'edit' ? 0 : 1)); } if ($action == 'create' || $action == 'edit') { diff --git a/htdocs/salaries/card.php b/htdocs/salaries/card.php index 6f09059997b..0a3a8443e5e 100644 --- a/htdocs/salaries/card.php +++ b/htdocs/salaries/card.php @@ -896,7 +896,7 @@ if ($id > 0) { print $formconfirm; - print dol_get_fiche_head($head, 'card', $langs->trans("SalaryPayment"), -1, 'salary', 0, '', '', 0, '', 1); + print dol_get_fiche_head($head, 'card', $langs->trans("SalaryPayment"), -1, 'salary', 0, '', '', 0, '', ($action == 'edit' ? 0 : 1)); $linkback = ''.$langs->trans("BackToList").''; diff --git a/htdocs/webhook/triggerhistory_card.php b/htdocs/webhook/triggerhistory_card.php index 4cc30361323..74de6d17ebd 100644 --- a/htdocs/webhook/triggerhistory_card.php +++ b/htdocs/webhook/triggerhistory_card.php @@ -263,7 +263,7 @@ if (($id || $ref) && $action == 'edit') { if ($object->id > 0 && (empty($action) || ($action != 'edit' && $action != 'create'))) { $head = triggerhistoryPrepareHead($object); - print dol_get_fiche_head($head, 'card', $langs->trans(""), -1, $object->picto, 0, '', '', 0, '', 1); + print dol_get_fiche_head($head, 'card', $langs->trans(""), -1, $object->picto); $formconfirm = ''; diff --git a/htdocs/workstation/workstation_card.php b/htdocs/workstation/workstation_card.php index f57a7dafe1d..c4e31df147e 100644 --- a/htdocs/workstation/workstation_card.php +++ b/htdocs/workstation/workstation_card.php @@ -312,7 +312,7 @@ if (($id || $ref) && $action == 'edit') { if ($object->id > 0 && (empty($action) || ($action != 'edit' && $action != 'create'))) { $head = workstationPrepareHead($object); - print dol_get_fiche_head($head, 'card', $langs->trans("Workstation"), -1, $object->picto); + print dol_get_fiche_head($head, 'card', $langs->trans("Workstation"), -1, $object->picto, 0, '', '', 0, '', 1); $formconfirm = ''; diff --git a/test/phpunit/FileUploadTest.php b/test/phpunit/FileUploadTest.php new file mode 100644 index 00000000000..c52f4116f60 --- /dev/null +++ b/test/phpunit/FileUploadTest.php @@ -0,0 +1,516 @@ +. + * or see https://www.gnu.org/ + */ + +/** + * \file test/phpunit/FileUploadTest.php + * \ingroup test + * \brief PHPUnit test of the class FileUpload, used by the drag and drop of a file on a card. + * \remarks To run this script as CLI: phpunit FileUploadTest.php < /dev/null + * The redirection of stdin is only needed on an interactive terminal, because the tested + * code reads php://input when the file is not a real http upload. + */ + +global $conf,$user,$langs,$db; +require_once dirname(__FILE__).'/../../htdocs/master.inc.php'; +require_once dirname(__FILE__).'/../../htdocs/core/class/fileupload.class.php'; +require_once dirname(__FILE__).'/../../htdocs/core/lib/files.lib.php'; +require_once dirname(__FILE__).'/../../htdocs/product/class/product.class.php'; +require_once dirname(__FILE__).'/../../htdocs/societe/class/societe.class.php'; +require_once dirname(__FILE__).'/../../htdocs/contact/class/contact.class.php'; +require_once dirname(__FILE__).'/../../htdocs/projet/class/project.class.php'; +require_once dirname(__FILE__).'/../../htdocs/projet/class/task.class.php'; +require_once dirname(__FILE__).'/CommonClassTest.class.php'; + +if (!defined('NOREQUIREUSER')) { + define('NOREQUIREUSER', '1'); +} +if (!defined('NOREQUIREDB')) { + define('NOREQUIREDB', '1'); +} +if (!defined('NOREQUIRESOC')) { + define('NOREQUIRESOC', '1'); +} +if (!defined('NOREQUIRETRAN')) { + define('NOREQUIRETRAN', '1'); +} +if (!defined('NOCSRFCHECK')) { + define('NOCSRFCHECK', '1'); +} +if (!defined('NOTOKENRENEWAL')) { + define('NOTOKENRENEWAL', '1'); +} +if (!defined('NOREQUIREMENU')) { + define('NOREQUIREMENU', '1'); +} +if (!defined('NOREQUIREHTML')) { + define('NOREQUIREHTML', '1'); +} +if (!defined('NOREQUIREAJAX')) { + define('NOREQUIREAJAX', '1'); +} +if (!defined("NOLOGIN")) { + define("NOLOGIN", '1'); +} + +/** + * Class for PHPUnit tests of FileUpload + * + * @backupGlobals disabled + * @backupStaticAttributes enabled + * @remarks backupGlobals must be disabled to have db,conf,user and lang not erased. + */ +class FileUploadTest extends CommonClassTest +{ + /** + * Directories created by the tests, removed at the end + * @var string[] + */ + protected static $dirstoclean = array(); + + /** + * Temporary files created by the tests, removed at the end + * @var string[] + */ + protected static $filestoclean = array(); + + /** + * Objects of the fixture, removed by the rollback of the transaction + * @var array + */ + protected static $objects = array(); + + + /** + * setUpBeforeClass + * + * @return void + */ + public static function setUpBeforeClass(): void + { + global $conf, $db, $user; + + parent::setUpBeforeClass(); + + $conf->global->MAIN_DISABLE_SUGGEST_REF_AS_PREFIX = 0; + + // A thirdparty + $thirdparty = new Societe($db); + $thirdparty->initAsSpecimen(); + $thirdparty->name = 'Test FileUpload'; + $thirdparty->country_id = 1; + $id = $thirdparty->create($user); + if ($id <= 0) { + die("Failed to create the thirdparty: ".$thirdparty->errorsToString()."\n"); + } + $thirdparty->fetch($id); + self::$objects['societe'] = $thirdparty; + + // A contact of this thirdparty + $contact = new Contact($db); + $contact->lastname = 'Doe'; + $contact->firstname = 'John'; + $contact->socid = $thirdparty->id; + $contact->country_id = 1; + $id = $contact->create($user); + if ($id <= 0) { + die("Failed to create the contact: ".$contact->errorsToString()."\n"); + } + $contact->fetch($id); + self::$objects['contact'] = $contact; + + // A product + $product = new Product($db); + $product->ref = 'PRODUCT-FILEUPLOAD-TEST'; + $product->label = 'Test FileUpload'; + $product->type = Product::TYPE_PRODUCT; + $id = $product->create($user); + if ($id <= 0) { + die("Failed to create the product: ".$product->errorsToString()."\n"); + } + $product->fetch($id); + self::$objects['product'] = $product; + + // A project whose ref holds the chars a path must not keep, and a task of this project + $project = new Project($db); + $project->ref = 'PJ/UP:é 2026'; + $project->title = 'Test FileUpload'; + $project->socid = $thirdparty->id; + $id = $project->create($user); + if ($id <= 0) { + die("Failed to create the project: ".$project->errorsToString()."\n"); + } + $project->fetch($id); + self::$objects['project'] = $project; + + $task = new Task($db); + $task->ref = 'TASK-FILEUPLOAD-TEST'; + $task->label = 'Test FileUpload'; + $task->fk_project = $project->id; + $id = $task->create($user); + if ($id <= 0) { + die("Failed to create the task: ".$task->errorsToString()."\n"); + } + $task->fetch($id); + self::$objects['project_task'] = $task; + } + + /** + * tearDownAfterClass + * + * @return void + */ + public static function tearDownAfterClass(): void + { + foreach (self::$dirstoclean as $dir) { + if (is_dir($dir)) { + dol_delete_dir_recursive($dir); + } + } + foreach (self::$filestoclean as $file) { + @unlink($file); + } + + // The objects created by setUpBeforeClass() are removed by the rollback of the parent, which + // closes the transaction opened by its own setUpBeforeClass(). + parent::tearDownAfterClass(); + } + + /** + * Build a FileUpload for an object and return its upload directory. + * + * @param string $element Element code + * @param int $id Id of the object + * @return string Upload directory + */ + protected function uploadDirOf($element, $id) + { + $upload = new FileUpload(null, $id, $element); + $this->assertIsArray($upload->options); + $this->assertArrayHasKey('upload_dir', $upload->options); + return $upload->options['upload_dir']; + } + + + // + // Resolution of the directory where the file is stored + // + + /** + * The directory of a product must be the one read by product/document.php, otherwise the file is + * stored but never shown into the "Attached files" tab. + * + * @return void + */ + public function testUploadDirOfAProduct() + { + global $conf; + + $object = self::$objects['product']; + + // The formula of product/document.php, written here on purpose so the test does not use the + // code it checks to build its expectation. + $expected = $conf->product->multidir_output[$object->entity].'/'.dol_sanitizeFileName($object->ref).'/'; + + $this->assertSame($expected, $this->uploadDirOf('product', $object->id)); + } + + /** + * A thirdparty stores its documents into a directory named after its id, not its ref. + * + * @return void + */ + public function testUploadDirOfAThirdpartyUsesTheId() + { + global $conf; + + $object = self::$objects['societe']; + + // The formula of societe/document.php + $expected = $conf->societe->multidir_output[$object->entity].'/'.$object->id.'/'; + + $this->assertSame($expected, $this->uploadDirOf('societe', $object->id)); + $this->assertStringNotContainsString(dol_sanitizeFileName($object->name), $this->uploadDirOf('societe', $object->id), 'The name of a thirdparty must never appear into its directory'); + } + + /** + * A contact is stored into a sub directory of the thirdparty module. getMultidirOutput() does not know + * this element and answers its sentinel, so this asserts the fallback on getElementProperties(). + * + * @return void + */ + public function testUploadDirOfAContactFallsBackOnGetElementProperties() + { + global $conf; + + $object = self::$objects['contact']; + + $this->assertStringStartsWith('error-diroutput-not-defined-for-this-object=', (string) getMultidirOutput($object, 'contact'), 'The fixture requires getMultidirOutput to fail on a contact'); + + // The formula of contact/document.php + $expected = $conf->societe->multidir_output[$object->entity].'/contact/'.dol_sanitizeFileName($object->ref).'/'; + + $this->assertSame($expected, $this->uploadDirOf('contact', $object->id)); + } + + /** + * A task is stored into a sub directory named after the ref of its project. That ref is a user input, + * so it must be sanitized the same way projet/tasks/document.php does it. + * + * @return void + */ + public function testUploadDirOfATaskSanitizesTheProjectRef() + { + global $conf; + + $task = self::$objects['project_task']; + $project = self::$objects['project']; + + $this->assertStringContainsString('/', $project->ref, 'The fixture requires a project ref holding a slash'); + + // The formula of projet/tasks/document.php + $expected = $conf->project->multidir_output[$project->entity].'/'.dol_sanitizeFileName($project->ref).'/'.dol_sanitizeFileName($task->ref).'/'; + + $dir = $this->uploadDirOf('project_task', $task->id); + + $this->assertSame($expected, $dir); + $this->assertStringNotContainsString('PJ/UP', $dir, 'The slash of the project ref must not create a sub directory'); + $this->assertStringNotContainsString(':', $dir, 'A colon of the project ref must not be kept'); + } + + /** + * Whatever the element, the directory is absolute, inside DOL_DATA_ROOT, and never holds the sentinel + * string returned by getMultidirOutput() when it fails. + * + * @return void + */ + public function testUploadDirIsNeverTheSentinelOfGetMultidirOutput() + { + foreach (array('product', 'societe', 'contact', 'project_task') as $element) { + $dir = $this->uploadDirOf($element, self::$objects[$element]->id); + + $this->assertStringNotContainsString('error-diroutput-not-defined-for-this-object', $dir, 'The sentinel leaked into the directory of the element '.$element); + $this->assertStringStartsWith(DOL_DATA_ROOT, $dir, 'The directory of the element '.$element.' must be inside DOL_DATA_ROOT'); + $this->assertStringNotContainsString('//', $dir, 'The directory of the element '.$element.' must not hold a double slash'); + $this->assertStringEndsWith('/', $dir, 'The directory must end with a slash, the file name is concatenated to it'); + } + } + + /** + * The file name is prefixed by the ref of the object, and that ref is sanitized because it is a user + * input that may hold a slash. + * + * @return void + */ + public function testSavingDocMaskIsSanitized() + { + $project = self::$objects['project']; + + $upload = new FileUpload(null, $project->id, 'project'); + + $this->assertSame(dol_sanitizeFileName($project->ref).'-__file__', $upload->options['saving_doc_mask']); + $this->assertStringNotContainsString('/', $upload->options['saving_doc_mask'], 'A file name mask must never hold a slash'); + } + + + // + // Refusals + // + + /** + * An object that does not exist must be refused, otherwise the file is stored at the root of the + * directory of the module, out of any object and out of any permission check. + * + * @return void + */ + public function testConstructRefusesAnObjectThatDoesNotExist() + { + $this->expectException(Exception::class); + $this->expectExceptionMessage('objectnotfound'); + + new FileUpload(null, 99999999, 'product'); + } + + /** + * An id of 0 must be refused too, it would answer the directory of every unsaved object. + * + * @return void + */ + public function testConstructRefusesAnEmptyId() + { + $this->expectException(Exception::class); + $this->expectExceptionMessage('objectnotfound'); + + new FileUpload(null, 0, 'product'); + } + + /** + * An element with no directory at all must be refused instead of writing anywhere. + * + * @return void + */ + public function testConstructRefusesAnUnknownElement() + { + $this->expectException(Exception::class); + + new FileUpload(null, 1, 'anelementthatdoesnotexist'); + } + + + // + // Deduplication of the name of the uploaded file + // + + + /** + * Build a FileUpload for an object of the fixture and empty its upload directory, so the tests below + * start from a known set of already used names. + * + * @param string $element Element code of the fixture + * @return FileUpload Instance ready to use + */ + protected function prepareUploadDir($element) + { + $object = self::$objects[$element]; + $upload = new FileUpload(null, $object->id, $element); + $dir = $upload->options['upload_dir']; + + if (is_dir($dir)) { + dol_delete_dir_recursive($dir); + } + dol_mkdir($dir); + self::$dirstoclean[$dir] = $dir; + + return $upload; + } + + /** + * Create the temporary file playing the role of the file uploaded by the browser. + * + * @return string Path of the file + */ + protected function makeTmpFile() + { + $tmpfile = DOL_DATA_ROOT.'/admin/temp/fileuploadtest-'.getmypid().'.txt'; + dol_mkdir(dirname($tmpfile)); + file_put_contents($tmpfile, 'content'); + self::$filestoclean[$tmpfile] = $tmpfile; + + return $tmpfile; + } + + /** + * Call the protected handleFileUpload() of FileUpload. + * + * @param FileUpload $upload Instance + * @param string $tmpfile Path of the file to upload + * @param string $name Name sent by the browser + * @return stdClass The file object answered by handleFileUpload() + */ + protected function callHandleFileUpload($upload, $tmpfile, $name) + { + $method = new ReflectionMethod('FileUpload', 'handleFileUpload'); + $method->setAccessible(true); + + // validate() reads CONTENT_LENGTH when the file is not a real http upload + $_SERVER['CONTENT_LENGTH'] = 10; + + return $method->invoke($upload, $tmpfile, $name, 0, 'text/plain', 0, 0); + } + + /** + * A name that is free must be kept as it is, only prefixed by the ref of the object. + * + * @return void + */ + public function testUploadOfAFreeNameIsKept() + { + $upload = $this->prepareUploadDir('product'); + $prefix = dol_sanitizeFileName(self::$objects['product']->ref).'-'; + + $file = $this->callHandleFileUpload($upload, $this->makeTmpFile(), 'afreename.txt'); + + $this->assertSame($prefix.'afreename.txt', $file->name, 'A free name must be kept, only the prefix of the ref is added'); + } + + /** + * Uploading twice the same file must not overwrite the first one: dol_move_uploaded_file() is called + * with $allowoverwrite = 1, so the name must be made unique before. + * trimFileName() already does that check, but on the name before the prefix of the ref is added, so + * it compares a name that is not the one stored. + * + * @return void + */ + public function testUploadOfAnAlreadyExistingNameIsRenamed() + { + $upload = $this->prepareUploadDir('product'); + $dir = $upload->options['upload_dir']; + $prefix = dol_sanitizeFileName(self::$objects['product']->ref).'-'; + + // The name is already used, with the prefix that trimFileName() does not know about + file_put_contents($dir.$prefix.'mydoc.txt', 'first'); + + $file = $this->callHandleFileUpload($upload, $this->makeTmpFile(), 'mydoc.txt'); + + $this->assertSame($prefix.'mydoc (1).txt', $file->name, 'The name must be made unique, the first file must not be overwritten'); + $this->assertSame('first', file_get_contents($dir.$prefix.'mydoc.txt'), 'The first file must be untouched'); + $this->assertFileExists($dir.$file->name, 'The new file must be stored under its new name'); + } + + /** + * The renaming must be repeated as long as the name is used, so a third upload gets a third name. + * + * @return void + */ + public function testUploadOfAnAlreadyExistingNameIsRenamedAgain() + { + $upload = $this->prepareUploadDir('product'); + $dir = $upload->options['upload_dir']; + $prefix = dol_sanitizeFileName(self::$objects['product']->ref).'-'; + + file_put_contents($dir.$prefix.'mydoc.txt', 'first'); + file_put_contents($dir.$prefix.'mydoc (1).txt', 'second'); + + $file = $this->callHandleFileUpload($upload, $this->makeTmpFile(), 'mydoc.txt'); + + $this->assertSame($prefix.'mydoc (2).txt', $file->name); + $this->assertSame('first', file_get_contents($dir.$prefix.'mydoc.txt')); + $this->assertSame('second', file_get_contents($dir.$prefix.'mydoc (1).txt')); + } + + /** + * A file whose name is executable is stored by dol_move_uploaded_file() with a '.noexe' suffix added. + * The check on an already used name must look for that suffixed name too, otherwise such a file is + * silently overwritten at each upload. + * + * @return void + */ + public function testUploadOfAnAlreadyExistingNoexeNameIsRenamed() + { + $upload = $this->prepareUploadDir('product'); + $dir = $upload->options['upload_dir']; + $prefix = dol_sanitizeFileName(self::$objects['product']->ref).'-'; + + // A previous upload of the same file was renamed with the .noexe suffix + file_put_contents($dir.$prefix.'myscript.php.noexe', 'first'); + + $file = $this->callHandleFileUpload($upload, $this->makeTmpFile(), 'myscript.php'); + + $this->assertStringStartsWith($prefix.'myscript (1).php', $file->name, 'A name already used with the .noexe suffix must be made unique too'); + $this->assertSame('first', file_get_contents($dir.$prefix.'myscript.php.noexe'), 'The first file must be untouched'); + } +} diff --git a/test/phpunit/FunctionsLibDragDropTest.php b/test/phpunit/FunctionsLibDragDropTest.php new file mode 100644 index 00000000000..33dae26d9f2 --- /dev/null +++ b/test/phpunit/FunctionsLibDragDropTest.php @@ -0,0 +1,482 @@ +. + * or see https://www.gnu.org/ + */ + +/** + * \file test/phpunit/FunctionsLibDragDropTest.php + * \ingroup test + * \brief PHPUnit test of the directory resolution used by the drag and drop of a file on a card. + * \remarks To run this script as CLI: phpunit FunctionsLibDragDropTest.php + */ + +global $conf,$user,$langs,$db; +require_once dirname(__FILE__).'/../../htdocs/master.inc.php'; +require_once dirname(__FILE__).'/../../htdocs/product/class/product.class.php'; +require_once dirname(__FILE__).'/../../htdocs/societe/class/societe.class.php'; +require_once dirname(__FILE__).'/../../htdocs/contact/class/contact.class.php'; +require_once dirname(__FILE__).'/CommonClassTest.class.php'; + +if (!defined('NOREQUIREUSER')) { + define('NOREQUIREUSER', '1'); +} +if (!defined('NOREQUIREDB')) { + define('NOREQUIREDB', '1'); +} +if (!defined('NOREQUIRESOC')) { + define('NOREQUIRESOC', '1'); +} +if (!defined('NOREQUIRETRAN')) { + define('NOREQUIRETRAN', '1'); +} +if (!defined('NOCSRFCHECK')) { + define('NOCSRFCHECK', '1'); +} +if (!defined('NOTOKENRENEWAL')) { + define('NOTOKENRENEWAL', '1'); +} +if (!defined('NOREQUIREMENU')) { + define('NOREQUIREMENU', '1'); +} +if (!defined('NOREQUIREHTML')) { + define('NOREQUIREHTML', '1'); +} +if (!defined('NOREQUIREAJAX')) { + define('NOREQUIREAJAX', '1'); +} +if (!defined("NOLOGIN")) { + define("NOLOGIN", '1'); +} + +/** + * Class for PHPUnit tests of the directory resolution used by the drag and drop of a file on a card + * + * @backupGlobals disabled + * @backupStaticAttributes enabled + * @remarks backupGlobals must be disabled to have db,conf,user and lang not erased. + */ +class FunctionsLibDragDropTest extends CommonClassTest +{ + /** + * The string returned by getMultidirOutput() when it does not know the module of the object. + * It is a relative path, so writing into it creates files under the web root. + */ + const SENTINEL = 'error-diroutput-not-defined-for-this-object='; + + /** + * The elements equipped with the drag and drop of a file on their card by this work. + * Their 'dir_output' must always be usable to forge an absolute path. + * + * @return array> + */ + public static function providerEquippedElements() + { + $elements = array( + 'contact', 'product', 'societe', 'action', 'expedition', 'reception', 'don', 'expensereport', + 'holiday', 'mo', 'partnership', 'stocktransfer', 'productlot', 'resource', 'workstation', + 'job', 'position', 'skill', 'evaluation', 'knowledgerecord', 'conferenceorbooth', 'asset', + 'payment', 'payment_supplier', 'payment_various', 'salary', 'chargesociales', 'project_task', + ); + $out = array(); + foreach ($elements as $element) { + $out[$element] = array($element); + } + return $out; + } + + + // + // getMultidirOutput() + // + + /** + * getMultidirOutput() answers a sentinel string, not an empty string, when it does not know the module. + * Every caller must reject it, so the contract is asserted here once. + * + * @return void + */ + public function testGetMultidirOutputReturnsARelativeSentinelOnFailure() + { + $object = new stdClass(); + $object->element = 'anelementthatdoesnotexist'; + $object->id = 1; + $object->entity = 1; + + $dir = getMultidirOutput($object, 'anelementthatdoesnotexist'); + + $this->assertSame(self::SENTINEL.'anelementthatdoesnotexist', $dir, 'The failure of getMultidirOutput must be reported by the sentinel string'); + $this->assertStringStartsNotWith('/', $dir, 'The sentinel is a relative path, so it must never be used to forge a path'); + + // The 'temp' mode has its own sentinel + $this->assertSame('error-dirtemp-not-defined-for-this-object=anelementthatdoesnotexist', getMultidirTemp($object, 'anelementthatdoesnotexist')); + + // And a bad mode has a third one + $this->assertSame('error-bad-value-for-mode', getMultidirOutput($object, 'anelementthatdoesnotexist', 0, 'notamode')); + } + + /** + * The sentinel is the answer for a majority of the elements equipped with the drag and drop, so any + * caller that forges a path from getMultidirOutput() alone is broken. This test documents the list. + * + * @return void + */ + public function testGetMultidirOutputIsUnableToResolveMostEquippedElements() + { + $nbresolved = 0; + $nbsentinel = 0; + foreach (self::providerEquippedElements() as $row) { + $element = $row[0]; + if ($element == 'project_task') { + continue; // Needs a real object, it calls fetchProject() + } + $object = new stdClass(); + $object->element = $element; + $object->id = 1; + $object->entity = 1; + $dir = getMultidirOutput($object, $element); + if (strpos((string) $dir, self::SENTINEL) === 0) { + $nbsentinel++; + } else { + $nbresolved++; + $this->assertStringStartsWith(DOL_DATA_ROOT, (string) $dir, 'A resolved directory must be inside DOL_DATA_ROOT for the element '.$element); + } + } + $this->assertGreaterThan(0, $nbsentinel, 'getMultidirOutput is expected to fail on some equipped elements, the fallback on getElementProperties is required'); + $this->assertGreaterThan(0, $nbresolved, 'getMultidirOutput is expected to resolve some equipped elements'); + } + + /** + * A module with a directory for the current entity only must not answer an undefined index (so a path + * relative to the web root) when the object belongs to another entity. + * + * @return void + */ + public function testGetMultidirOutputFallsBackOnTheCurrentEntity() + { + global $conf; + + $object = new Product($conf->db); + $object->id = 1; + $object->ref = 'AREF'; + $object->entity = 99; // No directory is declared for this entity + + $this->assertArrayNotHasKey(99, $conf->product->multidir_output, 'The fixture requires no directory declared for the entity 99'); + + $dir = getMultidirOutput($object, 'product'); + + $this->assertSame($conf->product->multidir_output[$conf->entity], $dir, 'The directory of the current entity must be used as a fallback'); + $this->assertStringStartsWith(DOL_DATA_ROOT, $dir, 'The fallback must not answer a path relative to the web root'); + + // Same fallback for the temporary directory + $dirtemp = getMultidirTemp($object, 'product'); + $this->assertStringStartsWith(DOL_DATA_ROOT, $dirtemp, 'The temporary directory must not be relative either'); + } + + /** + * The subdirectory of a partnership and of a stock transfer must be appended, because it is the one + * read by their "Attached files" tab (see partnership_document.php and stocktransfer_document.php). + * + * @return void + */ + public function testGetMultidirOutputAppendsTheSubDirectoryOfTheTab() + { + global $conf; + + $expected = array( + 'partnership' => '/partnership/partnership', + 'stocktransfer' => '/stocktransfer/stocktransfer', + 'knowledgerecord' => '/knowledgemanagement/knowledgerecord', + 'expedition' => '/expedition/sending', + ); + $asserted = 0; + foreach ($expected as $element => $suffix) { + if (!isset($conf->{explode('/', ltrim($suffix, '/'))[0]})) { + continue; + } + $object = new stdClass(); + $object->element = $element; + $object->id = 1; + $object->entity = $conf->entity; + $dir = getMultidirOutput($object, $element); + if (strpos((string) $dir, self::SENTINEL) === 0) { + continue; // Module not enabled on this instance + } + $this->assertSame(DOL_DATA_ROOT.$suffix, $dir, 'Wrong directory for the element '.$element); + $asserted++; + } + + if (!$asserted) { + // None of the modules of the elements above is enabled, so the test asserted nothing. Say it + // instead of reporting a green test that checked nothing. + $this->markTestSkipped('None of the modules partnership, stocktransfer, knowledgemanagement and expedition is enabled'); + } + } + + + // + // get_exdir() + // + + /** + * get_exdir() is the reference implementation used both by the "Attached files" tabs and by FileUpload + * to forge the directory of an object. Its fallbacks must be asserted, they are load bearing. + * + * @return void + */ + public function testGetExdirFallbacks() + { + global $db; + + $object = new Product($db); + $object->id = 42; + $object->ref = 'MYREF'; + + // Nominal case: the ref is used + $this->assertSame('MYREF', get_exdir(0, 0, 0, 1, $object, 'product')); + + // The trailing slash is added when $withoutslash is 0. FileUpload appends its own '/', so it must + // call get_exdir() with $withoutslash = 1 to avoid a double slash into the path. + $this->assertSame('MYREF/', get_exdir(0, 0, 0, 0, $object, 'product')); + + // The id is used as a fallback when the ref is empty (a draft object with no numbering yet) + $object->ref = ''; + $this->assertSame('42', get_exdir(0, 0, 0, 1, $object, 'product'), 'The id must be used when the ref is empty'); + $object->ref = null; + $this->assertSame('42', get_exdir(0, 0, 0, 1, $object, 'product'), 'The id must be used when the ref is null'); + $object->ref = '0'; + $this->assertSame('42', get_exdir(0, 0, 0, 1, $object, 'product'), 'A ref "0" is empty for php, the id is used'); + + // An object with neither a ref nor an id gives the directory '0', because the id is cast to an int + // and then to a string. This is a shared directory for every unsaved object, so a caller must never + // forge a path from an object it did not load: FileUpload throws 'objectnotfound' before this point. + $empty = new Product($db); + $empty->id = 0; + $empty->ref = ''; + $this->assertSame('0', get_exdir(0, 0, 0, 1, $empty, 'product'), 'An object with no id and no ref falls back on the directory "0"'); + + // The ref is a user input, it must be sanitized: no directory traversal, no separator + $object->ref = '../../etc'; + $this->assertStringNotContainsString('..', get_exdir(0, 0, 0, 1, $object, 'product'), 'A ref must never allow a directory traversal'); + $object->ref = 'A/B'; + $this->assertStringNotContainsString('/', get_exdir(0, 0, 0, 1, $object, 'product'), 'A ref must never introduce a sub directory'); + + // The modulepart is read from the object when it is not given + $object->ref = 'MYREF'; + $this->assertSame('MYREF', get_exdir(0, 0, 0, 1, $object), 'The modulepart must be deduced from the object'); + } + + /** + * A thirdparty stores its documents into a directory named after its id, because its ref is a company + * name and two thirdparties may share the same name. + * + * @return void + */ + public function testGetExdirUsesTheIdForAThirdparty() + { + global $db; + + $thirdparty = new Societe($db); + $thirdparty->id = 7; + $thirdparty->ref = 'My company'; + + $this->assertSame('7', get_exdir(0, 0, 0, 1, $thirdparty, 'societe'), 'The id must be used for a thirdparty'); + $this->assertSame('7', get_exdir(0, 0, 0, 1, $thirdparty, 'thirdparty'), 'The id must be used for a thirdparty'); + + // The rule is on the class, not only on the modulepart: a contact of the module 'societe' keeps its ref + $contact = new Contact($db); + $contact->id = 8; + $contact->ref = 'DOE'; + $this->assertSame('DOE', get_exdir(0, 0, 0, 1, $contact, 'contact'), 'A contact is not a thirdparty, its ref is used'); + } + + /** + * A module storing its documents on 2 levels answers the level directories only, so the caller must + * append the directory of the object itself. + * + * @return void + */ + public function testGetExdirForATwoLevelsModule() + { + global $db; + + $object = new Product($db); // Any object, only the id and the modulepart matter here + $object->id = 42; + $object->ref = 'MYREF'; + + $this->assertSame('2/4', get_exdir(0, 0, 0, 1, $object, 'invoice_supplier'), 'Two levels of directories are expected'); + $this->assertSame('2/4', get_exdir(0, 0, 0, 1, $object, 'supplier_invoice'), 'The two aliases must answer the same directory'); + $this->assertSame('2/4/', get_exdir(0, 0, 0, 0, $object, 'invoice_supplier')); + + // The levels are built from the id, not from the ref + $object->id = 1234; + $this->assertSame('4/3', get_exdir(0, 0, 0, 1, $object, 'invoice_supplier')); + } + + + // + // getElementProperties() + // + + /** + * The elements added or fixed by this work must answer the class that is really able to load them. + * + * @return void + */ + public function testGetElementPropertiesOfTheNewElements() + { + $expected = array( + // element => array(module, classname, classpath, classfile, table_element) + 'payment' => array('facture', 'Paiement', 'compta/paiement/class', 'paiement', 'paiement'), + 'payment_supplier' => array('fournisseur', 'PaiementFourn', 'fourn/class', 'paiementfourn', 'paiementfourn'), + 'payment_various' => array('bank', 'PaymentVarious', 'compta/bank/class', 'paymentvarious', 'payment_various'), + 'stocktransfer' => array('stocktransfer', 'StockTransfer', 'product/stock/stocktransfer/class', 'stocktransfer', 'stocktransfer_stocktransfer'), + 'job' => array('hrm', 'Job', 'hrm/class', 'job', 'hrm_job'), + 'position' => array('hrm', 'Position', 'hrm/class', 'position', 'hrm_job_user'), + 'skill' => array('hrm', 'Skill', 'hrm/class', 'skill', 'hrm_skill'), + 'evaluation' => array('hrm', 'Evaluation', 'hrm/class', 'evaluation', 'hrm_evaluation'), + ); + + foreach ($expected as $element => $values) { + $prop = getElementProperties($element); + + $this->assertSame($element, $prop['element'], 'The element '.$element.' must not be rewritten by the myobject_mysubobject rule'); + $this->assertSame($values[0], $prop['module'], 'Wrong module for the element '.$element); + $this->assertSame($values[1], $prop['classname'], 'Wrong classname for the element '.$element); + $this->assertSame($values[2], $prop['classpath'], 'Wrong classpath for the element '.$element); + $this->assertSame($values[3], $prop['classfile'], 'Wrong classfile for the element '.$element); + $this->assertSame($values[4], $prop['table_element'], 'Wrong table for the element '.$element); + + // The class must really exist, otherwise fetchObjectByElement() ends on a fatal error + $file = DOL_DOCUMENT_ROOT.'/'.$prop['classpath'].'/'.$prop['classfile'].'.class.php'; + $this->assertFileExists($file, 'The class file of the element '.$element.' does not exist'); + require_once $file; + $this->assertTrue(class_exists($prop['classname']), 'The class '.$prop['classname'].' of the element '.$element.' does not exist'); + + // And the table must exist too, restrictedArea() builds its sql on it. + // The table of an optional module is only created when the module is enabled, so the check is + // skipped otherwise: the mapping asserted above does not depend on the module being enabled. + if (isModEnabled($values[0])) { + $this->assertGreaterThan(0, $this->countTable($prop['table_element']), 'The table '.$prop['table_element'].' of the element '.$element.' is not readable'); + } + } + } + + /** + * Count the rows of a table, only to assert the table exists. + * + * @param string $table Table name without the prefix + * @return int -1 if the table does not exist, 1 otherwise + */ + protected function countTable($table) + { + global $db; + + $sql = "SELECT COUNT(*) as nb FROM ".$db->prefix().$db->escape($table); + $resql = $db->query($sql); + if (!$resql) { + return -1; + } + $db->free($resql); + return 1; + } + + /** + * The 'dir_output' of an element must be the directory read by its "Attached files" tab, otherwise a + * file uploaded by drag and drop is stored but never shown to the user. + * + * @return void + */ + public function testGetElementPropertiesDirOutputSubDirectory() + { + global $conf; + + $expected = array( + 'contact' => array('societe', '/societe/contact', '/societe/temp/contact'), + 'job' => array('hrm', '/hrm/job', '/hrm/temp/job'), + 'position' => array('hrm', '/hrm/position', '/hrm/temp/position'), + 'skill' => array('hrm', '/hrm/skill', '/hrm/temp/skill'), + 'evaluation' => array('hrm', '/hrm/evaluation', '/hrm/temp/evaluation'), + 'conferenceorbooth' => array('eventorganization', '/eventorganization/conferenceorbooth', '/eventorganization/temp/conferenceorbooth'), + ); + foreach ($expected as $element => $values) { + if (!isset($conf->{$values[0]})) { + $this->markTestSkipped('The module '.$values[0].' is not enabled, the fixture is not usable'); + } + $prop = getElementProperties($element); + $this->assertSame(DOL_DATA_ROOT.$values[1], $prop['dir_output'], 'Wrong dir_output for the element '.$element); + $this->assertSame(DOL_DATA_ROOT.$values[2], $prop['dir_temp'], 'Wrong dir_temp for the element '.$element.', the sub directory applies to it too'); + } + } + + /** + * When the module of an element is disabled, its directory must be an empty string and NOT the sub + * directory alone: a caller would then read or write into '/contact', at the root of the file system. + * This is the regression the guard on $dir_output prevents, and it can only be seen with a module off. + * + * @return void + */ + public function testGetElementPropertiesDirOutputWhenTheModuleIsDisabled() + { + global $conf; + + $elements = array('contact' => 'societe', 'job' => 'hrm', 'position' => 'hrm', 'skill' => 'hrm', + 'evaluation' => 'hrm', 'conferenceorbooth' => 'eventorganization'); + + foreach ($elements as $element => $module) { + $savconfmodule = isset($conf->$module) ? $conf->$module : null; + $savmodules = $conf->modules; + + // Simulate the module being disabled: no entry into $conf and no entry into $conf->modules + unset($conf->$module); + unset($conf->modules[$module]); + + try { + $prop = getElementProperties($element); + + $this->assertSame('', $prop['dir_output'], 'The dir_output of the element '.$element.' must be empty when the module '.$module.' is disabled, not the sub directory alone'); + $this->assertSame('', $prop['dir_temp'], 'The dir_temp of the element '.$element.' must be empty when the module '.$module.' is disabled'); + } finally { + if ($savconfmodule !== null) { + $conf->$module = $savconfmodule; + } + $conf->modules = $savmodules; + } + } + } + + /** + * The directory of any equipped element is either empty or inside DOL_DATA_ROOT. It is never a path + * at the root of the file system, and never the sentinel of getMultidirOutput(). + * + * @dataProvider providerEquippedElements + * + * @param string $element Element to check + * @return void + */ + public function testGetElementPropertiesDirOutputIsAlwaysSafe($element) + { + $prop = getElementProperties($element); + + foreach (array('dir_output', 'dir_temp') as $key) { + $dir = (string) $prop[$key]; + $this->assertStringNotContainsString(self::SENTINEL, $dir, 'The '.$key.' of the element '.$element.' must never be the sentinel'); + $this->assertTrue( + $dir === '' || strpos($dir, DOL_DATA_ROOT) === 0, + 'The '.$key.' of the element '.$element.' must be empty or inside DOL_DATA_ROOT, got "'.$dir.'"' + ); + } + } +} diff --git a/test/phpunit/FunctionsLibTest.php b/test/phpunit/FunctionsLibTest.php index c586d238936..c76ce2e67f5 100644 --- a/test/phpunit/FunctionsLibTest.php +++ b/test/phpunit/FunctionsLibTest.php @@ -33,6 +33,8 @@ global $conf,$user,$langs,$db,$mysoc; require_once dirname(__FILE__).'/../../htdocs/master.inc.php'; require_once dirname(__FILE__).'/../../htdocs/core/lib/date.lib.php'; require_once dirname(__FILE__).'/../../htdocs/product/class/product.class.php'; +require_once dirname(__FILE__).'/../../htdocs/societe/class/societe.class.php'; +require_once dirname(__FILE__).'/../../htdocs/contact/class/contact.class.php'; require_once dirname(__FILE__).'/CommonClassTest.class.php'; if (! defined('NOREQUIREUSER')) { @@ -109,6 +111,212 @@ class FunctionsLibTest extends CommonClassTest } + /** + * testGetExdirForObject + * + * get_exdir() with $level = 0 and $withoutslash = 1 is the reference implementation used to forge the + * directory where the documents of an object are stored. FileUpload (the drag and drop of a file on a + * card) relies on it to store the file into the directory read by the "Attached files" tab. + * + * @return void + */ + public function testGetExdirForObject() + { + global $conf, $db; + + // PRODUCT_USE_OLD_PATH_FOR_PHOTO makes get_exdir() answer a two level path built on the id for a + // product, instead of the ref. The assertions below are on the default path, so the option is forced + // off here: it is a global, another test of the suite may have left it on, and the whole suite runs in + // a single process. The option gets its own coverage at the end of this test. + $savoldpath = getDolGlobalInt('PRODUCT_USE_OLD_PATH_FOR_PHOTO'); + $conf->global->PRODUCT_USE_OLD_PATH_FOR_PHOTO = 0; + + // The ref is used when it is defined + $object = new Product($db); + $object->id = 42; + $object->ref = 'MYREF'; + $this->assertSame('MYREF', get_exdir(0, 0, 0, 1, $object, 'product'), 'The ref must be used when it is defined'); + + // The trailing slash is added when $withoutslash is 0. FileUpload appends its own '/' to the + // result, so it must call get_exdir() with $withoutslash = 1 to avoid a double slash in the path. + $this->assertSame('MYREF/', get_exdir(0, 0, 0, 0, $object, 'product'), 'A trailing slash is expected when $withoutslash is 0'); + + // The modulepart is deduced from the object when it is not given + $this->assertSame('MYREF', get_exdir(0, 0, 0, 1, $object), 'The modulepart must be deduced from the object'); + + // The ref is a user input: it must never introduce a directory traversal nor a sub directory + $object->ref = '../../etc'; + $this->assertStringNotContainsString('..', get_exdir(0, 0, 0, 1, $object, 'product'), 'A ref must never allow a directory traversal'); + $object->ref = 'A/B'; + $this->assertStringNotContainsString('/', get_exdir(0, 0, 0, 1, $object, 'product'), 'A ref must never introduce a sub directory'); + + // The id is used as a fallback when the ref is empty + $object->ref = ''; + $this->assertSame('42', get_exdir(0, 0, 0, 1, $object, 'product'), 'The id must be used when the ref is empty'); + $object->ref = null; + $this->assertSame('42', get_exdir(0, 0, 0, 1, $object, 'product'), 'The id must be used when the ref is null'); + $object->ref = '0'; + $this->assertSame('42', get_exdir(0, 0, 0, 1, $object, 'product'), 'A ref "0" is empty for php, so the id is used'); + + // An object with neither a ref nor an id falls back on the directory '0', shared by every unsaved + // object. A caller must never forge a path from an object it did not load. + $empty = new Product($db); + $empty->id = 0; + $empty->ref = ''; + $this->assertSame('0', get_exdir(0, 0, 0, 1, $empty, 'product'), 'An object with no id and no ref falls back on the directory "0"'); + + // The id is always used for a thirdparty, because its ref is a company name, so it is not unique + $thirdparty = new Societe($db); + $thirdparty->id = 7; + $thirdparty->ref = 'My company'; + $this->assertSame('7', get_exdir(0, 0, 0, 1, $thirdparty, 'societe'), 'The id must be used for a thirdparty'); + $this->assertSame('7', get_exdir(0, 0, 0, 1, $thirdparty, 'thirdparty'), 'The id must be used for a thirdparty'); + + // The rule is on the class, not only on the modulepart: a contact belongs to the module 'societe' + // but it is not a Societe, so it keeps its ref + $contact = new Contact($db); + $contact->id = 8; + $contact->ref = 'DOE'; + $this->assertSame('DOE', get_exdir(0, 0, 0, 1, $contact, 'contact'), 'A contact is not a thirdparty, its ref is used'); + + // A module storing its documents on several levels returns the level directories only, not the object + $object->ref = 'MYREF'; + $this->assertSame('2/4', get_exdir(0, 0, 0, 1, $object, 'invoice_supplier'), 'Two levels of directories are expected'); + $this->assertSame('2/4', get_exdir(0, 0, 0, 1, $object, 'supplier_invoice'), 'The two aliases must answer the same directory'); + $this->assertSame('2/4/', get_exdir(0, 0, 0, 0, $object, 'invoice_supplier'), 'A trailing slash is expected when $withoutslash is 0'); + + // The levels are built from the id, not from the ref + $object->id = 1234; + $this->assertSame('4/3', get_exdir(0, 0, 0, 1, $object, 'invoice_supplier'), 'The levels must be built from the id'); + + // With PRODUCT_USE_OLD_PATH_FOR_PHOTO a product joins the modules storing on two levels, so its + // directory is built on the id and the ref is ignored. This is the path the "Attached files" tab of a + // product reads on such an instance, so FileUpload must forge the same one. + $conf->global->PRODUCT_USE_OLD_PATH_FOR_PHOTO = 1; + $object->id = 42; + $object->ref = 'MYREF'; + $this->assertSame('2/4', get_exdir(0, 0, 0, 1, $object, 'product'), 'With the old path option a product uses two levels built on its id'); + + $conf->global->PRODUCT_USE_OLD_PATH_FOR_PHOTO = $savoldpath; + } + + /** + * testGetElementPropertiesDirOutput + * + * The 'dir_output' returned for an element must be the directory read by the "Attached files" tab of + * this element, otherwise a file uploaded by drag and drop is stored but never shown to the user. + * + * @return void + */ + public function testGetElementPropertiesDirOutput() + { + global $conf, $db; + + // A contact is stored into a sub directory of the thirdparty module, see contact/document.php. + // The exact value is asserted and not only the suffix: an assertion on the suffix alone would also + // pass on the value '/contact' returned when the module is disabled, which is the bug guarded here. + if (!isModEnabled('societe')) { + $this->markTestSkipped('The module societe must be enabled to check the directory of a contact'); + } + $prop = getElementProperties('contact'); + $this->assertSame('societe', $prop['module']); + $this->assertSame($conf->societe->multidir_output[$conf->entity].'/contact', $prop['dir_output'], 'A contact is stored into a /contact sub directory'); + $this->assertSame($conf->societe->multidir_temp[$conf->entity].'/contact', $prop['dir_temp'], 'The sub directory applies to the temporary directory too'); + + // The elements added or fixed here must answer the class that is really able to load them, and the + // table restrictedArea() builds its sql on. + $expected = array( + // element => array(module, classname, classpath, classfile, table_element) + 'payment' => array('facture', 'Paiement', 'compta/paiement/class', 'paiement', 'paiement'), + 'payment_supplier' => array('fournisseur', 'PaiementFourn', 'fourn/class', 'paiementfourn', 'paiementfourn'), + 'payment_various' => array('bank', 'PaymentVarious', 'compta/bank/class', 'paymentvarious', 'payment_various'), + 'stocktransfer' => array('stocktransfer', 'StockTransfer', 'product/stock/stocktransfer/class', 'stocktransfer', 'stocktransfer_stocktransfer'), + 'job' => array('hrm', 'Job', 'hrm/class', 'job', 'hrm_job'), + 'position' => array('hrm', 'Position', 'hrm/class', 'position', 'hrm_job_user'), + 'skill' => array('hrm', 'Skill', 'hrm/class', 'skill', 'hrm_skill'), + 'evaluation' => array('hrm', 'Evaluation', 'hrm/class', 'evaluation', 'hrm_evaluation'), + ); + foreach ($expected as $element => $values) { + $prop = getElementProperties($element); + + $this->assertSame($element, $prop['element'], 'The element '.$element.' must not be truncated by the myobject_mysubobject rule'); + $this->assertSame($values[0], $prop['module'], 'Wrong module for the element '.$element); + $this->assertSame($values[1], $prop['classname'], 'Wrong classname for the element '.$element); + $this->assertSame($values[2], $prop['classpath'], 'Wrong classpath for the element '.$element); + $this->assertSame($values[3], $prop['classfile'], 'Wrong classfile for the element '.$element); + $this->assertSame($values[4], $prop['table_element'], 'Wrong table for the element '.$element); + + // The class must really exist, otherwise fetchObjectByElement() ends on a fatal error + $file = DOL_DOCUMENT_ROOT.'/'.$prop['classpath'].'/'.$prop['classfile'].'.class.php'; + $this->assertFileExists($file, 'The class file of the element '.$element.' does not exist'); + require_once $file; + $this->assertTrue(class_exists($prop['classname']), 'The class '.$prop['classname'].' of the element '.$element.' does not exist'); + + // And the table must exist too, restrictedArea() builds its sql on it. + // The table of an optional module is only created when the module is enabled, so the check is + // skipped otherwise: the mapping asserted above does not depend on the module being enabled. + if (isModEnabled($values[0])) { + $sql = "SELECT COUNT(*) as nb FROM ".$db->prefix().$db->escape($values[4]); + $resql = $db->query($sql); + $this->assertNotFalse($resql, 'The table '.$values[4].' of the element '.$element.' is not readable'); + if ($resql) { + $db->free($resql); + } + } + } + + // The elements of the hrm module are all stored into a sub directory named after the element + if (isModEnabled('hrm')) { + foreach (array('job', 'position', 'skill', 'evaluation') as $element) { + $prop = getElementProperties($element); + $this->assertSame($conf->hrm->dir_output.'/'.$element, $prop['dir_output'], 'The element '.$element.' is stored into a /'.$element.' sub directory'); + $this->assertSame($conf->hrm->dir_temp.'/'.$element, $prop['dir_temp'], 'The sub directory applies to the temporary directory of '.$element.' too'); + } + } + + // The sub directory must not be appended when the module is disabled, otherwise we would return a + // path at the root of the file system (for example '/contact') instead of an empty string. This is + // only observable with the module really off, so it is simulated here. + $elements = array('contact' => 'societe', 'job' => 'hrm', 'position' => 'hrm', 'skill' => 'hrm', + 'evaluation' => 'hrm', 'conferenceorbooth' => 'eventorganization'); + foreach ($elements as $element => $module) { + $savconfmodule = isset($conf->$module) ? $conf->$module : null; + $savmodules = $conf->modules; + + unset($conf->$module); + unset($conf->modules[$module]); + + try { + $prop = getElementProperties($element); + $this->assertSame('', $prop['dir_output'], 'The dir_output of the element '.$element.' must be empty when the module '.$module.' is disabled, not the sub directory alone'); + $this->assertSame('', $prop['dir_temp'], 'The dir_temp of the element '.$element.' must be empty when the module '.$module.' is disabled'); + } finally { + if ($savconfmodule !== null) { + $conf->$module = $savconfmodule; + } + $conf->modules = $savmodules; + } + } + + // So the directory of an element is either empty, or a directory of the data directory of Dolibarr, + // and never the sentinel string returned by getMultidirOutput() when it fails. + $allelements = array('contact', 'job', 'position', 'skill', 'evaluation', 'conferenceorbooth', + 'partnership', 'stocktransfer', 'payment', 'payment_supplier', 'payment_various', 'product', + 'societe', 'action', 'expedition', 'reception', 'don', 'expensereport', 'holiday', 'mo', + 'productlot', 'resource', 'workstation', 'knowledgerecord', 'asset', 'salary', 'chargesociales'); + foreach ($allelements as $element) { + $prop = getElementProperties($element); + foreach (array('dir_output', 'dir_temp') as $key) { + $dir = (string) $prop[$key]; + $this->assertStringNotContainsString('error-diroutput-not-defined-for-this-object', $dir, 'The '.$key.' of the element '.$element.' must never be the sentinel of getMultidirOutput'); + $this->assertTrue( + $dir === '' || strpos($dir, DOL_DATA_ROOT) === 0, + 'The '.$key.' of the element '.$element.' must be empty or inside DOL_DATA_ROOT, got "'.$dir.'"' + ); + } + } + } + /** * testDolCheckFilters *