"PHP" / Говнокод #3292 Ссылка на оригинал

0

  1. 1
  2. 2
  3. 3
  4. 4
  5. 5
$image = $_FILES['image']['tmp_name'];    
    if( !empty($image) )
    {
       @$src = imagecreatefromjpeg($image);
if($src==false){exit ('Это не картинка'); }

Этот кусочек кода был спором 7 человек;)
Это говнокод?

Запостил: FallenServer FallenServer, (Updated )

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

  • ava guest # Ссылка на оригинал 0

    Я, guest, находясь в нетрезвом состоянии, торжественно заявляю:
    Есть функция getimagesize() которая выводит FALSE если пикча инвалидная
    Ответить

    • ava guest # Ссылка на оригинал 0 ⇈

      Не соглашусь. Бывают ситуации, когда вывода теоретически возможного сообщения об ошибке нужно по-любому избежать, даром что обработку ошибки ты предусматриваешь. Сермяжный пример: $dbLink = @mysql_connect(...). (Про PDO, конверсию ошибок в исключения, абстракцию от БД и прочие true coding practices в курсе, но это не значит, что я вижу смысл абсолютно всегда ими пользоваться.) Другой вариант - @unlink(), если мы не уверены, что удалить файл удастся (safe_mode иногда может не приниматься в расчёт функцией is_writable()). Ещё вариант - @unserialize(), когда мы не уверены, что данные приходят из надёжного источника (например, из cookies) - не городить же валидацию сериализованных данных. Хотя всё это, конечно же, не отменяет необходимости стремиться избежать использования @ насколько это только возможно.
      Ответить

      • ava striker # Ссылка на оригинал 0 ⇈

        в пхп можно настроить отлов всех типов ошибок (warning, notice, etc) и кидать вместо них исключения
        в том же мзз unlink выглядит так
        try {
        unlink('file.txt');
        } catch (mzzException) {
        echo 'Ошибка удаления файла';
        }
        Ответить

        • ava guest # Ссылка на оригинал 0 ⇈

          try-catch блоки кидать везде подряд - тоже безвкусица и говнокод
          Ответить

      • ava striker # Ссылка на оригинал 0 ⇈

        Но вообще, ваша позиция мне знакома, поэтому плюсанул вам каммент
        Ответить

      • ava wwwguru # Ссылка на оригинал 0 ⇈

        Можно даже без try .. catch - просто своя функция в set_error_handler()
        Ответить

  • ava guest # Ссылка на оригинал 0

    если в tmp_name содержится расширение (не вспомню точно на самом деле)), то лучше бы сначала проверить на предмет оного, чем сразу-же напрягать сервер вызовом imagecreatefromjpeg.
    Ну а во-вторых - форматирование кода ни к черту, но блиадь, кучу одноразовых переменных - объявим. Зачем? Что-бы легче свой кал понять можно было?
    На говнокод не тянет, но на никчемный калокод - еще как.
    Ответить

      • ava turdman # Ссылка на оригинал 0 ⇈

        Отфильтровать очевидные ошибки пользователя.
        Хотя практическая польза, скорее всего, будет невелика
        Ответить

        • ava cheef # Ссылка на оригинал 0 ⇈

          Это пустая трата времени, так-как совсем ненадёжно. Проще сразу же проверять mime-type и не париться
          Ответить

          • ava Lure Of Chaos # Ссылка на оригинал 0 ⇈

            однопенисуально, что расширение, что миме, которое от броузера пришло - все равно не гарантия, что картинка
            Ответить

            • ava cheef # Ссылка на оригинал 0 ⇈

              причём тут finfo и " расширение, что миме, которое от броузера пришло"? Почитайте документацию.

              Вы случаем на пол-ставки КО не подрабатываете? 🙂 Очевиден факт, что можно подделать расширение, заголовки, в том числе и файла. Но наиболее приемлемый уровень надёжности обеспечивает проверка заголовков файла, её достаточно во всех случаях, кроме исключительно специфичных - когда программист параноик 🙂 А остальные решения вполне можно причислить к велосипедам.
              Ответить

  • ava guest # Ссылка на оригинал 0

    Этот код, безусловно, говно в силу следующих причин:
    1) Множество кодов на PHP является подмножеством говнокодов
    2) Парсинг параметров и работа с картинками не должна делатся в одном месте: это разные задачи, и они должны быть разнесены в разные классы иначе код не покрыть юнит-тестами, а так же получается low cohesion
    3) затыкание чего либо собачкой (равно как и использование ERROR_LEVEL ниже DEBUG) -- дурной тон и прямой путь к ошибкам
    4) использование "exit" не позволяет использовать этот модуль в других задачах: библиотека не имеет права останавливать работу приложения.
    5) Хардкод литералов не позволяет нормально локализовать приложение.

    Вывод: Этот типичный код на php, нарушающий все азы не только объектно-ориентированного и модульного, но даже банльно структурного программирования.

    Благодарю тебя Господи, что я не PHP программист.
    Ответить

    • ava telnet # Ссылка на оригинал 0 ⇈

      Хорошо сказал, всё по делу. Вот только
      > 1) Множество кодов на PHP является подмножеством говнокодов
      Не всякий код на PHP - говнокод. Я согласен, что из-за низкого порога вхождения и косяков в, скажем так, идеологии языка писать на PHP говнокод во сто крат легче и проще, чем нормальный чистый код, и даже есть искушение писать как проще, а не как правильнее. Я прекрасно вижу, как пагубно сии факты влияют на профессиональные качества программистов, как тормозят их рост, а в клинических случаях и вовсе его стопорят ("знаешь что, иди-ка ты на хрен со своими парадигмами, я так привык/завтра заказ сдавать..."). Но это не означает, что нормальный чистый код на PHP а) написать невозможно и б) никто никогда не пишет. Хотя тот факт, что отношение обезьян к людям среди PHPшников заметно выше, чем среди пишущих на других языках, самого очень расстраивает.
      Ответить

      • ava Lure Of Chaos # Ссылка на оригинал 0 ⇈

        наверное все дело в принципах и возможностях языка... Чем строже язык, тем меньше говна - я выделяю c,с# и java
        Я представляю какое вонючее говно можно было бы написать на том же Katahdin
        Ответить

        • ava telnet # Ссылка на оригинал 0 ⇈

          Да, пожалуй. Но в случае с PHP клиника ещё и в том, что многие идеологически неправильные принципы вводились в него искусственно, причём подчас с благими намерениями. Банальнейший пример тому - magic_quotes: официальное его назначение - помочь обезьянам, забывающим экранировать входящие данные при подстановке в SQL-запросы и файловые функции, сделать их быдлоскрипты безопаснее. В итоге сильно безопаснее скрипты таки не стали, зато всем пришлось плеваться и очищать данные от лишних слэшей, а затем, когда magic_quotes стало можно выключать, писать костыли для совместимости со включённым и выключенным режимами. Маразм чистейший, но скоро вот отмечаем десять лет, как все с ним трахаются.
          Ответить

          • ava Lure Of Chaos # Ссылка на оригинал 0 ⇈

            между прочим, именно поэтому пхп и вылез на вершину популярности, потому что в самом начале было заявлено, что писать быдлоскрипты на нем быстро и просто. Практика показала, что это плохо и появились костыли, потом костыли к костылям и в результате невозможно избавиться от множества раздражающих вещей
            Ответить

  • ava redh3ad # Ссылка на оригинал 0

    >Есть функция getimagesize() которая выводит FALSE если пикча инвалидная +1

    В любом случае:
    $type = image_type_to_mime_type($image);
 if ($type !== 'IMAGETYPE_JPEG') {
     die('Это не картинка');
 } else {
     ...
 }
    Ответить

      • ava mrbig66 # Ссылка на оригинал 0 ⇈

        Зависит от задумки автора. Вы же не хотите, чтобы пользователи начали загружать на ваш сайт исключительно бээмпэшки.
        Ответить

      • ava redh3ad # Ссылка на оригинал 0 ⇈

        У ОПа:
        @$src = imagecreatefromjpeg($image);

        Естествеено, по-хорошему проверка на все типы изображений.
        Ответить

    • ava telnet # Ссылка на оригинал 0 ⇈

      Что-то тут напутано. По мануалу, image_type_to_mime_type() возвращает MIME-тип (в данном случае "image/jpeg"), а на вход мы ей кормим число, равное одной из констант IMAGETYPE_xxx. По мануалу же, это число можно взять из возврата функции getimagesize() (элемент массива с индексом 2). Правда, из возврата той же функции можно сразу взять MIME-тип (элемент массива с символьным ключом 'mime').
      Ответить

      • ava redh3ad # Ссылка на оригинал 0 ⇈

        Да, каюсь, напутал 🙂
        ...
