1. PHP / Говнокод #23402

    +2

    1. 1
    2. 2
    $DB->Query("UPDATE b_search_content SET TITLE ='".$name."' WHERE URL='".$URL."' AND PARAM1='USER'" );
    $DB->Query("UPDATE b_search_content SET TITLE = CONCAT(TITLE,' тел.' '". $phone ." ' '  ' '". $email."' ) WHERE URL='".$URL."' AND PARAM1='USER'" );

    Перед одним сотрудником встала задача изменить содержимое поля индексной таблицы в Bitrix...

    Запостил: omar, 12 Октября 2017

    Комментарии (49) RSS

    • В $URL не может быть кавычек, так что всё ОК
      Ответить
      • Я ж JSом уже проверил, зачем второй раз то же самое проверять?
        Ответить
        • Я же написал в подсказке для юзера, что кавычки недопустимы.
          Ответить
          • Если что, потом посмотрим по логам, кто использовал кавычки, и забаним.
            Ответить
            • Оплатят годовую техподдержку - тогда и будем логи смотреть. А так у нашей студии и без этого работы хватает. Надо до завтра еще 20 шаблонов на джумлу натянуть.
              Ответить
      • Суть не в $URL, а в двух запросах вместо одного.
        Ответить
        • Т. е. с $URL всё ок?
          Ответить
          • По данному коду не видно. Может там майэскюэль_эскапе выше.
            Ответить
            • Так ведь все равно говно! Должно быть мойэскьюэльи_риал_эскейп
              Ответить
              • mysqli_real_chisto_concretno_vnature_escape_blya_budu
                Ответить
                • mysqli_besplatny_escape_bez_registracii_ i_sms_online_realno_rabotayet_100_procen tov($query)

                  Учитывая, что в ПХП половина стандартных функций deprecated, примерно так и нужно искать актуальные.
                  Ответить
          • Там просто из URL вырезаются слова DELETE, UPDATE и Insert, так что код безопасен
            Ответить
            • DROP
              Ответить
              • На DROP может не оказаться прав у юзера, от имени которого CMS коннектится к базе.
                Ответить
                • Да под рутом она коннектится. Ты думаешь автор потрудился создать несколько юзеров?
                  Ответить
                  • Автор – нет. Админ питушиного хостинга – да.
                    Ответить
                    • Юзер, которого выдал хостинг, все права на свою БД имеет. Так что один хуй почти рут.
                      Ответить
                      • Совсем не факт что у него есть права на DROP всей базы
                        он может иметь права на удаление таблиц

                        Ответить
                • TRUNCATE
                  Ответить
                  • ой, ну мое решение не идеальное конечно. Можно подумать в твоей программе не бывает багов
                    Ответить
            • Ясно. Будем использовать REPLACE.
              Ответить
            • > так что код безопасен
              Емнип, кот "безопасен" в этом плане совсем по другой причине - пыхомускуль по дефолту не умеет 2 запроса в одной строке.
              Ответить
          • Там этот URL не приходит из вне, а составляется вручную из имени пользователя, который в свою очередь берётся из базы. В том скрипте у человека вообще полный треш, начиная от именования переменных, заканчивая неправильным пониманием приниципов работы встроенных функций Bitrix.
            Ответить
            • > неправильным пониманием приниципов работы встроенных функций Bitrix
              Не надо понимать эти функции, и не надо их использовать. Все проекты на Bitrix надо саботировать, при первой же правке коммитить им бекдор и ломать продакшен, каждый день, пусть бекапятся. Когда им надоест -- предложить перевести на ларавафел или симфони или чо там есть.
              Ответить
            • --почему в вашей системе нельзя использовать кавычку в имени? Меня зовут О'Конелл
              --Понимаете, мы потом собираем из имени URL, и посылаем его в базу, и там может случиться SQL инъекция. Может быть попробуете другое имя?
              Ответить
    • Один/два запроса, возможные проблемы с $URL это конечно да. Но вот то, что в системе имеющей АПИ (и для данного случая в том числе, хотя возможно тут изначально не корректным путем задача решается) идет работа с БД напрямую ни кого не смутило?
      Ответить
      • > работа с БД напрямую ни кого не смутило
        В это настолько распространенная практика, особенно в пыхомирке, что способна смутить только совсем зелёного джуна, прочитавшего "Совершенный код" и впервые узревшего реальный коммерческий говнокод.
        К тому же, не все тут битриксмакаки, что бы знать, что в этом вашем битриксе есть, а чего нет.
        Ответить
        • Т.е. в других CMS и фреймоврках это нормальная практика?
          Обсуждать говнокод или нет в отрыве от контекста - вообще глупое занятие. Ну давайте взглянем на этот код: параметры могли быть обработаны до этих двух строк. Первая строка, да просто в запарке удалить забыл (ведь вторая просто перекрывает действие первой). Разве такое достойно рассмотрения на гвонокоде? Да такого добра в любом проекте можно найти. А может там какой то хитрый триггер в БДесть, и нужно именно эти два запроса в такой последовательности.... (тогда это уже ближе к теме, но нужен и триггер)

          Вообще так сложилось это год занимаюсь вытаскиванием проектов на Битрикс из Ж. Там где работали чайники это понятно и не интересно. Но вот море случаев, где код написан явно не плохим PHPшником. (просто он ввиду лени или
          высокой самооценки) не разобрался с битриксом.. В итоге: если поместить его код здесь: все скажут все правильно сделано, даже красиво... Но!. С учетом того, что это битрикс, все же код, при всей "красоте", является быдлокодом.

          И тут собственно все так же для любого языка. И красивый код написанный в стиле (утрировано) MFC , будет в Qt проекте быдлокодом.

          ИМХО ;)
          Ответить
        • Ну и для пояснения, что здесь не так:
          Данная таблица может быть пересоздана кликом админа (или того кому, таковые права предоставлены) по кнопке в админке. В принципе при обновлении Битрикс, может выскочить плашка "необходимо пересоздать индекс". (А на некоторых проектах встречал вообще на крон сажали полную переиндексацию) Чтобы эти изменения не похерились они должны учитываться при переиндексации. Следовательно, этот код должен быть либо в системных скриптах (что грубая ошибка при работе с любой системой), либо в обработчике соответствующего события (единственно правильное решение). Фишка в том, что в обработчик поступают все необходимые поля, и в базе ни чего править не надо.
          Ответить
      • Че не так в работе с бд напрямую? У нас в плюсах никаких ормов нет, руками запросы пишем. И вообще все эти ваши ормы для хайлоада не годятся, хороший программист должен контролировать каждый запрос.
        Ответить
        • В каких плюсах? В С++ что ли? Огорчу, но просто вы не знаете о их существовании ;)
          Хороший программист не должен приводит систему в состояние, когда нарушается стандартное поведение системы. Т.е. там надо либо писать документацию, запрещать обновления, и закрывать часть штатного функционала. Иначе, при смене программиста. Попробуйте как то покататься на велосипеде у которого при повороте руля в одну сторону, колесо поворачивает в другую. Со своими таблицами/сущностями можно делать все что угодно. С системными, а точнее сущностями других разработчиков, это уже само по себе быдлоподход.

          Теперь к хайлоаду. Этот данный конкретный случай здесь каким боком? Проект с миллиардами пользователей, по которым опупеть как нужен поиск? Точнее даже так. Это должен быть проект на который постоянно скриптами заливают новых пользователей пачками по несколько сотен тысяч.
          Ответить
        • Ты не поверишь, но 80% времени занимает 20% кода.
          Нужно оптимайзить эти 20%, а на остальное похуй.

          Именно потому всё пишут на ЯПах высокого уровня, а затем оптимайзят ботлнонеки.

          Тоже самое и с ORM: в джанге, например, принято для всего юзать ORM, а если конкретное место тормозит то его просто переписывают на raw SQL.
          Ответить
          • я кавайная ботлнека (^• ω •^)
            Ответить
          • Мы все же обсуждаем не сферического коня в вакууме, а вполне определенный код. Даже если плюнуть на потерю обновлений (а значит все баги и правки в последующем в коде битрикс придется самим делать - что повысит долгосрочную стоимость проекта), плюнуть на то, что странно само по себе возникновение хайлоадзадачи в плане зарегистрированных пользователей сайта.
            Ок если тут "хайлоад" тогда надо отказываться от стандартной таблицы, и не тащить с ней груз универсальности. А создавать свою таблицу и вписывать в логику работы: это будет и более правильно, и более шустро работать, не потеряется возможность обновлять систему.

            Тут надо понять одно: битрикс, как и любая другая CMS это универсальный инструмент, под широкий круг задач. И если у вас проект высоко нагруженный, в узких местах вам необходимо избавиться от не нужного функционала. И решение со своей заточенной под проект таблицей будет работать быстрее и надежнее (если конечно грамотный программист) даже с использованием ОРМ, чем костыли в стандартном функционале с прямыми обращениями. Ну или уж если костылить, тогда быстрее все это запихнуть в триггер БД. (вроде было выше, что данные все берутся из оной).
            Ответить
            • Чувак, ты реально крэйзи.
              Ответить
              • Человек просто искренне пишет. Он же не знает что тут сидят глумливые битарды
                Ответить
                • Для этих случаев я и придумал фразу <<Чувак, ты реально крэйзи>>. Она в меру добрая и нелепая.
                  Ответить
                • "Злые вы, уйду от вас" (с) не мой...
                  Ответить
                • А по-моему он просто пикабушник.
                  Ответить
                  • Ну чо-то он слишком вореционирует со своим битриксом ебучим.
                    Ответить

    Добавить комментарий