Wavyzz/dolibarr
2
0
Fork 1
mirror of https://github.com/Dolibarr/dolibarr.git synced 2025-12-06 09:38:23 +01:00
Code Issues Packages Projects Releases Wiki Activity

Clean code

Browse Source
This commit is contained in:
Laurent Destailleur
2024-02-02 19:50:43 +01:00
parent ff04c99542
commit 32e205b5d1
7 changed files with 58 additions and 17 deletions
Download Patch File Download Diff File Expand all files Collapse all files

2
htdocs/admin/system/filecheck.php
View File

@@ -79,7 +79,7 @@ print '<br>';
$file_list = array('missing' => array(), 'updated' => array()); $file_list = array('missing' => array(), 'updated' => array());
// Local file to compare to // 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; $xmlfile = DOL_DOCUMENT_ROOT.'/install/'.$xmlshortfile;
if (!preg_match('/\.zip$/i', $xmlfile) && dol_is_file($xmlfile.'.zip')) { if (!preg_match('/\.zip$/i', $xmlfile) && dol_is_file($xmlfile.'.zip')) {

11
htdocs/api/class/api_setup.class.php
View File

@@ -2060,10 +2060,15 @@ class Setup extends DolibarrApi
$file_list = array('missing' => array(), 'updated' => array()); $file_list = array('missing' => array(), 'updated' => array());
// Local file to compare to // 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; $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 // Remote file to compare to
$xmlremote = ($target == 'default' ? '' : $target); $xmlremote = (($target == 'default' || $target == 'local') ? '' : $target);
if (empty($xmlremote) && getDolGlobalString('MAIN_FILECHECK_URL')) { if (empty($xmlremote) && getDolGlobalString('MAIN_FILECHECK_URL')) {
$xmlremote = getDolGlobalString('MAIN_FILECHECK_URL'); $xmlremote = getDolGlobalString('MAIN_FILECHECK_URL');
} }
@@ -2093,7 +2098,7 @@ class Setup extends DolibarrApi
if (dol_is_file($xmlfile)) { if (dol_is_file($xmlfile)) {
$xml = simplexml_load_file($xmlfile); $xml = simplexml_load_file($xmlfile);
} else { } else {
throw new RestException(500, $langs->trans('XmlNotFound').': '.$xmlfile); throw new RestException(500, $langs->trans('XmlNotFound').': /install/'.$xmlshortfile);
} }
} else { } 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. $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.

12
htdocs/compta/facture/class/facture.class.php
View File

@@ -539,14 +539,16 @@ class Facture extends CommonInvoice
} }
$this->entity = $_facrec->entity; // Invoice created in same entity than template $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 // Fields coming from GUI (priority on template).
$this->fk_project = GETPOST('projectid', 'int') > 0 ? ((int) GETPOST('projectid', 'int')) : $_facrec->fk_project; // @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_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->note_private = GETPOSTISSET('note_private') ? GETPOST('note_private', 'restricthtml') : $_facrec->note_private;
$this->model_pdf = GETPOSTISSET('model') ? GETPOST('model', 'alpha') : $_facrec->model_pdf; $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->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 ? ((int) GETPOST('mode_reglement_id', 'int')) : $_facrec->mode_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 ? ((int) GETPOST('fk_account')) : $_facrec->fk_account; $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 // 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; $this->total_ht = $_facrec->total_ht;

6
htdocs/core/ajax/fileupload.php
View File

@@ -97,14 +97,16 @@ switch ($_SERVER['REQUEST_METHOD']) {
break; break;
case 'POST': case 'POST':
if (isset($_REQUEST['_method']) && $_REQUEST['_method'] === 'DELETE') { if (isset($_REQUEST['_method']) && $_REQUEST['_method'] === 'DELETE') {
$upload_handler->delete(); $file = GETPOST('file');
$upload_handler->delete($file);
} else { } else {
$upload_handler->post(); $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) // 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; break;
case 'DELETE': case 'DELETE':
$upload_handler->delete(); $file = GETPOST('file');
$upload_handler->delete($file);
break; break;
default: default:
header('HTTP/1.0 405 Method Not Allowed'); header('HTTP/1.0 405 Method Not Allowed');

4
htdocs/core/class/commonobject.class.php
View File

@@ -5573,7 +5573,7 @@ abstract class CommonObject
* Common function for all objects extending CommonObject for generating documents * Common function for all objects extending CommonObject for generating documents
* *
* @param string $modelspath Relative folder where generators are placed * @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 Translate $outputlangs Output language to use
* @param int $hidedetails 1 to hide details. 0 by default * @param int $hidedetails 1 to hide details. 0 by default
* @param int $hidedesc 1 to hide product description. 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') 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 param here has been posted, we use this value first.
if (GETPOSTISSET($fieldname)) { if (GETPOSTISSET($fieldname)) {

5
htdocs/core/class/fileupload.class.php
View File

@@ -560,11 +560,12 @@ class FileUpload
/** /**
* Delete uploaded file * Delete uploaded file
* *
* @param string $file File
* @return int * @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); $file_path = $this->options['upload_dir'].dol_sanitizeFileName($file_name);
$success = dol_is_file($file_path) && $file_name[0] !== '.' && unlink($file_path); $success = dol_is_file($file_path) && $file_name[0] !== '.' && unlink($file_path);
if ($success) { if ($success) {

35
test/phpunit/CodingPhpTest.php
View File

@@ -212,8 +212,7 @@ class CodingPhpTest extends PHPUnit\Framework\TestCase
'translate.class.php', 'translate.class.php',
'utils.class.php', 'utils.class.php',
'TraceableDB.php', 'TraceableDB.php',
'multicurrency.class.php', 'multicurrency.class.php'
'infobox.class.php'
))) { ))) {
// Must not find $db-> // Must not find $db->
$ok=true; $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.'); $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; //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 { } else {
// Check into Include files // Check into Include files
if (! in_array($file['name'], array( if (! in_array($file['name'], array(