Форум PHP

Всем здравствуйте!
Ребят, объясните пожалуйста - если человек пишет вот такую строку кода:

$q = mysql_query("SELECT * FROM some_table WHERE id = {$_GET['id']}");

и при этом ранее в своем коде он выполняет такую операцию -

$_GET['id'] = intval($_GET['id']);

,
то что плохого в этом коде? Какие уязвимости, какие замечания по стилю?
Напишите, пожалуйста свое мнение.
Спасибо.

Тут нет уязвимостей

Отрицательные числа проскочат.

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

Это недочёт. Зачем нагружать MySQL заведомо бессмысленными запросами?

А вот еще такой момент - через GET может быть передано число, оно будет даже положительным, но, допустим, такого id в базе нет. Тогда получается нечто похожее на ситуацию с отрицательным числом - в базе заведомо нет такого id, значит MySQL будет так же гоняться впустую. Предлагался такой вариант - сначала проверять, есть ли вообще такой id в базе (это, скорее всего, будет выглядеть как SELECT COUNT(*) FROM... WHERE id = ...), и если есть, то только тогда уже выполнять целевой запрос... Но это как-бы тоже дополнительная нагрузка на сервер БД. Что скажете?

Скажу что это бред.

> Тогда получается нечто похожее на ситуацию с отрицательным числом - в базе заведомо нет такого id.
А вот это как раз нельзя назвать "заведомо", так как мы не знаем этого заранее, без запроса к СУБД.
Например для поля MEDIUMINT UNSIGNED заведомо некорректные данные — не числа, дробные числа и числа, не входящие в промежуток от 0 до 16777215.

> через GET может быть передано число, оно будет даже положительным
Не забываем, что intval() переводит в число всё, даже пустую строку. Так что запрос сработает в любом случае, даже если передавать не числа.

Спасибо за пояснения!

мне не нравится только стиль

А как конкретно можно аргументировать это "не нравится"? Имеется в виду написание кода, его читаемость, зрительное восприятие? Поясните, пожалуйста.

1) Когда я вижу надпись $_GET['id'] , я изначально считаю, что там находится пользовательский ввод и ничто иное.
2) Я не допускаю присваивания переменным суперглобальных массивов (кроме $_SESSION и $GLOBALS конечно) каких-либо значений. Суперглобальные массивы заполняет php на старте и точка. Единственное исключение из этого правила - откат магических кавычек. Именно откат - до состояния пользовательского ввода. Поэтому это скорее не исключение, а подтверждение правила.

Вышеуказанные строки эту позицию грубо нарушают.

Все понятно, благодарю.

еще момент

3) Код, формирующий оператор SQL, я пытаюсь писать так, чтобы ситуация с отсутствием инъекций была ясна из текста самого оператора, и не требовала глядеть куда-то сильно далеко от точки этого текста.

То есть, приведенный мною пример следовало бы записать так:

$q = mysql_query("SELECT * FROM some_table WHERE id = ".intval($_GET['id']));

Я правильно понял?

Да. В таком виде оператор выглядит абсолютно безопасно.

Вот так лучше:

$res = mysql_query("SELECT * FROM some_table WHERE id = ". (int)$_GET['id']);

1. Функция mysql_query возвращает указатель на результат ($res), а не запрос ($q)
2. (int) - языковая конструкция, а intval() - функция. А значит первая работает на много быстрее.

На сколько наносекунд быстрее-то?

На порядок. Мелочь, а приятно.