Проверка корректности данных

Popoff

popoff.donetsk.ua
А я обычно ещё предварительно проверяю, что пришло число, и если не число - значит вывожу сообщение об ошибке, что, мол, запись с таким идентификатором не найдена, без всяких SQL-запросов.

[вынесено из http://phpclub.ru/talk/showthread.php?s=&postid=855133 ]
 

vovanium

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

Alexandre

PHPПенсионер
Я у себя по логам вижу, как часто всякие кулхацкеры пытаются ввести левые данные, зачем для таких людей пытаться правильно вывести контент, их контент в принципе не интересует, раз они вводят всякую фигню
и что же ты тогда делаешь?
 

vovanium

Новичок
Alexandre
и что же ты тогда делаешь?
просто вывожу сообщение об ошибке, зачем выполнять с десяток запросов, парсить шаблоны и т.п., если чел вводил вместо id какую-то строку?
 

vovanium

Новичок
xbs
ОПЕРАТИВНО локализовывать инъекцию
Интересно как ты что-то оперативно найдешь, если ты тупо все запросы отправляешь в БД. Если у меня вместо числа попытаются ввести что-то левое, моя cms соответственно реагирует (я просто выдаю ошибку, обычно 404, по желанию можно логгировать, извещать и т.п.).
Тебе не надо заниматься этим со всеми данными пришедшими от пользователя
Вот в этом главная ошибка, проверять нужно все что пришло от пользователя ;)
 

zerkms

TDD infected
Команда форума
vovanium
Если у меня вместо числа попытаются ввести что-то левое, моя cms соответственно реагирует (я просто выдаю ошибку, обычно 404, по желанию можно логгировать, извещать и т.п.).
а я никогда не проверяю, кстати, и не логгирую. зачем?? ну приведётся строка к нулю, ну будет искаться "новость" с id = 0. и точно так же (!!!) в итоге будет показана 404 ошибка. без дополнительных плясок и кода.
 

Alexandre

PHPПенсионер
vovanium
лень писать было, но делаю типа того:
Код:
$id=(int)$id;
if (!$id) {
// либо эксепшен кидаем, либо  биндим код ошибки и return false;
}
$res=mysql->quere(  "select ... where id=$id " );
естественно для строк либо mysql_real_escape_string();
либо использую подготовленные операторы, там уже заложен escape_string для каждого строкового параметра
 

zerkms

TDD infected
Команда форума
Alexandre
зачем проверять !$id? как потом обрабатывается $res ниже, после запроса (на случай, если запись не найдена)?
 

Alexandre

PHPПенсионер
зачем проверять !$id?
!$id - эквивалентно $id=0, соответственно в $id до преобразования была какая-то текстовая ерунда. Например выполни код
Код:
echo (int)$id='aas23';
- выкидываем из метода, зачем нам выполнять заранее ненужный запрос.
как потом обрабатывается $res ниже, после запроса (на случай, если запись не найдена)?
ну вы совсем тормозите?
Код:
$res=mysql->quere(  "select ... where id=$id " );
if (!$res) {
     //  кидаем эксепшен
    //  или пишем код ошибки "записей нет " и  return false; 
}
вообще-то есть куча разных оберточных классов
 

zerkms

TDD infected
Команда форума
Alexandre
покажи полный псевдокод с обоими условиями. у тебя дублирование в обработчике двух ситуаций? уточни оба обработчика, хотя бы словами. что делается в первом и втором.
 

Alexandre

PHPПенсионер
Код:
  $id=(int)$id;
    if (!$id) {
        $this->errMsg[]='ошибка в user_id'; //  или throw new DbExeption( 'bad user' ); - если мы его определяем и ловим 
        return false;
    }
    $res=mysql->quere(  "select ... where id=$id " );
    if (!$res) {
        $this->errMsg[]='запись не найдена';
        return false; //  //  или throw new DbExeption( 'user not found' ); - если мы его определяем и ловим 
    }
 

zerkms

