Pull to refresh

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

Reading time 6 min
Views 17K
Сразу оговорю, что ничего инновационного в статье не предлагается. Я просто описываю один небольшой случай работы с чужим кодом. Опытные разработчики, пожалуй, улыбнутся, ибо наверняка с подобным сталкивались и сами, а может и еще хуже. Тех же, для кого это всё в новинку, просьба взять на заметку, как не надо оформлять свой код, уж особенно если речь идет о публичном плагине. В статье далее описывается работа с посторонним плагином для 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 всё это позволяет сделать и люди, к сожалению, пользуются данной свободой не по назначению. Я понимаю, что разработчики, сохраняя исходный код в базе движка, за который они берут деньги, таким образом попытались сделать его менее доступным, но это крайне неверный подход. Самый банальный аргумент: исходя из моего опыта этот подход не сработал — я без особых сложностей всё равно добрался до платных исходников, но из-за зря потраченного мной времени моё желание распространять этот платный код бесплатно только усилилось, а так оно было нулевым. Даже появлялись мысли вообще написать свой плагин с точно такой же функциональностью, чтобы намеренно составить конкуренцию разработчикам. В общем, если скрыть исходники всё равно не получается, то сделайте их читабельными для других разработчиков. Это точно не скажется отрицательно на вашей доходности, но зато очень хорошо скажется на репутации и только повысит ваши возможности в перспективе заработать больше.
Tags:
Hubs:
+11
Comments 30
Comments Comments 30

Articles