LINUX.ORG.RU
ФорумTalks

За что не любят пхпшников

 


0

1

Открываю сегодня один проект. Перехожу в мастер ветку, делаю гит пулл. И вижу такую картину в одном из файлов:

            $id = 0;
            for(; $id < 1000000; $id++) {
                if($_GET['key'] == md5($id . 'security')) {
                    break;
                }
            }
            if($id) {
                $file = query("select * from files where id='" . $id . "'");
                header("Content-type: " . $file['type']);
                header('Content-Length: ' . $file['size']);
                header('Content-Disposition: attachment; filename="' . $file['name'] . '"');

Для тех кто не понял что тут происходит: получаем значение ключа из гета и в цикле перебираем варианты от 0 до 1 млн, если присланный ключ совпал с хешем, тогда «бряк» :)

Не-не, а дальше то - if($id) т.е. если $id больше нуля, что будет однозначно, т.к. хотябы одна итерация но будет - лезем в базу и отдаем файл.

А цикл - видимо сделана задержка а-ля comet, мол, ты прислал какашку - на тебе мильён итераций в цикле!

При этом, замечу, отдаётся файл по найденному в этом цикле $id. То есть это отдача файла по его номеру в закодированном виде, фактически. Только кривая, да. Надо было заранее все хэши в базу положить, и селектить по ним.

om-nom-nimouse ★★ ()
Ответ на: комментарий от om-nom-nimouse

Да ладно тебе, о PHP-коде за авторством людей, знающих смысл термина «вычислительная сложность алгоритма» никто и не знает.

За что не любят пхпшников

За изобилие говнокода и соответствующую репутацию же.

t184256 ★★★★★ ()
Ответ на: комментарий от t184256

Альтернативы? Хотя зачем я это спрашиваю. Не я же вешал этот проект на гит.

deep-purple ★★★★★ ()

А вот постгрес умеет md5 и функциональные индексы.

crowbar ()

есть же всякие govnokod.ru, code_wtf

bl ★★★ ()
Ответ на: комментарий от bl

И правильно. Автор явно твёрдо выучил, что преждевременная оптимизация - зло.

Suntechnic ★★★★★ ()

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

Wolfram ()

здравствуйте ,это сайт про пхп?

Мусье,вы вкладочкой с хаброй не ошиблись?

GNU-Ubuntu1204LTS ★★★ ()
Ответ на: комментарий от t184256

Да ладно тебе, о PHP-коде за авторством людей, знающих смысл термина «вычислительная сложность алгоритма» никто и не знает.

Их работодатели знают.

drull ★☆☆☆ ()

Да вроде понятно всё. Есть пронумерованные файлы. Клиенту выдается право какой-то из этих файлов скачать. Но чтобы не палить внутреннюю структуру вначале кодируют номер файла, чтобы клиент считал что это ему магическую вундервафлю выдали а не номер. Ну и потом когда клиент пришел с правильным GETом, ему этот файл выгребли из базки и выплюнули.

код ужасный, идея ужасная)

stevejobs ★★★★☆ ()

делаю гит пулл

Если делаешь pull, используй git pull --ff-only. Иначе он делает автоматическое слияние веток.

i-rinat ★★★★★ ()

Типичный говнокод, как уже высказались, ничего нового.

Почему не любят? Чаще всего относятся брезгливо, естественная реакция цивилизованного человека, из гигиенических соображений. Не любят за то что распространяют заразу, коей является похапе.

outtaspace ★★★ ()

Норм код. Теперь, если ты знаешь где тормоза происходят, сделай оптимизации.

gh0stwizard ★★★★★ ()
Ответ на: комментарий от vtVitus

Я пользовался гитом, когда это было ещё не модно и молодёжно!

Ха. Я перестал пользоваться гитом еще до того, как молодежь о нем узнала.

tailgunner ★★★★★ ()
Ответ на: комментарий от outtaspace

