Обобщенная функция PHP для редактирования данных в таблице


У меня есть некоторые устаревшие код, который мне нужно поддерживать и есть эта функция, которая работает прекрасно, но я пытаюсь понять, если он работает с помощью кодирования практики или нет... я пытаюсь понять, если это безопасно и безопасной работы сайта с функцией как-она-есть.

Функция будет называться:

$this->editData('users',"activation_key",$_GET['key']);

Фактическая функция (неотредактированные):

function editData($table_name,$fldname,$fldval,$other='',$lang_flag=0)
{

    $link = $this->my_connect();
    $this->my_select_db($this->DATABASE_NAME,$link);
$arr_types =  array("TR_", "TN_", "TREF_", "PHR_", "PHN_", "IR_", "IN_", "MR_", "MN_", "TNEF_", "TRC_", "TNC_", "TRFN_", "TNFN_","TNURL_","TRURL_");
    if (!empty($GLOBALS["HTTP_POST_VARS"])) {
        reset($GLOBALS["HTTP_POST_VARS"]);
        while (list($k,$v)=each($GLOBALS["HTTP_POST_VARS"])) 
        {

            for($p=0;$p<count($arr_types);$p++)
            {
                if(stristr($k,$arr_types[$p])!="")
                {
                    $k = str_replace($arr_types[$p],"",$k);
                }
            }
            ${strtolower($k)}=$v;
            //echo "<br> k =$k -- v = $v";
        }   
    }
    if (!empty($_POST)) {
        reset($_POST);
        while (list($k,$v)=each($_POST)) 
        {

            for($p=0;$p < count($arr_types);$p++)
            {
                if(stristr($k,$arr_types[$p])!="")
                {
                    $k = str_replace($arr_types[$p],"",$k);
                }
            }
            ${strtolower($k)}=$v;
            //echo "<br> k =$k -- v = $v";
        }   
    }
    if (!empty($GLOBALS["HTTP_GET_VARS"])) {
        reset($GLOBALS["HTTP_GET_VARS"]);
        while (list($k,$v)=each($GLOBALS["HTTP_GET_VARS"])) 
        {

            for($p=0;$p < count($arr_types);$p++)
            {
                if(stristr($k,$arr_types[$p])!="")
                {
                    $k = str_replace($arr_types[$p],"",$k);
                }
            }
            ${strtolower($k)}=$v;
            //echo "<br> k =$k -- v = $v";
        }   
    }
    if (!empty($_GET)) {
        reset($_GET);
        while (list($k,$v)=each($_GET)) 
        {

            for($p=0;$p < count($arr_types);$p++)
            {
                if(stristr($k,$arr_types[$p])!="")
                {
                    $k = str_replace($arr_types[$p],"",$k);
                }
            }
            ${strtolower($k)}=$v;
            //echo "<br> k =$k -- v = $v";
        }   
    }
     $result=$this->my_query("show fields from $table_name",$link);
    $query="update $table_name SET   ";
     while ($a_row = $this->my_fetch_array($result)) {
        $field="$a_row[Field]";


            if($field!=$fldname)
            {
                if(isset($$field))
                {
                       $query.="`".$field."`=";

                       $query.="'".$this->removeQuotes($$field,$lang_flag)."' , ";
                }
                else
                {
                    if(isset($GLOBALS["$field"]))
                    {
                        //echo "<br> var ".$GLOBALS["$field"];
                        $query.="`".$field."`=";
                        $query.="'".$this->removeQuotes($GLOBALS["$field"],$lang_flag)."' , ";

                    }
                }
            }
     }
     $query = substr($query,0,-2);
    $query.=" where `$fldname`='$fldval' $other"; 
    $query  ;
    //echo $query;
     $result=$this->my_query($query,$link);
     $this->my_free_result($result);
     $this->my_close($link);
     return $result;
}


681
5
задан 10 февраля 2011 в 08:02 Источник Поделиться
Комментарии
1 ответ

Проще говоря, этот код не много лучшей практики также содержит огромные проблемы безопасности.
Предложение по улучшению для лучшей практики:


$ГЛОБАЛС["HTTP_POST_VARS"] является устаревшим в пользу из $_POST, где

Так ты обходишь $ГЛОБАЛС["HTTP_POST_VARS"] и $_POST, где избыточна и плохое. Просто цикл через $_POST, где будет сделать то же самое. То же самое идет для $ГЛОБАЛС["HTTP_GET_VARS"] и переменная$_GET.


