Опыт выявления одного бага или как не надо оформлять свой код

Сразу оговорю, что ничего инновационного в статье не предлагается. Я просто описываю один небольшой случай работы с чужим кодом. Опытные разработчики, пожалуй, улыбнутся, ибо наверняка с подобным сталкивались и сами, а может и еще хуже. Тех же, для кого это всё в новинку, просьба взять на заметку, как не надо оформлять свой код, уж особенно если речь идет о публичном плагине. В статье далее описывается работа с посторонним плагином для wordpress. После моих “приключений” мне очень не хочется никаким образом упоминать название плагина, поэтому в кусках исходного кода я подменил название переменных, функций, чтобы максимально исключить возможность отсылки к плагину.

Часто вижу, как люди плохо отзываются о 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:

image

Вот я пришел к необъявленому классу. Первым делом скачал с сервера все остальные исходники движка и плагинов и поиском по всем файлам попытался найти определение класса. Был достаточно сильно удивлен, когда ничего не нашлось. Другими словами постинг картинки происходит в классе, которого нет в исходниках.

Спросил у знакомого, что же это за плагин, в результате чего выяснилось, что плагин публичный, конкретно данная модификация (постинг в 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 всё это позволяет сделать и люди, к сожалению, пользуются данной свободой не по назначению. Я понимаю, что разработчики, сохраняя исходный код в базе движка, за который они берут деньги, таким образом попытались сделать его менее доступным, но это крайне неверный подход. Самый банальный аргумент: исходя из моего опыта этот подход не сработал — я без особых сложностей всё равно добрался до платных исходников, но из-за зря потраченного мной времени моё желание распространять этот платный код бесплатно только усилилось, а так оно было нулевым. Даже появлялись мысли вообще написать свой плагин с точно такой же функциональностью, чтобы намеренно составить конкуренцию разработчикам. В общем, если скрыть исходники всё равно не получается, то сделайте их читабельными для других разработчиков. Это точно не скажется отрицательно на вашей доходности, но зато очень хорошо скажется на репутации и только повысит ваши возможности в перспективе заработать больше.
Поделиться публикацией
Похожие публикации
AdBlock похитил этот баннер, но баннеры не зубы — отрастут

Подробнее
Реклама
Комментарии 29
  • +3
    XDebug? Нет, не слышали.
    А так да, автоформатирование выручает, не раз проверял.
    • 0
      Теперь слышали =) Вообще php использую не настолько активно, чтобы разбираться с дебагерами под него, да еще и на удаленных сайтах, соответственно и код индусов до сих пор мне не попадался. Первый опыт.
      • 0

        "Важное" мнение мимокрокодила-питониста про xdebug: вчера впервые пытался его подружить с phpstorm, завелось, но evaluate expression обламывается на почти всём чуть сложнее арифметики :(


        Возился с связкой xcode + iOS simulator + selenium2 + appium + behat, пытался получить что-то вроде ipython из которого смогу поэкспериментировать с интерактивным управлением мобильным safari из php.

    • 0
      Поэтому php-разработчиков и не любят: нет жестких рамок(как в python например касаемо форматирования), легаси-код, скрытие ошибок(код запустился без класса?), evalы, через которые легко можно вскрыть сайт. Но это ложь, их не любят по другой причине. Сам пишу на всем, в том числе на php. Не от языка программирования отношение зависит, а от людей. Просто php проще для входа, поэтому новичков в php больше, а значит больше легаси и косяков. Python на винду поставить сложнее, чем denwer. И слава байт-коду.
    • +5
      А как Вам вот такое (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.
      И вроде понимаешь что надо их переименовать, но ведь чтобы переименовать надо же сначала понять что они значили по авторской задумке.

      А у Вас тут прямо тепличные условия какие-то описаны.
      Подытожу — есть такая профессия — в чужом *****-коде ковыряться.
      • –1
        300? Что так мало? Настоящие дельфисты пишут методы минимум по 5000 строк. В обработчике события. И названия переменных слишком понятные: Ymin1, Xmax1. Даёшь s,sss,s1, и конечно Button1! И на сладкое: использование {$include} и {$IF ()} для повторного использования кода.
        • +3

          Обычно, если есть такой код, то скорее всего где-то выше есть ещё и глобальные переменные с такими же именами)

          • 0
            Самое забавное что в стандарте (ИКД) GPS и ГЛОНАСС — ровно так. Назначения описаны, но названия переменных так похожи… И ведь не поменяешь — все уже привыкли именно к таким именам.
            • +1
              самый обычный Delphi-код, ничего не обычного :)
              как и с PHP — низкий порог вхождения (особенно в те времена когда диск с пиратским Delphi и книга Фаронова лежала в каждом ларьке у метро), а потом студенты, инженеры, и прочие люди, ранее не занимавшиеся промышленным программирование доросли до больших проектов и продолжили делать там то же самое.
              • +1
                Я однажды встречал что-то подобное, но с одним важным отличием — комментарии таки были, все что когда-то было в методе и теперь не используется оставлялось в виде закомментированного кода ибо системы контроля версии не использовались. Иногда там было по 3-4 экрана закомментированного кода.
              • –2

                Вот почему когда я разбираю код на 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);}}


                на нем писать нельзя.
                • +5
                  Можно:
                  CutFromTo = CutFromTo if locals().__contains__('CutFromTo') else lambda string, frm, to: substr(tmp,0,stripos(substr(string,stripos(string,frm)+strlen(frm)),to))
                  • 0
                    Эх, пока писал свой вариант, вы меня опередили…
                    Я тоже хотел с лямбдой сделать. Но в итоге решил, что это неспортивно, поэтому у меня всё-таки в две строчки.

                    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]
                  • 0
                    Можно же в лямбду завернуть, будет примерно так же.
                    • 0

                      Можно, и нельзя приватные члены класса, константы, много чего еще. Не надо мериться — у всех хватает кривых конструкций на уровне языка и возможностей стрелять себе в ногу.

                    • +2
                      На самом деле в этом плагине был еще довольно не плохой индуский код.
                      Его писал уверенный миддл индус.
                      Я серьезно.

                      Все эти шутки про идусский код оказались правдой и там таакооое бывает)
                      Лучше один раз увидеть:)

                      Последний раз, очень крутой индо-программист с окладом 400$ в неделю (это овердофига для индусов) удалил проект с качественным кодом на Laravel, где все было разложено по полочкам и работало. Вместо него он начал писать с нуля где каждая страница представляла из себя отдельный ПХП файл.
                      Естественно все интеграторы оплаты, кредитные карты и все такое были удалены вместе с проектом.
                      Дак вот он пытался открывать сайт интегратора через фрэйм и слать им запросы обычными ajax, которые естественно не принимались.
                      А клиент у меня спрашивал, мол как так, в предыдущей версии все работает.

                      Вобщем смех и грех:)
                      • 0
                        А вот у меня случай тоже интересный был с индусо-кодом на WordPress. Сайт парсил сам себя. Магазин на WooCommerce где пагинация в виде «бесконечного» скроллинга была организована крайне интересным способом. При обращении к определенному адресу, призванному на ajax запрос вернуть еще часть списка товаров, делался запрос к этому же сайту с помощью file_get_contents(), к странице с обычной пагинацией. Далее контент парсился с помощью библиотеки «Simple HTML DOM» c целью достать по селектору #content только ту часть страницы где товары. Ну и потом результат этих манипуляций возвращался.
                        • 0
                          Самое странное, что они делают такими сложными и трудными способами, что даже не понятно как они до них додумались)
                          Вот неужели он не догадался ввести переменную и отправлять ее, но сделал парсер.
                          Ну вот как так?)
                          • 0
                            Возможно, они, будучи не знакомы с основами, ищут решение, как им кажется, в наиболее логичном направлении. Как я выше привёл пример, использование {$include} для выноса повторяющегося кода в Delphi.
                            • 0

                              Тут была статья про написание программы генетическими алгоритмами:
                              Комбинируем куски кода до тех пор, пока не получим приемлемое решение.
                              Возможно индусы пишут похожим образом.
                              Вышеупомянутый когда-то парсил сайт и сейчас использует свои наработки.

                            • +1
                              Таких случаев море, в сети есть даже уроки, который учат ровно тому же самому. Видел лично)
                          • +2

                            Шутки про индусский код были весьма мной любимы, пока не достался мне в доработку проект после них. Файлы по шесть тысяч строк без комментариев осложнялись хаотичным именованием переменных — не более двух символов в имени. Вишенкой на торте было объявление функции в шаблоне страницы одного из самописных плагинов, вызываемой без всяких проверок и зависимостей в другом таком плагине. Думаю, у каждого, кто работал с индусским кодом, есть истории, как не надо писать.

                            • 0
                              круто. я такое делал сотни раз.
                              • +1

                                Я хотел было написать, может кто-то специально использовал обфускатор чтобы защитить платный плагин, но потом прочёл все комментарии...

                                • 0
                                  Самая большая проблема в том, что синтаксис


                                  Спорно, синтаксис не запретит использовать eval, copy/paste programming и продавать свой говнокод.

                                  Тут проблема в генетеке автора такого кода.

                                  Только полноправные пользователи могут оставлять комментарии. Войдите, пожалуйста.