lainwir3d/dolibarr
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Clean code

Browse Source
This commit is contained in:
Laurent Destailleur 2021-09-30 15:24:57 +02:00
parent aaf170313f
commit 608b6f5fa3
8 changed files with 61 additions and 26 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

6
htdocs/accountancy/admin/card.php
View File

@ -75,7 +75,7 @@ if ($action == 'add' && $user->rights->accounting->chartofaccount) {
setEventMessages($langs->trans("ErrorFieldRequired", $langs->transnoentities("Label")), null, 'errors');
$action = 'create';
} else {
$sql = 'SELECT pcg_version FROM ' . MAIN_DB_PREFIX . 'accounting_system WHERE rowid='.((int) $conf->global->CHARTOFACCOUNTS);
$sql = "SELECT pcg_version FROM " . MAIN_DB_PREFIX . "accounting_system WHERE rowid = ".((int) $conf->global->CHARTOFACCOUNTS);
dol_syslog('accountancy/admin/card.php:: $sql=' . $sql);
$result = $db->query($sql);
@ -138,7 +138,7 @@ if ($action == 'add' && $user->rights->accounting->chartofaccount) {
} else {
$result = $object->fetch($id);
$sql = 'SELECT pcg_version FROM '.MAIN_DB_PREFIX.'accounting_system WHERE rowid='.((int) $conf->global->CHARTOFACCOUNTS);
$sql = "SELECT pcg_version FROM ".MAIN_DB_PREFIX."accounting_system WHERE rowid=".((int) $conf->global->CHARTOFACCOUNTS);
dol_syslog('accountancy/admin/card.php:: $sql=' . $sql);
$result2 = $db->query($sql);
@ -260,7 +260,7 @@ if ($action == 'create') {
print '<input type="text" name="pcg_type" list="pcg_type_datalist" value="'.dol_escape_htmltag(GETPOSTISSET('pcg_type') ? GETPOST('pcg_type', 'alpha') : $object->pcg_type).'">';
// autosuggest from existing account types if found
print '<datalist id="pcg_type_datalist">';
$sql = 'SELECT DISTINCT pcg_type FROM ' . MAIN_DB_PREFIX . 'accounting_account';
$sql = "SELECT DISTINCT pcg_type FROM " . MAIN_DB_PREFIX . "accounting_account";
$sql .= " WHERE fk_pcg_version = '" . $db->escape($accountsystem->ref) . "'";
$sql .= ' AND entity in ('.getEntity('accounting_account', 0).')'; // Always limit to current entity. No sharing in accountancy.
$sql .= ' LIMIT 50000'; // just as a sanity check

4
htdocs/accountancy/journal/bankjournal.php
View File

@ -362,7 +362,7 @@ if ($result) {
// Retrieve the accounting code of the social contribution of the payment from link of payment.
// Note: We have the social contribution id, it can be faster to get accounting code from social contribution id.
$sqlmid = 'SELECT cchgsoc.accountancy_code';
$sqlmid = "SELECT cchgsoc.accountancy_code";
$sqlmid .= " FROM ".MAIN_DB_PREFIX."c_chargesociales cchgsoc";
$sqlmid .= " INNER JOIN ".MAIN_DB_PREFIX."chargesociales as chgsoc ON chgsoc.fk_type = cchgsoc.id";
$sqlmid .= " INNER JOIN ".MAIN_DB_PREFIX."paiementcharge as paycharg ON paycharg.fk_charge = chgsoc.rowid";
@ -1019,7 +1019,7 @@ if (empty($action) || $action == 'view') {
// Test that setup is complete (we are in accounting, so test on entity is always on $conf->entity only, no sharing allowed)
$sql = 'SELECT COUNT(rowid) as nb FROM '.MAIN_DB_PREFIX.'bank_account WHERE entity = '.$conf->entity.' AND fk_accountancy_journal IS NULL AND clos=0';
$sql = "SELECT COUNT(rowid) as nb FROM ".MAIN_DB_PREFIX."bank_account WHERE entity = ".((int) $conf->entity)." AND fk_accountancy_journal IS NULL AND clos=0";
$resql = $db->query($sql);
if ($resql) {
$obj = $db->fetch_object($resql);

2
htdocs/admin/multicurrency.php
View File

@ -138,7 +138,7 @@ if ($action == 'add_currency') {
$TCurrency = array();
$sql = 'SELECT rowid FROM '.MAIN_DB_PREFIX.'multicurrency WHERE entity = '.$conf->entity;
$sql = "SELECT rowid FROM ".MAIN_DB_PREFIX."multicurrency WHERE entity = ".((int) $conf->entity);
$resql = $db->query($sql);
if ($resql) {
while ($obj = $db->fetch_object($resql)) {

4
htdocs/compta/paiement/class/paiement.class.php
View File

@ -304,8 +304,8 @@ class Paiement extends CommonObject
$facid = $key;
if (is_numeric($amount) && $amount <> 0) {
$amount = price2num($amount);
$sql = 'INSERT INTO '.MAIN_DB_PREFIX.'paiement_facture (fk_facture, fk_paiement, amount, multicurrency_amount)';
$sql .= ' VALUES ('.((int) $facid).', '.((int) $this->id).", ".((float) $amount).", ".((float) $this->multicurrency_amounts[$key]).')';
$sql = "INSERT INTO ".MAIN_DB_PREFIX."paiement_facture (fk_facture, fk_paiement, amount, multicurrency_amount)";
$sql .= " VALUES (".((int) $facid).", ".((int) $this->id).", ".((float) $amount).", ".((float) $this->multicurrency_amounts[$key]).")";
dol_syslog(get_class($this).'::create Amount line '.$key.' insert paiement_facture', LOG_DEBUG);
$resql = $this->db->query($sql);

2
htdocs/core/class/commoninvoice.class.php
View File

@ -734,7 +734,7 @@ abstract class CommonInvoice extends CommonObject
$sql .= 'fk_facture, ';
}
$sql .= ' amount, date_demande, fk_user_demande, code_banque, code_guichet, number, cle_rib, sourcetype, entity)';
$sql .= ' VALUES ('.((int) $this->id);
$sql .= " VALUES (".((int) $this->id);
$sql .= ", ".((float) price2num($amount));
$sql .= ", '".$this->db->idate($now)."'";
$sql .= ", ".((int) $fuser->id);

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

@ -8682,7 +8682,7 @@ abstract class CommonObject
// If field is an implicit foreign key field
if (preg_match('/^integer:/i', $this->fields[$key]['type']) && empty($values[$key])) {
if (isset($this->fields[$key]['default'])) {
$values[$key] = $this->fields[$key]['default'];
$values[$key] = ((int) $this->fields[$key]['default']);
} else {
$values[$key] = 'null';
}
@ -8699,9 +8699,9 @@ abstract class CommonObject
$this->db->begin();
if (!$error) {
$sql = 'INSERT INTO '.MAIN_DB_PREFIX.$this->table_element;
$sql .= ' ('.implode(", ", $keys).')';
$sql .= ' VALUES ('.implode(", ", $values).')';
$sql = "INSERT INTO ".MAIN_DB_PREFIX.$this->table_element;
$sql .= " (".implode(", ", $keys).')';
$sql .= " VALUES (".implode(", ", $values).")"; // $values can contains 'abc' or 123
$res = $this->db->query($sql);
if ($res === false) {
@ -8717,7 +8717,7 @@ abstract class CommonObject
// If we have a field ref with a default value of (PROV)
if (!$error) {
if (key_exists('ref', $this->fields) && $this->fields['ref']['notnull'] > 0 && key_exists('default', $this->fields['ref']) && $this->fields['ref']['default'] == '(PROV)') {
$sql = "UPDATE ".MAIN_DB_PREFIX.$this->table_element." SET ref = '(PROV".$this->id.")' WHERE (ref = '(PROV)' OR ref = '') AND rowid = ".((int) $this->id);
$sql = "UPDATE ".MAIN_DB_PREFIX.$this->table_element." SET ref = '(PROV".((int) $this->id).")' WHERE (ref = '(PROV)' OR ref = '') AND rowid = ".((int) $this->id);
$resqlupdate = $this->db->query($sql);
if ($resqlupdate === false) {

10
htdocs/webservices/server_supplier_invoice.php
View File

@ -350,13 +350,9 @@ function getSupplierInvoicesForThirdParty($authentication, $idthirdparty)
if (!$error) {
$linesinvoice = array();
$sql .= 'SELECT f.rowid as facid';
$sql .= ' FROM '.MAIN_DB_PREFIX.'facture_fourn as f';
//$sql.=', '.MAIN_DB_PREFIX.'societe as s';
//$sql.= ' LEFT JOIN '.MAIN_DB_PREFIX.'product as p ON pt.fk_product = p.rowid';
//$sql.=" WHERE f.fk_soc = s.rowid AND nom = '".$db->escape($idthirdparty)."'";
//$sql.=" WHERE f.fk_soc = s.rowid AND nom = '".$db->escape($idthirdparty)."'";
$sql .= " WHERE f.entity = ".$conf->entity;
$sql .= "SELECT f.rowid as facid";
$sql .= " FROM '.MAIN_DB_PREFIX.'facture_fourn as f";
$sql .= " WHERE f.entity = ".((int) $conf->entity);
if ($idthirdparty != 'all') {
$sql .= " AND f.fk_soc = ".((int) $idthirdparty);
}

49
test/phpunit/CodingPhpTest.php
View File

@ -250,7 +250,6 @@ class CodingPhpTest extends PHPUnit\Framework\TestCase
$ok=true;
$matches=array();
// Check string get_class...
preg_match_all('/'.preg_quote('get_class($this)."::".__METHOD__', '/').'/', $filecontent, $matches, PREG_SET_ORDER);
foreach ($matches as $key => $val) {
$ok=false;
@ -260,9 +259,9 @@ class CodingPhpTest extends PHPUnit\Framework\TestCase
$this->assertTrue($ok, 'Found string get_class($this)."::".__METHOD__ that must be replaced with __METHOD__ only in '.$file['relativename']);
//exit;
// Check string $this->db->idate without quotes
$ok=true;
$matches=array();
// Check string $this->db->idate without quotes
preg_match_all('/(..)\s*\.\s*\$this->db->idate\(/', $filecontent, $matches, PREG_SET_ORDER);
foreach ($matches as $key => $val) {
if ($val[1] != '\'"' && $val[1] != '\'\'') {
@ -276,11 +275,10 @@ class CodingPhpTest extends PHPUnit\Framework\TestCase
//exit;
$ok=true;
$matches=array();
// Check sql string DELETE|OR|AND|WHERE|INSERT ... yyy = ".$xxx
// with xxx that is not 'thi' (for $this->db->sanitize) and 'db-' (for $db->sanitize). It means we forget a ' if string, or an (int) if int, when forging sql request.
$ok=true;
$matches=array();
preg_match_all('/(DELETE|OR|AND|WHERE|INSERT)\s.*([^\s][^\s][^\s])\s*=\s*"\s*\.\s*\$(...)/', $filecontent, $matches, PREG_SET_ORDER);
foreach ($matches as $key => $val) {
if ($val[2] == 'ity' && $val[3] == 'con') { // exclude entity = ".$conf->entity
@ -300,8 +298,39 @@ class CodingPhpTest extends PHPUnit\Framework\TestCase
$this->assertTrue($ok, 'Found non quoted or not casted var into sql request '.$file['relativename'].' - Bad.');
//exit;
// Check that forged sql string is using " as string PHP quotes
$ok=true;
$matches=array();
preg_match_all('/\$sql \.= \'\s*VALUES.*\$/', $filecontent, $matches, PREG_SET_ORDER);
foreach ($matches as $key => $val) {
//if ($val[1] != '\'"' && $val[1] != '\'\'') {
var_dump($matches);
$ok=false;
break;
//}
//if ($reg[0] != 'db') $ok=false;
}
//print __METHOD__." Result for checking we don't have non escaped string in sql requests for file ".$file."\n";
$this->assertTrue($ok, 'Found a forged SQL string that mix on same line the use of \' for PHP string and PHP variables into file '.$file['relativename'].' Use " to forge PHP string like this: $sql = "SELET ".$myvar...');
//exit;
// Check that forged sql string is using " as string PHP quotes
/*
$ok=true;
$matches=array();
preg_match_all('/\$sql \.*= \'SELECT.*\$/', $filecontent, $matches, PREG_SET_ORDER);
foreach ($matches as $key => $val) {
var_dump($matches);
$ok=false;
break;
}
$this->assertTrue($ok, 'Found a forged SQL string that mix on same line the use of \' for PHP string and PHP variables into file '.$file['relativename'].' Use " to forge PHP string like this: $sql = "SELET ".$myvar...');
*/
// Check sql string VALUES ... , ".$xxx
// with xxx that is not 'db-' (for $db->escape). It means we forget a ' if string, or an (int) if int, when forging sql request.
$ok=true;
$matches=array();
preg_match_all('/(VALUES).*,\s*"\s*\.\s*\$(...)/', $filecontent, $matches, PREG_SET_ORDER);
foreach ($matches as $key => $val) {
if ($val[1] == 'VALUES' && $val[2] == 'db-') { // exclude $db->escape(
@ -321,6 +350,8 @@ class CodingPhpTest extends PHPUnit\Framework\TestCase
// Check '".$xxx non escaped
// Check string ='".$this->xxx with xxx that is not 'escape'. It means we forget a db->escape when forging sql request.
$ok=true;
$matches=array();
preg_match_all('/=\s*\'"\s*\.\s*\$this->(....)/', $filecontent, $matches, PREG_SET_ORDER);
foreach ($matches as $key => $val) {
if ($val[1] != 'db->' && $val[1] != 'esca') {
@ -332,6 +363,8 @@ class CodingPhpTest extends PHPUnit\Framework\TestCase
$this->assertTrue($ok, 'Found non escaped string in building of a sql request (case 1) in '.$file['relativename'].' - Bad.');
// Check string sql|set...'".$yyy->xxx with xxx that is not 'escape', 'idate', .... It means we forget a db->escape when forging sql request.
$ok=true;
$matches=array();
preg_match_all('/(sql|SET|WHERE|INSERT|VALUES).+\s*\'"\s*\.\s*\$(.........)/', $filecontent, $matches, PREG_SET_ORDER);
foreach ($matches as $key => $val) {
if (! in_array($val[2], array('this->db-', 'this->esc', 'db->escap', 'dbs->esca', 'mydb->esc', 'dbsession', 'db->idate', 'escapedli', 'excludeGr', 'includeGr'))) {
@ -345,6 +378,8 @@ class CodingPhpTest extends PHPUnit\Framework\TestCase
//exit;
// Check string sql|set...'.$yyy->xxx with xxx that is not 'escape', 'idate', .... It means we forget a db->escape when forging sql request.
$ok=true;
$matches=array();
preg_match_all('/(\$sql|SET\s|WHERE\s|INSERT\s|VALUES\s|VALUES\().+\s*\'\s*\.\s*\$(.........)/', $filecontent, $matches, PREG_SET_ORDER);
foreach ($matches as $key => $val) {
if (! in_array($val[2], array('this->db-', 'db->sanit', 'conf->ent', 'key : \'\')', 'key])."\')', 'excludefi', 'regexstri', ''))) {
@ -361,6 +396,8 @@ class CodingPhpTest extends PHPUnit\Framework\TestCase
// Checks with IN
// Check string 'IN (".xxx' or 'IN (\'.xxx' with xxx that is not '$this->db->sanitize' and not '$db->sanitize'. It means we forget a db->sanitize when forging sql request.
$ok=true;
$matches=array();
preg_match_all('/ IN \([\'"]\s*\.\s*(.........)/i', $filecontent, $matches, PREG_SET_ORDER);
foreach ($matches as $key => $val) {
if (!in_array($val[1], array('$db->sani', '$this->db', 'getEntity', 'WON\',\'L', 'self::STA', 'Commande:', 'CommandeF', 'Entrepot:', 'Facture::', 'FactureFo', 'ExpenseRe', 'Societe::', 'Ticket::S'))) {
@ -374,6 +411,8 @@ class CodingPhpTest extends PHPUnit\Framework\TestCase
//exit;
// Check string 'IN (\'".xxx' with xxx that is not '$this->db->sanitize' and not '$db->sanitize'. It means we forget a db->sanitize when forging sql request.
$ok=true;
$matches=array();
preg_match_all('/ IN \(\'"\s*\.\s*(.........)/i', $filecontent, $matches, PREG_SET_ORDER);
foreach ($matches as $key => $val) {
if (!in_array($val[1], array('$db->sani', '$this->db', 'getEntity', 'WON\',\'L', 'self::STA', 'Commande:', 'CommandeF', 'Entrepot:', 'Facture::', 'FactureFo', 'ExpenseRe', 'Societe::', 'Ticket::S'))) {