$properties = getimagesize($file)
    or die('Это не картинка');
switch ($properties[2]) {
    case IMAGETYPE_JPEG:
        $src = imagecreatefromjpeg($file);
    break;
    case IMAGETYPE_GIF:
        $src = imagecreatefromgif($file); 
    break;
    case IMAGETYPE_PNG:
        $src = imagecreatefrompng($file); 
    break;
    ...
    ...
}
        Ответить

        • ava This is obvious # Ссылка на оригинал 0 ⇈ 

          $props = getimagesize($file)
while(!$props)
{ 
          die; 
          if(!die){
                      exit; 
           } else if (!die && !exit) 
           { 
                          do
                          {
                            die; exit;
                           } while(!die && !exit);
           }
}
          Ответить

          • ava telnet # Ссылка на оригинал 0 ⇈

            Первые две строчки должны быть спеты нежным женским голосом, остальное гроулом под звуки расстроенной электрогитары, визг болгарки, скрежет напильника и стоны забиваемого скота. Сингл назовём "Узнай размеры картинки или умри" ("Get image size or die").
            Ответить

  • ava cheef # Ссылка на оригинал 0

    вообще действительно приличной реализацией этой задачи была бы проверка mime-type, через тот же finfo
    Ответить

    • ava telnet # Ссылка на оригинал 0 ⇈

      > PHP >= 5.3.0, PECL fileinfo >= 0.1.0
      Пока ещё не кроссхостингово. Кто как, а я пока не рискую пользоваться новыми фишками 5.3. Вот пошире распространится...
      Ответить

      • ava cheef # Ссылка на оригинал 0 ⇈

        Это да, но я использую версию из PECL с php < 5.3. Оказалось, что проще установить эту либу на продакшене, чем изобретать велосипеды.
        Ответить

    • ava HyperGeek # Ссылка на оригинал 0 ⇈

      Ну или ImageMagick, он более распространен. К тому же getimagesize() не распознает некоторые JPEGи (может уже пофиксили).
      Ответить

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

Из-за тебя ушел bormand, guest!

    А не использовать ли нам bbcode?

    ↑
    8
    ↓