diff --git a/htdocs/admin/system/filecheck.php b/htdocs/admin/system/filecheck.php index aecb1fd6f7a..3d242947af8 100644 --- a/htdocs/admin/system/filecheck.php +++ b/htdocs/admin/system/filecheck.php @@ -79,7 +79,7 @@ print '
'; $file_list = array('missing' => array(), 'updated' => array()); // Local file to compare to -$xmlshortfile = dol_sanitizeFileName(GETPOST('xmlshortfile', 'alpha') ? GETPOST('xmlshortfile', 'alpha') : 'filelist-'.DOL_VERSION.(!getDolGlobalString('MAIN_FILECHECK_LOCAL_SUFFIX') ? '' : $conf->global->MAIN_FILECHECK_LOCAL_SUFFIX).'.xml'.(!getDolGlobalString('MAIN_FILECHECK_LOCAL_EXT') ? '' : $conf->global->MAIN_FILECHECK_LOCAL_EXT)); +$xmlshortfile = dol_sanitizeFileName(GETPOST('xmlshortfile', 'alpha') ? GETPOST('xmlshortfile', 'alpha') : 'filelist-'.DOL_VERSION.getDolGlobalString('MAIN_FILECHECK_LOCAL_SUFFIX').'.xml'.getDolGlobalString('MAIN_FILECHECK_LOCAL_EXT')); $xmlfile = DOL_DOCUMENT_ROOT.'/install/'.$xmlshortfile; if (!preg_match('/\.zip$/i', $xmlfile) && dol_is_file($xmlfile.'.zip')) { diff --git a/htdocs/api/class/api_setup.class.php b/htdocs/api/class/api_setup.class.php index 93fdcdd0da5..653f04ee0b8 100644 --- a/htdocs/api/class/api_setup.class.php +++ b/htdocs/api/class/api_setup.class.php @@ -2060,10 +2060,15 @@ class Setup extends DolibarrApi $file_list = array('missing' => array(), 'updated' => array()); // Local file to compare to - $xmlshortfile = dol_sanitizeFileName(GETPOST('xmlshortfile', 'alpha') ? GETPOST('xmlshortfile', 'alpha') : 'filelist-'.DOL_VERSION.(!getDolGlobalString('MAIN_FILECHECK_LOCAL_SUFFIX') ? '' : $conf->global->MAIN_FILECHECK_LOCAL_SUFFIX).'.xml'.(!getDolGlobalString('MAIN_FILECHECK_LOCAL_EXT') ? '' : $conf->global->MAIN_FILECHECK_LOCAL_EXT)); + $xmlshortfile = dol_sanitizeFileName('filelist-'.DOL_VERSION.getDolGlobalString('MAIN_FILECHECK_LOCAL_SUFFIX').'.xml'.getDolGlobalString('MAIN_FILECHECK_LOCAL_EXT')); + $xmlfile = DOL_DOCUMENT_ROOT.'/install/'.$xmlshortfile; + if (!preg_match('/\.zip$/i', $xmlfile) && dol_is_file($xmlfile.'.zip')) { + $xmlfile = $xmlfile.'.zip'; + } + // Remote file to compare to - $xmlremote = ($target == 'default' ? '' : $target); + $xmlremote = (($target == 'default' || $target == 'local') ? '' : $target); if (empty($xmlremote) && getDolGlobalString('MAIN_FILECHECK_URL')) { $xmlremote = getDolGlobalString('MAIN_FILECHECK_URL'); } @@ -2093,7 +2098,7 @@ class Setup extends DolibarrApi if (dol_is_file($xmlfile)) { $xml = simplexml_load_file($xmlfile); } else { - throw new RestException(500, $langs->trans('XmlNotFound').': '.$xmlfile); + throw new RestException(500, $langs->trans('XmlNotFound').': /install/'.$xmlshortfile); } } else { $xmlarray = getURLContent($xmlremote, 'GET', '', 1, array(), array('http', 'https'), 0); // Accept http or https links on external remote server only. Same is used into filecheck.php. diff --git a/htdocs/compta/facture/class/facture.class.php b/htdocs/compta/facture/class/facture.class.php index b1047c84903..ad43e7e250a 100644 --- a/htdocs/compta/facture/class/facture.class.php +++ b/htdocs/compta/facture/class/facture.class.php @@ -539,14 +539,16 @@ class Facture extends CommonInvoice } $this->entity = $_facrec->entity; // Invoice created in same entity than template - // Fields coming from GUI (priority on template). TODO Value of template should be used as default value on GUI so we can use here always value from GUI - $this->fk_project = GETPOST('projectid', 'int') > 0 ? ((int) GETPOST('projectid', 'int')) : $_facrec->fk_project; + // Fields coming from GUI (priority on template). + // @TODO Value of template should be used as default value on the form on the GUI, and we should here always use the value from GUI + // set by posted page wth $object->xxx = ... and this section should be removed. + $this->fk_project = GETPOST('projectid', 'int') > 0 ? GETPOSTINT('projectid') : $_facrec->fk_project; $this->note_public = GETPOSTISSET('note_public') ? GETPOST('note_public', 'restricthtml') : $_facrec->note_public; $this->note_private = GETPOSTISSET('note_private') ? GETPOST('note_private', 'restricthtml') : $_facrec->note_private; $this->model_pdf = GETPOSTISSET('model') ? GETPOST('model', 'alpha') : $_facrec->model_pdf; - $this->cond_reglement_id = GETPOST('cond_reglement_id', 'int') > 0 ? ((int) GETPOST('cond_reglement_id', 'int')) : $_facrec->cond_reglement_id; - $this->mode_reglement_id = GETPOST('mode_reglement_id', 'int') > 0 ? ((int) GETPOST('mode_reglement_id', 'int')) : $_facrec->mode_reglement_id; - $this->fk_account = GETPOST('fk_account') > 0 ? ((int) GETPOST('fk_account')) : $_facrec->fk_account; + $this->cond_reglement_id = GETPOST('cond_reglement_id', 'int') > 0 ? GETPOSTINT('cond_reglement_id') : $_facrec->cond_reglement_id; + $this->mode_reglement_id = GETPOST('mode_reglement_id', 'int') > 0 ? GETPOSTINT('mode_reglement_id') : $_facrec->mode_reglement_id; + $this->fk_account = GETPOST('fk_account') > 0 ? GETPOSTINT('fk_account') : $_facrec->fk_account; // Set here to have this defined for substitution into notes, should be recalculated after adding lines to get same result $this->total_ht = $_facrec->total_ht; diff --git a/htdocs/core/ajax/fileupload.php b/htdocs/core/ajax/fileupload.php index 25ea79b1939..14523c60719 100644 --- a/htdocs/core/ajax/fileupload.php +++ b/htdocs/core/ajax/fileupload.php @@ -97,14 +97,16 @@ switch ($_SERVER['REQUEST_METHOD']) { break; case 'POST': if (isset($_REQUEST['_method']) && $_REQUEST['_method'] === 'DELETE') { - $upload_handler->delete(); + $file = GETPOST('file'); + $upload_handler->delete($file); } else { $upload_handler->post(); // Note: even if this return an error on 1 file in post(), we will return http code 200 because error must be managed by the caller (some files may be ok and some in error) } break; case 'DELETE': - $upload_handler->delete(); + $file = GETPOST('file'); + $upload_handler->delete($file); break; default: header('HTTP/1.0 405 Method Not Allowed'); diff --git a/htdocs/core/class/commonobject.class.php b/htdocs/core/class/commonobject.class.php index 377914449c2..c8e8c7176ec 100644 --- a/htdocs/core/class/commonobject.class.php +++ b/htdocs/core/class/commonobject.class.php @@ -5573,7 +5573,7 @@ abstract class CommonObject * Common function for all objects extending CommonObject for generating documents * * @param string $modelspath Relative folder where generators are placed - * @param string $modele Generator to use. Caller must set it to obj->model_pdf or GETPOST('model_pdf','alpha') for example. + * @param string $modele Generator to use. Caller must set it to obj->model_pdf or $_POST for example. * @param Translate $outputlangs Output language to use * @param int $hidedetails 1 to hide details. 0 by default * @param int $hidedesc 1 to hide product description. 0 by default @@ -5952,7 +5952,7 @@ abstract class CommonObject **/ public function getDefaultCreateValueFor($fieldname, $alternatevalue = null, $type = 'alphanohtml') { - global $conf, $_POST; + global $_POST; // If param here has been posted, we use this value first. if (GETPOSTISSET($fieldname)) { diff --git a/htdocs/core/class/fileupload.class.php b/htdocs/core/class/fileupload.class.php index 53379f3323f..0a9523a5e82 100644 --- a/htdocs/core/class/fileupload.class.php +++ b/htdocs/core/class/fileupload.class.php @@ -560,11 +560,12 @@ class FileUpload /** * Delete uploaded file * + * @param string $file File * @return int */ - public function delete() + public function delete($file) { - $file_name = GETPOST('file') ? basename(GETPOST('file')) : null; + $file_name = $file ? basename($file) : null; $file_path = $this->options['upload_dir'].dol_sanitizeFileName($file_name); $success = dol_is_file($file_path) && $file_name[0] !== '.' && unlink($file_path); if ($success) { diff --git a/test/phpunit/CodingPhpTest.php b/test/phpunit/CodingPhpTest.php index 10cfda909e7..1480203b604 100644 --- a/test/phpunit/CodingPhpTest.php +++ b/test/phpunit/CodingPhpTest.php @@ -212,8 +212,7 @@ class CodingPhpTest extends PHPUnit\Framework\TestCase 'translate.class.php', 'utils.class.php', 'TraceableDB.php', - 'multicurrency.class.php', - 'infobox.class.php' + 'multicurrency.class.php' ))) { // Must not find $db-> $ok=true; @@ -228,6 +227,38 @@ class CodingPhpTest extends PHPUnit\Framework\TestCase $this->assertTrue($ok, 'Found string $db-> into a .class.php file in '.$file['relativename'].'. Inside a .class file, you should use $this->db-> instead.'); //exit; } + + if (preg_match('/\.class\.php/', $file['relativename']) && ! in_array($file['relativename'], array( + 'adherents/canvas/actions_adherentcard_common.class.php', + 'contact/canvas/actions_contactcard_common.class.php', + 'compta/facture/class/facture.class.php', + 'core/class/commonobject.class.php', + 'core/class/extrafields.class.php', + 'core/class/html.form.class.php', + 'core/class/html.formfile.class.php', + 'core/class/html.formcategory.class.php', + 'core/class/html.formmail.class.php', + 'core/class/html.formother.class.php', + 'core/class/html.formsms.class.php', + 'core/class/html.formticket.class.php', + 'core/class/utils.class.php', + ))) { + // Must not find GETPOST + $ok=true; + $matches=array(); + // Check string GETPOSTFLOAT a class.php file (should not be found into classes) + preg_match_all('/GETPOST\(["\'](....)/', $filecontent, $matches, PREG_SET_ORDER); + foreach ($matches as $key => $val) { + if (in_array($val[1], array('lang', 'forc'))) { + continue; + } + //var_dump($val); + $ok=false; + break; + } + //print __METHOD__." Result for checking we don't have non escaped string in sql requests for file ".$file."\n"; + //$this->assertTrue($ok, 'Found string GETPOST into a .class.php file in '.$file['relativename'].'.'); + } } else { // Check into Include files if (! in_array($file['name'], array(