Типичный говнокод, как уже высказались, ничего нового.

В чем именно говнокод? Я вот вижу, что многие даже не поняли смысл. Цикл делается для реверса md5, аля микро-брутфорс. Другое дело, что ни ТС, ни кто-либо еще не откоментировал, что означает $_GET['key'] и как он проставляется, зачем нужен и т.п. Есть идея, что это своя реализация кукисов, но что-то не похоже :)

gh0stwizard ★★★★★ ()
Ответ на: комментарий от gh0stwizard

Да в общем везде.

Константа 1000000, инициализирована переменная $id, сам цикл for не нужен, конкатенации не нужны, select *, сравнение с приведением типов вместо проверки идентичности.

outtaspace ★★★ ()
Ответ на: комментарий от bluesman

да, это хороший индикатор неосиляторов гита

n_play ()

Сколько БОЛИ php-шных макак в треде.

Запели свои песни про «говнокод на любом языке». Но пока что говнокод есть только у них.

kike ()
Ответ на: комментарий от kike

php - это инструмент. Если ты так возносишь инструмент что клеишь на себя ярлык какого-либо ЯП, то мне тебя жаль.

ritsufag ★★★★★ ()
Ответ на: комментарий от outtaspace

Константа 1000000

Так-так, начали с Макконнелла. Однако, это не является критической ошибкой, особенно, если весь код умещается в файле из 25 строк.

инициализирована переменная $id

Конечно, можно запихнуть в цикл. Только это поломает логику, лол.

сам цикл for не нужен

А что нужно?

конкатенации не нужны

Так-так, sprintf() стопудово быстрее и оптимизирован лучше в ЯП, где конкатенации и так оптимизированны.

select *

Вот. Вот это реально полезно оптимизировать. Молодец.

сравнение с приведением типов вместо проверки идентичности

Если я правильно все понимаю в $_GET['anykey'] всегда находятся строки. Они даже не конвертируются при парсинге. Даже если там будет число, то сверка с типом не даст прироста производительности, т.к. выполнение приведения типа не менее затратно чем сравнить два числа (первый символ в строке).

gh0stwizard ★★★★★ ()
Последнее исправление: gh0stwizard (всего исправлений: 1 )
Ответ на: комментарий от deep-purple

Ты кормишь зелёного человечка.

Deleted ()

А какие варианты решения задачи при условии, что клиенту нельзя отдавать номер файла, а базу данных трогать запрещено?

Хеш заменить на симметричный шифр — нормальный вариант?

crowbar ()
Ответ на: комментарий от crowbar

Все проще. Файлы начинаются с 12345678. Итого $id + 12345678 = id в БД.

gh0stwizard ★★★★★ ()
Ответ на: комментарий от outtaspace

конкатенации не нужны

Всетаки, забыл про bind_columns. Тут, конечно ты прав. Однако, без литратестов точно не сказать, уж очень примитивно все.

gh0stwizard ★★★★★ ()
Ответ на: комментарий от J

уши не в ту сторону смотрят

Нет, все корректно, уши эти - жопные, и смотрят в правильную сторону.

deep-purple ★★★★★ ()
Ответ на: комментарий от crowbar

А какие варианты решения задачи при условии, что клиенту нельзя отдавать номер файла, а базу данных трогать запрещено?

Более быстрый, но не менее укуренный вариант - проверять, что $_GET['key'] и в самом деле /^[0-9a-zA-Z]{32}$/, после чего делать select по MD5(CONCAT(id, 'security')) = $_GET['key']

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

om-nom-nimouse ★★ ()
Последнее исправление: om-nom-nimouse (всего исправлений: 1 )
Ответ на: комментарий от om-nom-nimouse

Это если только при генерации самой ссылки с кеем так же тащить хешсумму айдишника из муцкуля

deep-purple ★★★★★ ()
Вы не можете добавлять комментарии в эту тему. Тема перемещена в архив.