d_x умотал в Лас-Вегас просаживать годовой бюджет маленькой африканской страны, а значит, за неимением лучших тем (точнее они есть, но я лентяй), пришло время для очередного ревью. Кстати, товарищи, неужели никто не пишет на C/C++/Asm? В запросах сплошной Perl.
Начнем с кода скрипта, который прислал Alexey Shrub. Скрипт представляет из себя сокращатель ссылок и большей частью базируется на готовых модулях. Исходный код полностью. Подход нормальный, но ревьюить толком нечего ввиду отсутствия опыта использования данных модулей. Поэтому упомяну только несколько доступных мне мелочей:
|
1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 |
my $index = q{ <!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.0 Strict//EN" "http://www.w3.org/TR/xhtml1/DTD/xhtml1-strict.dtd"> <html xmlns="http://www.w3.org/1999/xhtml" xml:lang="en" lang="en"> <head> <meta http-equiv="Content-Type" content="text/html; charset=UTF-8" /> <title>URL shotener</title> </head> <body> <form action="/"> <fieldset> <label for="url">Enter URL:</label> <input type="text" name="url" id="url" /> <input type="submit" value="Get short link" /> </fieldset> </form> </body> </html> }; |
Мне кажется, стоило бы использовать существующий heredoc-синтаксис, который позволяет избежать потенциальных проблем при модификации подобных строковых переменных, или можно было этот фрагмент вынести в отдельный файл.
|
1 2 3 4 5 6 7 8 9 |
my $respond = shift; my $url = $query->{url}; my $url_hash = Digest::Tiger::hexhash( $url ); unless ( $url ) { my $w = $respond->([200, ['Content-Type' => 'text/html; charset=utf-8']]); $w->write( $index ); undef $w; return; } |
Тут мне не особо понятна логика. Окей, мы получили переменную $url, потом попытались получить хэш от неё (ну, в php вроде бы md5 от пустой строки успешно вычисляется, так что тут наверное тоже ошибки не возникнет), а далее решили провести её валидацию. Может, стоило сначала провести валидацию, а потом уже делать всё остальное? Также здесь и далее по коду используется вызов undef $w;. Тоже непонятен смысл, переменная локальная, при выходе из зоны видимости должна уничтожаться, хотя, может, это хитрое требование используемого модуля.
И последний фрагмент из этого скрипта:
|
1 2 3 4 5 6 7 8 |
my $respond = shift; my $key = ( $req->path =~ m!/(.+)$! )[0]; $redis->get( $key, { on_done => sub { my ( $url ) = @_; my $w = $respond->([301, ['Location' => $url]]); $w->write( "Go to $url" ); undef $w; } |
Меня интересует вторая строчка этого фрагмента. Я бы написал нечто вроде
|
1 |
my ($key) = ($req->path =~ /(.+)$/); |
Допустим, m использован ради того, чтобы в качестве обрамляющих символов поставить восклицательный знак (но зачем?), но целесообразность использования в данном случае [0] для меня так и осталась загадкой.
Следующий скрипт нам прислал некто Abironka. Полный листинг можно посмотреть тут. Это простая отправлялка вывода команды или тела файла на pastebin.
В начале скрипта мы наблюдаем краткое описание и пример использования. Это можно было бы переоформить в соответствии со стандартом, описанным здесь. По сути неважно, но приятно, когда соблюдаются общепринятые стандарты.
|
1 2 3 4 5 6 |
our $VERSION = '0.9'; ... if ($options{h}) { usage($VERSION); exit; } |
Объявили глобальную переменную и передали её аргументом в функцию. Почему бы не обращаться к ней напрямую из функции usage? Разве что планировалось включить несколько вариантов мануалов для разных версий, но, думается, вряд ли.
|
1 2 3 |
if ( $options{i} && !$options{s} ) { simple_sintax_detect(\%options); } |
Не ради кода, но правильность написания слов стоит проверять хотя бы с помощью Google. Существуют syntax и sin tax. Думаю, речь шла о syntax.
|
1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 |
sub simple_sintax_detect { my $options = shift; my $f_ext = $1 if $options->{i} =~ /^[\wа-я-]+\.([a-z]{1,4})$/i; if ($f_ext =~ /^(?:txt|text|)$/i) { $options->{s} = 'text'; } elsif ($f_ext =~ /^(?:pl|cgi)$/i) { $options->{s} = 'perl'; } elsif ($f_ext =~ /^sh$/i) { $options->{s} = 'bash'; } elsif ($f_ext =~ /^php$/i) { $options->{s} = 'php'; } } |
Вместо букв "а", "я" стоило бы использовать их hex представление, чтобы избежать возможных проблем с кодировкой при правке скрипта сторонним лицом или на специфических системах. В [\wа-я-] последнюю тирешечку стоит экранировать, а список соответствий расширений файлов языкам лучше поместить в отдельный ассоциативный массив где-нибудь в начале и таким образом избавиться от кучи elsif.
|
1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 |
sub read_input_file { my $options = shift; if ( !$options->{i} ) { while (<>) { $msg_body .= $_; } } elsif ( $options->{i} ) { open(INFILE, '<', $options->{i}) or die "Can't open $options->{i}: $!\n"; while (<INFILE>) { $msg_body .= $_; } close(INFILE); } return $msg_body; } |
Тут мы виртуозно оперируем с глобальной переменной $msg_body, а потом её же и возвращаем зачем-то. Наверное, стоило передать её внутрь, либо объявить как локальную внутри, либо не передавать, но и не писать return $msg_body. Кстати, второй вариант чтения файла можно было реализовать более идиоматически (при условии, что мы используем вариант с return ...). Примерно так:
|
1 2 3 4 5 6 |
sub read_file { local @ARGV = shift; local $/ = wantarray ? $/ : undef; <>; } |
Естественно, с адаптацией под данный случай. И последний фрагмент:
|
1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 |
sub ua_init { my $cookies=HTTP::Cookies->new('file'=>'./cookies.lwp','autosave'=>0); my $ua = LWP::UserAgent->new( 'cookie_jar' => $cookies, 'requests_redirectable' => ['GET', 'POST'], ); $ua->default_header( 'Accept' => 'text/html, application/xml;q=0.9, application/xhtml+xml, */*;q=0.1', 'Accept-Charset' => 'utf-8; *;q=0.1', 'Accept-Language' => 'ru,en-us;q=0.7,en;q=0.3', 'Accept-Encoding' => 'deflate, gzip, x-gzip, *;q=0', ); return $ua; } |
Создаем объект HTTP::Cookies, хорошо, но файл для хранения указывать не имеет смысла, на мой взгляд. Пусть в памяти хранит. Далее мы передаем созданный объект в качестве значения cookie_jar для LWP::UserAgent и в дальнейшем переменную $cookies не используем. Тогда можно было бы ограничиться следующим кодом:
|
1 2 3 4 5 |
my $ua = LWP::UserAgent->new( 'cookie_jar' => new HTTP::Cookies, 'requests_redirectable' => ['GET', 'POST'], ); |
Последний на сегодня скрипт, а точнее модуль, присланный e.s. С листингом можно ознакомиться здесь. Этот модуль предназначен для получения доступной информации по сайту из Яндекс.Каталога (название, описание, тИЦ, рубрики...). Кода в нем мало, документация оформлена надлежащим образом, поэтому рассмотрим единственную более-менее крупную функцию - yaca_lookup, которая реализует основной функционал данного модуля. Рассматривать будет фрагментами.
|
1 2 3 4 5 6 7 |
$address =~ s|.*?://||; # loose scheme $address =~ s|.*?(:.*?)?@||; # loose authentication $address =~ s|(\w):\d+|$1|; # loose port $address =~ s|\?.*||; # loose query $address =~ s|/$||; # loose trailing slash my $contents = get( 'http://search.yaca.yandex.ru/yca/cy/ch/' . $address ); |
Мы получили на вход URL и хотим получить информацию о нём из Яндекс.Каталога. В данном фрагменте адрес приводится к некому каноничному для Яндекса виду. Я бы посоветовал не играться с регулярными выражениями в данном случае, а воспользоваться модулем URI::Split, тем более он входит в стандартный комплект модулей (по крайней мере, в случае с ActivePerl).
|
1 2 3 4 5 6 7 |
if( $contents =~ /<p class="b-cy_error-cy">/ ) { # "ресурс не описан в Яндекс.Каталоге" # It's not in the catalog, but tIC is always displayed. # Ex.: Индекс цитирования (тИЦ) ресурса — 10 ( $self->{_tic} ) = $contents =~ /<p class="b-cy_error-cy">.*?\s(\d+)/s; $self->{_tic} = 0 unless defined $self->{_tic}; } |
Последние несколько строк можно было записать проще, например, следующим образом:
|
1 |
$self->{_tic} = $contents =~ /<p class="b-cy_error-cy">.*?\s(\d+)/s ? $1 : 0; |
Далее
|
1 2 3 4 5 |
my( $entry ) = $contents =~ qr{(<tr>\s*<td><img.*/arr-hilite\.gif".*?</tr>)}s; ( $self->{_orderNum}, $self->{_uri}, $self->{_shortDescr}, undef, $self->{_longDescr}, $self->{_tic} ) = # $1 $2 $3 $4 $5 $entry =~ qr{<td>(\d+)\.\s*</td>.*<a href="(.*?)".*?>(.*)</a>(<div>(.*)</div>.*?)?(\d+)<}s; |
Исходя из undef в списке переменных, делаю предположение, что автор не в курсе non-capturing groupings, которым можно воспользоваться для отсечения захвата ненужных результатов.
|
1 2 3 4 5 6 7 |
if( $path ) { $path =~ s{</?a.*?>|</?h1>|\n}{}gs; # remove A, H1 tags and newline $path =~ s|\x{0420}\x{0443}\x{0431}\x{0440}\x{0438}\x{043A}\x{0438} / ||; # removed "Рубрики" - it always starts with it # http://www.rishida.net/tools/conversion/ push( @{$self->{_categories}}, $path.' / '.$rubric ) if $entry; } |
Регулярные выражения для удаления HTML-разметки? А если новые возможные элементы добавят? Можно было воспользоваться HTML::Strip. Оставшийся код особых нареканий не вызвал.
На этом, пожалуй, завершу обзор. Если ваш код не попал в очередное ревью - не волнуйтесь, всё, что приходит, рано или поздно будет разобрано.
А мой код разбирать планируется?
Там на masm - e MAC-вьювер?
))
d_x в Вегасе, когда вернется, тогда и будет разбор
А с кем он там? :3
С друзьями
Парни, разобрали бы лучше толковый код. Правда Си или Асм, ну а это что - ерунда. Написали бы пакер, со степенью сжатия как у Upack, и размером таким-же - да разобрали бы - вот это было бы интересно. Фундаментальное что-нить. Пакер с высокой степенью сжатия (выше UPX), при этом имеющий небольшой размер (как FiPP 24Кб с GUI) да еще и с антиотладочными трюками! Вот это было бы действительно потрясно. А то - сокращатель ссылок. Каими, ты же очень достойный реверсер, изложи свои мысли по качеству и степени сжатия РЕ-формата. Ведь достойных пакеров, которые сжимают лучше де-фактового UPX можно на пальцах посчитать.. И все так же юзают LZMA, в чем их фишка??
Что присылают, то и разбирается.
Пакер и так присутствует http://kaimi.io/2011/12/%d1%83%d0%bf%d0%b0%d0%ba%d0%be%d0%b2%d1%89%d0%b8%d0%ba-pe-%d1%84%d0%b0%d0%b9%d0%bb%d0%be%d0%b2-exe-dll/
Хочется особых степеней сжатия - исходный код открыт.
LZMA используют, т.к. бесплатный открытый формат, да и реализаций полно.
Хочется что-то там для работы с PE - d_x приедет и выложит здоровенную либу классов для этих целей.
А реверсер из меня никакой, все достойные без проблем снимают темиду, вмпротект, армадилло, энигму и т.п., а также обладают кучей наработок по всему этому.
Так вот да, хочется чего-нить достойного для РЕ, заинтриговал - буду ждать))
Пакер я видел - степень сжатия мала и ехешники бьет очень часто..(
По поводу LZMA - я не к тому, и так понятно, что опенсорс. Просто к примеру Upack сжимает в разы лучше, чем UPX, хотя оба используют LZMA, вот и не пойму, откуда такая разница..(
На счет снятия вмпрота и фимки - а зачем?? Проще де проинлайнить и все. Тем более вмпрот инлайнится не так сложно, если что - обратись к Максимусу (ну ты понял) он тебе расскажет, т.к. он действительно на этом собаку съел.
Подскажи, как в d_x'овском пакере степень сжатия поднять? Заранее спасибо.
Не знаю что он там бьет. Всякие файрфоксы, калькуляторы и т.п. без проблем пакуется и работает.
Так возьми и разбери реализацию методов сжатия в upack или попроси у автора исходный код.
Затем, что весь нужный код завернут в виртуальную машину, что абсолютно не радует.
Написать базонезависимый распаковщик под предполагаемый алгоритм и написать методы упаковки, а потом ими заменить соответствующие моменты в пакере.