Часто вижу, как люди плохо отзываются о php-программистах и о php в целом как языке программирования. Сам я не сталкивался с проблемами языка — пишу себе небольшие проекты, сторонние фреймворки не использую и радуюсь жизни. Иногда обращаются знакомые с просьбами. Если задачка интересная или просто малозатратная по времени, то обычно я не отказываю. И вот обратился ко мне знакомый с просьбой посмотреть, почему плагин автопостинга в различные социальные сети не работает с Instagram как полагается. Долго не думая, я прикинул, что ничего сложного в этом нет — открою sublime text, скачаю исходники плагина, поиском доберусь до кода постинга в Instagram и поправлю необходимое, но не тут-то было…
Настроил FtpSync, скачал папку плагина, открыл подходящий по названию файл и увидел что-то вроде такого:
if (!function_exists('CutFromTo')){ function CutFromTo($string, $from, $to){$fstart = stripos($string, $from); $tmp = substr($string,$fstart+strlen($from)); $flen = stripos($tmp, $to); return substr($tmp,0, $flen);}}
Именно так: по несколько операций в одну строчку файл с общим количеством строк больше тысячи. Тут я понял, что дело плохо и открыл phpstorm, через Tools -> Deployment настроил доступ к ftp. Дальше с помощью Ctrl + Shift + Alt + L привел код в порядок и он стал более читабельным:
if (!function_exists('CutFromTo')) {
function CutFromTo($string, $from, $to)
{
$fstart = stripos($string, $from);
$tmp = substr($string, $fstart + strlen($from));
$flen = stripos($tmp, $to);
return substr($tmp, 0, $flen);
}
}
Проблема плагина заключалась в том, что при публикации картинки вместо заголовка выводилась ошибка с приблизительным содержанием “Can not upload image to /tmp/FILE_NAME”, но картинка при этом успешно заливается. Поиском по файлам (Ctrl + Shift + F) я быстро добрался до единственного места в коде, где используется именно такой текст. (К слову похожий текст ошибки с разной вариацией постановки слов в предложении был в различных местах кода. Разработчикам стоило вынести текст в константу или создать функцию для получения различных вариаций в зависимости от параметров. В общем, присутствие copy/paste programming.)
Мой знакомый пытался гуглить ошибку и даже нашел на форуме авторов плагина тему пользователя с этой же проблемой. Представители плагина недвусмысленно попытались объяснить, что человеку стоит лучше понимать php, указали, что в их коде используются только системные функции и проблема не на их стороне, предложив почитать FAQ. В общем и целом поддержка на низком уровне.
Простым тестом добавления слова в строку я попытался убедиться, что вызывается именно найденная мной функция в коде, но нет. После небольшого удивления я стал отслеживать цепочку вызовов при нажатии на кнопку. В точно так же небрежно отформатированном javascript коде я нашел какой action отсылается через ajax, опять же поиском нашел место в коде php и с помощью
echo “aga”;
постоянно проверял, что двигаюсь по верному пути, пока не пришел вот к такому месту в коде:
$nt = new SomeClass();
$nt->debug = false;
if (!empty($options['ck'])) $nt->ck = $options['ck'];
if (!empty($options['proxy']) && !empty($options['proxyOn'])) {
$nt->proxy['proxy'] = $options['proxy']['proxy'];
if (!empty($options['proxy']['up'])) $nt->proxy['up'] = $options['proxy']['up'];
};
$loginErr = $nt->connect($options['uName'], $pass);
if (!$loginErr) $ret = $nt->post($msg, $imgURL, $options['imgAct']); else {
$badOut['Error'] .= 'Something went wrong - ' . print_r($loginErr, true);
$ret = $badOut;
}
Отдельно картинка из phpstorm ide:
Вот я пришел к необъявленому классу. Первым делом скачал с сервера все остальные исходники движка и плагинов и поиском по всем файлам попытался найти определение класса. Был достаточно сильно удивлен, когда ничего не нашлось. Другими словами постинг картинки происходит в классе, которого нет в исходниках.
Спросил у знакомого, что же это за плагин, в результате чего выяснилось, что плагин публичный, конкретно данная модификация (постинг в Instagram) в нем платная и его разработчики индусы. Все остальные социальные сети работают хорошо. Мое удивление от происходящего только усилилось, но ладно.
Следующим кодом:
$reflector = new ReflectionClass('SomeClass');
echo $reflector->getFileName();
добрался до файла в уже другом плагине и в указанной строчке кода я нашел вызов функции ‘eval’:
$t = get_site_option($this->c);
$d = $this->k . 'decode';
if (!empty($t)) {
$t = $d($t);
eval($t);
}
Плагин с такой вот функцией — это платное дополнение для подключения Instagram в том числе. Исходный код для работы с “премиум” сетями сохраняется в базе и при каждой загрузке грузится из базы с помощью ‘eval’.
Наконец-то я добрался до кода, в котором происходит вывод не актуальной ошибки, теперь мне необходимо было открыть код в phpstorm для последующего анализа, для чего я просто код из базы записал в файл и для возможности что-то изменить делаю require вместо eval:
$t = get_site_option($this->c);
$fileName = $path . $this->c . '.php';
$d = $this->k . 'decode';
if (!empty($t)) {
$t = $d($t);
if (file_exists($fileName)) {
require_once $fileName;
} else {
eval($t);
file_put_contents($fileName, "<?php $t ?>");
}
}
Плагином предусматривается промежуточное сохранение публикуемой картинки в папке /tmp в системе и в случае, если не получается, то в папке WordPress для загрузок. Строка с ошибкой используется в нескольких местах кода и остальную часть кода для записи и проверки на запись в разных временных папках разработчики просто копировали и немного изменяли текст ошибки. Собственно, в переменную записывалась только ошибка сохранения файла:
if (!is_writebale($path)) {
$variable = “can not upload image from /tmp/FILENAME”;
if (!tryToSaveImageInWordpressFolder()) {
return $variable;
}
}
Непонятно по каким причинам эта же переменная далее в коде использовалась для caption атрибута при постинге на instagram:
sendImageToInstagram(array('caption' => $variable));
Поменял переменную и готово. 5 минут на выявление и исправление бага и около 2 часов на поиск расположения необходимого кода с указанным багом.
Пожалуй, надо бы сделать какие-то выводы из всей ситуации. На самом деле какой-то высокой морали в этом нет. После «приключений» мне просто захотелось поделиться. Так что как я уже написал во вступлении это просто небольшая история из моего опыта с чужим кодом, плохим кодом. Надеюсь, кого-то история все-таки улыбнула, кто-то может заметил для себя что-то новое в подходе анализа чужого кода, а кто-то может быть и вовсе взял для себя на заметку, как лучше оформлять свой код.
В процессе анализа я столкнулся с множеством штамповых проблем, самые основные из них:
- нелепое оформление кода: несколько операций в одну строчку,
- активное применение copy/paste programming,
- использование одной и той же переменной для разных целей, в чем и заключался конечный баг,
- еще одна большая проблема чтения была в конкатенации названий функций, когда собирается название вызываемой функции из нескольких строк ($func = $prefix. "_". $name) — гибко, да, но в будущем с помощью, например, Alt + F7 в ide теряется возможность отслеживания использования таких функций и повышается сложность чтения, я бы рекомендовал использовать switch и константы.
Было и множество других незначительных упущений, которые по чуть-чуть делали код всё более нечитабельным для посторонних. Самая большая проблема в том, что синтаксис php всё это позволяет сделать и люди, к сожалению, пользуются данной свободой не по назначению. Я понимаю, что разработчики, сохраняя исходный код в базе движка, за который они берут деньги, таким образом попытались сделать его менее доступным, но это крайне неверный подход. Самый банальный аргумент: исходя из моего опыта этот подход не сработал — я без особых сложностей всё равно добрался до платных исходников, но из-за зря потраченного мной времени моё желание распространять этот платный код бесплатно только усилилось, а так оно было нулевым. Даже появлялись мысли вообще написать свой плагин с точно такой же функциональностью, чтобы намеренно составить конкуренцию разработчикам. В общем, если скрыть исходники всё равно не получается, то сделайте их читабельными для других разработчиков. Это точно не скажется отрицательно на вашей доходности, но зато очень хорошо скажется на репутации и только повысит ваши возможности в перспективе заработать больше.
Комментарии (29)
torrie
18.11.2017 23:01Поэтому php-разработчиков и не любят: нет жестких рамок(как в python например касаемо форматирования), легаси-код, скрытие ошибок(код запустился без класса?), evalы, через которые легко можно вскрыть сайт. Но это ложь, их не любят по другой причине. Сам пишу на всем, в том числе на php. Не от языка программирования отношение зависит, а от людей. Просто php проще для входа, поэтому новичков в php больше, а значит больше легаси и косяков. Python на винду поставить сложнее, чем denwer. И слава байт-коду.
Chaak
19.11.2017 01:08Практики CodeReview никто не отменял. Практически на любом языке можно накосячить.
www.php-fig.org/psr/psr-2
github.com/php-fig/fig-standards/blob/master/proposed/extended-coding-style-guide.md
JekaMas
19.11.2017 13:07Не надо про python… Тоже всякого пишут. Везде всякого пишут. И в императивных, и в декларативных, и в php, и в python и даже в строгом golang.
dkdkdk
18.11.2017 23:09А как Вам вот такое (Delphi)?
procedure TPrintClass.GetPreview(ofsP:integer=0);
var rw, rp, cr, PhysM:TRect;
pw, ph, ww, wh, n:integer;
kx, ky, k, sc:double;
land:boolean;
ofs:TPoint;
MyRgn: HRGN;
mt, ml, mr, mb, kp:double;
Xi, Yi, i:integer;
Xmin1, Ymin1, Xmax1, Ymax1, x, y, w, h:integer;
И далее идет функция строк эдак на 300 в которой все эти замечательные переменные активно используются, умножаются, делятся, только разве что не размножаются, примерно как-то вот так:
kx:=ww/pw;
ky:=wh/ph;
k:=min(kx, ky);
rp.Right:=round(pw*k);
rp.Bottom:=round(ph*k);
rp.Left:=(bmp.Width — rp.Right + 50) div 2;
rp.Top:=(bmp.Height — rp.Bottom + 50) div 2;
inc(rp.Right, rp.Left);
inc(rp.Bottom, rp.Top);
Комментариев на эти 300 строчек ровно 0.
И вот надо тебе в этом замечательном превью что-то поменять слегонца :)
Один раз pw с ww перепутаешь — все: кровь, кишки, access violation.
И вроде понимаешь что надо их переименовать, но ведь чтобы переименовать надо же сначала понять что они значили по авторской задумке.
А у Вас тут прямо тепличные условия какие-то описаны.
Подытожу — есть такая профессия — в чужом *****-коде ковыряться.vbif
19.11.2017 01:36300? Что так мало? Настоящие дельфисты пишут методы минимум по 5000 строк. В обработчике события. И названия переменных слишком понятные: Ymin1, Xmax1. Даёшь s,sss,s1, и конечно Button1! И на сладкое: использование {$include} и {$IF ()} для повторного использования кода.
debose
19.11.2017 03:06Обычно, если есть такой код, то скорее всего где-то выше есть ещё и глобальные переменные с такими же именами)
Jef239
19.11.2017 04:09Самое забавное что в стандарте (ИКД) GPS и ГЛОНАСС — ровно так. Назначения описаны, но названия переменных так похожи… И ведь не поменяешь — все уже привыкли именно к таким именам.
Whuthering
19.11.2017 13:08самый обычный Delphi-код, ничего не обычного :)
как и с PHP — низкий порог вхождения (особенно в те времена когда диск с пиратским Delphi и книга Фаронова лежала в каждом ларьке у метро), а потом студенты, инженеры, и прочие люди, ранее не занимавшиеся промышленным программирование доросли до больших проектов и продолжили делать там то же самое.
aknew
19.11.2017 13:53Я однажды встречал что-то подобное, но с одним важным отличием — комментарии таки были, все что когда-то было в методе и теперь не используется оставлялось в виде закомментированного кода ибо системы контроля версии не использовались. Иногда там было по 3-4 экрана закомментированного кода.
SerhiyRomanov
18.11.2017 23:34Вот почему когда я разбираю код на Python после индусов, то сильно радуюсь что так
if (!function_exists('CutFromTo')){ function CutFromTo($string, $from, $to){$fstart = stripos($string, $from); $tmp = substr($string,$fstart+strlen($from)); $flen = stripos($tmp, $to); return substr($tmp,0, $flen);}}
на нем писать нельзя.etho0
19.11.2017 02:49+1Можно:
CutFromTo = CutFromTo if locals().__contains__('CutFromTo') else lambda string, frm, to: substr(tmp,0,stripos(substr(string,stripos(string,frm)+strlen(frm)),to))fireSparrow
19.11.2017 02:53Эх, пока писал свой вариант, вы меня опередили…
Я тоже хотел с лямбдой сделать. Но в итоге решил, что это неспортивно, поэтому у меня всё-таки в две строчки.
if(not('cut_from_to' in dir())): def cut_from_to(string, from_, to): fstart = string.lower().find(from_.lower()) ; tmp = string[fstart + len(from_):]; flen = tmp.lower().find(to.lower()); return tmp[:flen]
JekaMas
19.11.2017 13:10Можно, и нельзя приватные члены класса, константы, много чего еще. Не надо мериться — у всех хватает кривых конструкций на уровне языка и возможностей стрелять себе в ногу.
vism
19.11.2017 02:05На самом деле в этом плагине был еще довольно не плохой индуский код.
Его писал уверенный миддл индус.
Я серьезно.
Все эти шутки про идусский код оказались правдой и там таакооое бывает)
Лучше один раз увидеть:)
Последний раз, очень крутой индо-программист с окладом 400$ в неделю (это овердофига для индусов) удалил проект с качественным кодом на Laravel, где все было разложено по полочкам и работало. Вместо него он начал писать с нуля где каждая страница представляла из себя отдельный ПХП файл.
Естественно все интеграторы оплаты, кредитные карты и все такое были удалены вместе с проектом.
Дак вот он пытался открывать сайт интегратора через фрэйм и слать им запросы обычными ajax, которые естественно не принимались.
А клиент у меня спрашивал, мол как так, в предыдущей версии все работает.
Вобщем смех и грех:)Babich_S
19.11.2017 13:09А вот у меня случай тоже интересный был с индусо-кодом на WordPress. Сайт парсил сам себя. Магазин на WooCommerce где пагинация в виде «бесконечного» скроллинга была организована крайне интересным способом. При обращении к определенному адресу, призванному на ajax запрос вернуть еще часть списка товаров, делался запрос к этому же сайту с помощью file_get_contents(), к странице с обычной пагинацией. Далее контент парсился с помощью библиотеки «Simple HTML DOM» c целью достать по селектору #content только ту часть страницы где товары. Ну и потом результат этих манипуляций возвращался.
vism
19.11.2017 13:14Самое странное, что они делают такими сложными и трудными способами, что даже не понятно как они до них додумались)
Вот неужели он не догадался ввести переменную и отправлять ее, но сделал парсер.
Ну вот как так?)vbif
19.11.2017 13:44Возможно, они, будучи не знакомы с основами, ищут решение, как им кажется, в наиболее логичном направлении. Как я выше привёл пример, использование {$include} для выноса повторяющегося кода в Delphi.
Deosis
20.11.2017 07:36Тут была статья про написание программы генетическими алгоритмами:
Комбинируем куски кода до тех пор, пока не получим приемлемое решение.
Возможно индусы пишут похожим образом.
Вышеупомянутый когда-то парсил сайт и сейчас использует свои наработки.
ProstoDesign
19.11.2017 13:15Таких случаев море, в сети есть даже уроки, который учат ровно тому же самому. Видел лично)
mrMaxSimka
19.11.2017 13:09Шутки про индусский код были весьма мной любимы, пока не достался мне в доработку проект после них. Файлы по шесть тысяч строк без комментариев осложнялись хаотичным именованием переменных — не более двух символов в имени. Вишенкой на торте было объявление функции в шаблоне страницы одного из самописных плагинов, вызываемой без всяких проверок и зависимостей в другом таком плагине. Думаю, у каждого, кто работал с индусским кодом, есть истории, как не надо писать.
chirkin
19.11.2017 23:41Я хотел было написать, может кто-то специально использовал обфускатор чтобы защитить платный плагин, но потом прочёл все комментарии...
vovasik
20.11.2017 14:15Самая большая проблема в том, что синтаксис
Спорно, синтаксис не запретит использовать eval, copy/paste programming и продавать свой говнокод.
Тут проблема в генетеке автора такого кода.
Akdmeh
XDebug? Нет, не слышали.
А так да, автоформатирование выручает, не раз проверял.
Slavik7 Автор
Теперь слышали =) Вообще php использую не настолько активно, чтобы разбираться с дебагерами под него, да еще и на удаленных сайтах, соответственно и код индусов до сих пор мне не попадался. Первый опыт.
weirded
"Важное" мнение мимокрокодила-питониста про xdebug: вчера впервые пытался его подружить с phpstorm, завелось, но evaluate expression обламывается на почти всём чуть сложнее арифметики :(
Возился с связкой xcode + iOS simulator + selenium2 + appium + behat, пытался получить что-то вроде ipython из которого смогу поэкспериментировать с интерактивным управлением мобильным safari из php.
krundetz
Может пригодится.
github.com/Litipk/Jupyter-PHP