Вы должны быть проходя в объект $ссылку как параметр.

Поскольку нет кода, предназначенного для изменения условно-подключение к базе данных (параметров подключения всегда одинаковы). Это предотвращает ваши скрипты создания нескольких баз данных, средств связи и разрушая их (больше памяти и времени). В ресурс базы данных в качестве параметра функции-это тип внедрения зависимостей, который делает ваш код для тестирования, если вы хотите в будущем. Так как это также появляется, чтобы быть в классе, рассмотреть возможность создания подключения к базе данных в __построить и использовать один экземпляр.


Использование сессий в пользу $ГЛОБАЛС

Практически любое использование $Globals-это считается плохой практикой. Я уверен, что есть несколько исключений из правил. Но вам определенно не следует использовать для хранения данных в scopeless моды. Это занятия для.


removeQuotes() никогда не вызывается на $имя поля или $fldval

Как вы можете себе представить этот серьезный вопрос безопасности, оставляя свой сценарий открыт для атак путем внедрения кода SQL.


Использовать использования mysql_real_escape_string в пользу домашнего бактерицидная функции, т. е. removeQuotes()

Существует гораздо больше для безопасного побега параметров запроса, удаление слешей. И по моему опыту домашние санировать функции часто незаметно не удастся, когда вы перепутали строки и количеством параметров.

Как предмет читабельности и стиля , я хотел бы сделать следующие предложения, я знаю эти вопросы, но есть и общие способы делать вещи.


Заменить использование конкатенации строк

$query.="`".$field."`=";
$query.="'".$this->removeQuotes($$field,$lang_flag)."' , ";
... more concatenation ...
$query = substr($query,0,-2);

за время работы

$queryParams[] = "`".$field."`='".$this->removeQuotes($$field,$lang_flag)."'";
... more array filling ...
$query = join(',', $queryParams);

Общие WTFs


Это ничего не делать вообще $запроса ;

И код, который ничего не делает очень плохое.


Называть графа() один раз.

Это не проблема, что-то подобное

для($Р=0;$п

На самом деле называет графа() на каждой итерации цикла, даже если возвращаемое значение никогда не изменится. Это заставляет переводчика, чтобы сделать больше работы, чем нужно, хотя я подозреваю, что во время компиляции в байт-код, это будет автоматически оптимизирован. Это может быть заменен на:

// only call count() once
$count = count($arr_types);
for($p=0;$p<$count;$p++)


Не гнездо без надобности.

Не гнездо условные (если/другое/переключатель) и петли(В/К) без надобности. Это делает код сложнее, код труднее читать и мысленно следовать, и, главное, трудно проверить.

if(isset($$field))
{
$query.="`".$field."`=";

$query.="'".$this->removeQuotes($$field,$lang_flag)."' , ";
}
else
{
if(isset($GLOBALS["$field"]))
{
$query.="`".$field."`=";
$query.="'".$this->removeQuotes($GLOBALS["$field"],$lang_flag)."' , ";
}
}

Приведенный выше код в точности равна следующей, менее раскроя.

if(isset($$field))
{
$query.="`".$field."`=";

$query.="'".$this->removeQuotes($$field,$lang_flag)."' , ";
}
else if(isset($GLOBALS["$field"]))
{
$query.="`".$field."`=";
$query.="'".$this->removeQuotes($GLOBALS["$field"],$lang_flag)."' , ";
}

Личные предпочтения


Это совершенно субъективно, но есть о чем подумать.

if(stristr($k,$arr_types[$p])!="")

равна

if(stristr($k,$arr_types[$p]) == true)

равна

if(stristr($k,$arr_types[$p]))


Принимая во внимание вышеизложенное, это:

while (list($k,$v)=each($_GET)) 
{
for($p=0;$p < count($arr_types);$p++)
{
if(stristr($k,$arr_types[$p])!="")
{
$k = str_replace($arr_types[$p],"",$k);
}
}
${strtolower($k)}=$v;
}

Точно так же, как:

while (list($k,$v)=each($_GET)) 
{
$count = count($arr_types);
for($p=0; $p<$count; $p++);
{
$k = str_replace($arr_types[$p],"",$k);
}
${strtolower($k)}=$v;
}

Что бы вы предпочли испытаний и/или работать?

10
ответ дан 10 февраля 2011 в 12:02 Источник Поделиться