znaeff Jun 8 2016 at 08:08

1С: Битрикс, защита произвольных форм от спама

2 min

11K

CleanTalk Anti-Spam corporate blog1С-Bitrix*Information Security*

-6

Comments 12

mmjurov Jun 8 2016 at 08:52

Реклама, Битрикс, говнокод… все что нужно для отличного поста на хабр!

MetaDone Jun 8 2016 at 10:01

последнее время очень быстро расхватывают решения на другие ресурсы

Alexufo Jun 8 2016 at 10:52

Какое прекрасное место для самолюбования :-) Комменты завораживают Постучали по клаве — получился битрикс — ну постучи так до международного рынка, постучи))) Давно ясно что прибыль им не код приносит)) Можно ли за это их винить? Да, так как это вносит общий хаос кроме комфорта конкретного слоя лиц. Но бесконечные вливания о его говености напоминают уже удары палки по железобетонной конструкции. Мир меня обидел битриксом…
Битрикс — это взрыв шаблона о том, как можно делать бизнес)

MetaDone Jun 8 2016 at 11:18

«бесконечные вливания о его говености» — они ж не просто так появляются. Дада, всем ясно что инструмент призван решать задачи, что бизнесу без разницы какой код под капотом.

Битрикс — это взрыв шаблона о том, как можно делать бизнес)

Заплатили — получили решение, все просто. Просто, если решение стандартное. Но стандартное решение — вещь скучная, неоригинальная и редкая. Поэтому нужно дорабатывать. А с учетом корявостей внутренностей продукта становится очевидна причина «бесконечных вливания о его говености». Так что это не остановится до тех пор, пока его не перепишут по человечески.

Alexufo Jun 8 2016 at 10:49

Это блог компании. Могут пиарить что хотят. Сleantalk работает лично у меня и на phpbb и wordpress и скажу довольно не плохо. По поводу говнокода, помоему ребята пишут в стиле той системы для которой модуль :-)

Akuma Jun 8 2016 at 10:37

> Всё, что нужно, это поместить код вызова метода CleantalkAntispam::CheckAllBefore с нужными параметрами
И ниже лапша непонятного кода :)

zenn Jun 8 2016 at 11:13

Давайте попробуем вразумить автора этого «кода» и наставить его на путь истинный, если конечно это возможно…
1. Обратите внимание на рекомендации по стилю форматирования кода — PSR. В вашем случае, вопросы вызывают именования переменных и функций через-раз в under_scope и camelCase. В PSR описано что когда и как нужно применять. Обратите внимание на global, но судя по документации битрикса от этого не избавиться никак…
2. Я не нашел пожалуй ни единого оправдания для использования такого лапше-кода:

  $aParams = array();
  $aParams['type'] = 'comment';
  $aParams['sender_email'] = isset($arrVALUES['email']) ? $arrVALUES['email'] : '';
  ...
  $aResult = CleantalkAntispam::CheckAllBefore($aParams, FALSE);

Может стоило оформить это как-то так:

$result = CleantalkAntispam::CheckAllBefore([
    'type' => 'comment',
    'sender_email' => (string)$arrVALUES['title'],
    ...
], false);

3. К этому участку кода претензий не меньше:

 if (isset($aResult) && is_array($aResult)) {
   if ($aResult['errno'] == 0) {
    if ($aResult['allow'] == 1) {
        // valid response
        return;
    } else {
     if (preg_match('//u', $aResult['ct_result_comment'])) {
      // generate error
     } else {
      // generate error
     }
     // invalid response
     $APPLICATION->ThrowException($err_str);
     return false;
    }
   }
  }
 }

Структурно он выглядит очень дико и иррационально. По пунктам:

Много излишней вложенности. Мне честно не ясно что делает код в preg_replace и с чем, это видимо не известно и самому автору, пожалуй остался только бог… но многие if-ы можно запросто обрезать или вложить в 1 контейнер
Почему функция возвращает 2 разных по типу результата? В одном случае это bool — return false; а во втором это будет return; что даст null
Почему используется не строгая типизация в условиях if? (== вместо необходимого ===)? Модуль — ваш, обертка к модулю — ваша. Поясните, откуда взялось это: $aResult['allow'] == 1? Почему ваш же модуль не генерирует bool, а если и генерирует bool почему вы сравниваете его с неявной типизацией == с Integer ?

На этом я пожалуй остановлюсь, у меня нет желания изучать сам модуль и какие аргументы и каким образом он передает в onBeforeЧетатам() этого хука форм, но даже эта реализация говорит о том, что в коде самого модуля происходит что то ужасное.

П.с. — друзьям и всем, кому интересен «битрикс» и его «инновации» в мир веб-программирования (сарказм): вот так истинные «джедаи» генерируют ошибки:

$APPLICATION->ThrowException($err_str);

а вы там все используете свои try-catch блоки для обработки и throw new для генерации…

Suntechnic Jun 8 2016 at 13:16

Я работаю с битрикс, но я ничерта не понял. Я не хочу вчитываться в код этой… м… функции. Тем более что выше уже написали почему.
Только несколько вопросов:

Почему:
> AddEventHandler('form', 'onBeforeResultAdd', 'my_onBeforeResultAdd');
А не AddEventHandler('form', 'onBeforeResultAdd', ['my_Class', 'my_onBeforeResultAdd']);?

> CModule::IncludeModule('cleantalk.antispam');
Почему вы подключаете модуль на каждом хите? На кой лят это нужно? Почему не подключать модуль прямо в вашей функции?

И здесь не любя битрикс. А знаете почему? Вот из-за таких «статей».

Вобщем не надо больше так.

nikizan Jun 8 2016 at 13:33

В случае использования капчи предельно ясен принцип оценки — ручной ввод корректной информации является подтверждением что работает человек. В Вашем случае неясен принцип и критерии оценки, что ставит под сомнение рациональность использования «кота в мешке». С одной стороны разработчик сайта упрощает для посетителей заполнение требуемой информации на формах сайта, с другой стороны разработчик сайта напротив усложняет для посетителей заполнение — в частности, из-за того, что введенную информацию по неизвестной причине антиспам не принимает (допустим) и никто не может понять почему, и что делать пользователю для разрешения ситуации — он просто покинет сайт, что приведет к оттоку посещаемости, что уже является минусом использования подобного закрытого решения.

зы: Это из разряда — «мы это уже проходили»…

Suntechnic Jun 8 2016 at 14:47

Да, можно выводить описание проблем, чтобы роботу было сразу понятно под что подстраиваться )))

shagimuratov Jun 8 2016 at 18:09

Описание проблемы сервис выводит, вот пример,

*** Forbidden. Enable JavaScript. Anti-spam service cleantalk.org. ***

Либо в настройках Панели управления можете указать свое собственное сообщение для отвергнутых форм,
https://cleantalk.org/my/service?action=edit

По алгоритмам есть пояснения здесь,
https://habrahabr.ru/company/cleantalk/blog/282586/
https://habrahabr.ru/company/cleantalk/blog/283300/

znaeff Jun 8 2016 at 15:22

Небольшое пояснение: был дан лишь пример использования нашего модуля для защиты любой формы. Не более.

Для реального применения нужны ограничения вызова, проверки и т.д. Но это всё зависит от самой формы, от модуля для формы и поэтому здесь не рассмотрено.