TDD infected
Команда форума
Alexandre
почему $id, приведённый к 0 из 'asd' - ошибка, а $id = 1 из '1ololo' - нет?
почему ты проверяешь после кастования, а не до?
зачем дублировать часть кода, если проверка у тебя всё равно получается глупая?
ты представляешь, как происходит поиск по PK с $id = 0? насколько он сложный?

мораль: не нужно мудрить, берите и пишите код:
Код:
    $id=(int)$id;
    $res=mysql->quere(  "select ... where id=$id " );
    if (!$res) {
        $this->errMsg[]='запись не найдена';
        return false; //  //  или throw new DbExeption( 'user not found' ); - если мы его определяем и ловим 
    }
и ничегошеньки вы не заметите, уверяю.
 

zerkms

TDD infected
Команда форума
Alexandre
1. повторю вопрос: в условиях наличия соединения, ты представляешь сложность выполнения этого запроса для субд?
2. повторяю вопросы:
почему $id, приведённый к 0 из 'asd' - ошибка, а $id = 1 из '1ololo' - нет?
почему ты проверяешь после кастования, а не до?
 

Фанат

oncle terrible
Команда форума
zerkms
ты прав в аргументах, но неправ в выводе.
я считаю, что проверка аргументов - дело факультатвиное. говорить, что она совсем бессмысленная, наверное, не стоит. кто хочет - пусть делает.
Другое дело, что, как ты совершенно правильно замечаешь, проверять надо не приведением (1' будет валидно), а проверкой типа (и диапазона).
плюс, запрос не делать если эта проверка не прошла.
 

zerkms

TDD infected
Команда форума
*****
да. не делать запрос если нормальная полноценная валидация не прошла - логично.
но если всё сводится к "показать новость с id (int)" то усложнять код дополнительными конструкциями смысла нет.
БД целочисленный автоинкрементный PK находит мгновенно, видя, что он не попадает в min-max диапазоны индекса.
выбирая из "логичность кода" и "производительность" в этом случае я выберу п.1.
 

Фанат

oncle terrible
Команда форума
не, ну это ж псеудокод. нигде же не оговаривается, что это именно целочисленная и именно новость. мы же в общем говорим. а ты о частностях. но в этой частности я с тобой согласен.
что нас к вопросу топикстартера, впрочем, все равно не приближает %)

-~{}~ 27.05.09 15:19:

а мне ему еще про идентификаторы и операторы рассказать надо %)
 

Alexandre

PHPПенсионер
но если всё сводится к "показать новость с id (int)" то усложнять код дополнительными конструкциями смысла нет.
залезли в частности...
с .zerkms согласен - все зависит от запроса
я часто привожу к инт - и не парюсь
для моих целей хватает
 

vovanium

Новичок
Еще раз основные причины почему у меня выдается ошибка в случаях ввода левых данных:
- у сайта далеко не один запрос их около десятка, и когда какой-нибудь бот начнет перебирать различные варианты строк вместо id, то в итоге получится сотни левых некэшированных запросов (в моем случае, по логам было видно, что перебирают боты, причем тупорылые, т.к. пытаются к разным урлам, добавлять одно и тоже)
- когда запросы коверкаются людьми, и в итоге на неправильный запрос выдается страница, то это страница довольно легко может попасть в индекс поисковикам (счетчики, контекстная реклама и т.п.), зачем плодить дубликаты.
zerkms
выбирая из "логичность кода" и "производительность" в этом случае я выберу п.1.
ну и в чем нелогичность? ты же всеравно делаешь проверку, как минимум на то не нулевой ли id, в чем проблема, сделать проверку число это или нет?
Или для тебя проблема в логичности, если будет
if (id = число и id больше нуля) {
выполняем запрос
}
else {
ошибка 404
}
 

Alexandre

PHPПенсионер
почему $id, приведённый к 0 из 'asd' - ошибка, а $id = 1 из '1ololo' - нет?
и то, и то - ошибка, но часто она бывает слишком мелочная, чтоб на нее обращать внимания
почему ты проверяешь после кастования, а не до?
что значить после кастования?
 
Сверху