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

    +153

    1. 1
    2. 2
    3. 3
    4. 4
    $res1 = mysql_query("INSERT INTO tbl_orders
    (date_order,name,id_city,city_text,kod_tel_1,tel_1,kod_tel_2,tel_2,description,subject_text,id_teacher,id_category,category_text,check_student,place_student,check_teacher,place_teacher,price,email,icq,order_close,id_status,to_send,id_whence_came,text_whence_came,one_teacher)
    VALUES ('$ts','$name','$city','$other_city','$kod_tel_1','$tel_1','$kod_tel_2','$tel_2','$prim','$other','$id_rep','$category','$other_cat','$check_uch','$district_uch','$check_prep','$district_prep','$stoim','$email','$icq','0','1','1','$whence_came','$text_whence_came','$one_teacher')")
    or die ("Query failed".mysql_error());

    Услада ума, любящего искать связи. Автор чутка лажанул — некоторые переменные всё же совпадают с полями таблицы.

    Запостил: defann, 07 Декабря 2009

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

    • Я всегда так делаю, зато не парюсь, куда новый столбец добавлять в таблицу.
      Добавлю новыой столбец 5 в таблице, а код-то мой будет работать, и не надо мне его исправлять.
      Ответить
    • Ну у меня нет слов, Олег. Это *пиздец*.
      Ответить
      • Как сложно названия столбцов написать.
        Ответить
        • Олег, разуй глаза и прочти код.
          Ответить
          • а-а-а-а, $res1 = и or die. Точно.
            Ответить
            • Не понял.
              Ответить
              • - это большой, очень большой инсерт...
                - аффтар не парится с конкатенацией, и юзает двойные кавычки. Несомненно огромный плюс к быстродействию!
                - аффтар юзает туеву хучу переменных, бОльшая часть из которых не совпадает с названием полей => полная помойка как минимум в текущей области видимости и в башке того, кто будет это читать спустя какое-то время. В идеале использовать массив, с ключами равными названиям полей.
                - аффтар мудаг, ибо либо просто не проверяет переменные перед инсертом => SQL Inj (сложно проверить руками такую тучу переменных), либо (если он трудолюбив и усерден :))) проверяет большим копипастом одной и той же функции - но тут даже эвтаназия бессильна...
                - аффтар не юзает враппер БД, что еще раз указывает на его усердие и трудолюбие (настоящий программист должен быть ленив)
                Ответить
                • Господи, хоть один человек в здравом рассудке.
                  Ответить
                  • Лекарство от говнокода: http://www.ozon.ru/context/detail/id/1657382/
                    Ответить
                • - это большой, очень большой инсерт...
                  И?

                  Тут может быть Sql-инъекция. Но не зная, как человек проверял переменные до
                  это запроса, я утверждать о ней не буду.

                  "бОльшая часть из которых не совпадает с названием полей". Как раз, совпадает: я специально скопировал этот запрос в текстовой файлик и подгонял пробелами названия переменных чтоб они стояли под названиями тех столбцов, куда вставляются.
                  Ответить
                  • - это большой, очень большой инсерт...
                    И?


                    Это большой, очень большой инсерт, написанный РУКАМИ и В СТРОКУ.
                    Даже вам, при всем желании защитить этот кусок говна пришлось подгонять пробелами названия переменных :) Это говорит о чем? Правильно - об отсутствии у автора понимания необходимости грамотного форматирования кода.

                    Нащот инъекции: если НЕ проверял - значит инъекция => говнокод. Если проверял - то видимо это делалось скопипасченной пачкой вызовов некой функции проверки. Копипаста = говнокод.

                    Еще вопросы?
                    Ответить
                    • вопросов нет. Лучше использовать классы для DB, (например, от того же Дмитрия Котерова).
                      Насчет проверки переменных: мог быть такой код у него выше:
                      $names = array('ts', 'name', 'city', ....);
                      foreach ($names as $name) {
                      check($$name);
                      }
                      Ответить
                      • Олег, ну не выпячивайте вы так своё невежество, зачем же.
                        Ответить
                        • А в чём невежество? Я не хочу быть невежественным, разъясните.
                          Ответить
              • Смотри мой ответ ниже.
                Ответить
            • поделитесь, что вы там увидели то =)
              Ответить
              • Пошел, посмотрел документацию по mysql_query
                узнал, что в случае инсерта, она возвращает true или false,
                Провел следующий эксперемент.

                <?php
                $var = ret() or die("yo");
                print $var;
                function ret () {
                return false;
                }
                ?>

                Так вот у меня вызывался die,

                то есть когда тут стоит "or die", присваивать значение переменной $res не нужно. Если die не выполнится, то $res будет true. Ну а если die выполнится, то уже будет все равно что в $res.
                Ответить
                • это к чему вообще? :-0
                  Ответить
                • вы сейчас объяснили очевидное) хотите ещё магии?
                  defined('SOME_CONSTANT') || define('SOME_CONSTANT', 'Hello world');
                  Ответить
    • or die ("Brian failed".hands_error());
      Ответить
    • код отдаёт поповщиной
      Ответить

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