Форум PHP

Здравствуйте. Мне нужно проверить, насколько хорошо работает форма для загрузки картинки на сайт. Есть у кого-нибудь какой-нибудь файл .jpg, .gif, .png, который на самом деле скрипт или в котором часть скрипта PHP? Как проверить мне, крепкая ли проверка у меня написана? )

а Вы скриптик выложите сюда, а если большой, то прикрепите к сообщению и попросите прокомментировать...

Проверка:

<?php $_FILES [ 'file' ] [ 'tmp_name' ] = htmlspecialchars ( $_FILES [ 'file' ] [ 'tmp_name' ] ); $getimagesize = getimagesize ( $_FILES [ 'file' ] [ 'tmp_name' ] ); $filesize = filesize ( $_FILES [ 'file' ] [ 'tmp_name' ] ); if ( $getimagesize [ 'mime' ] != 'image/jpeg' && $getimagesize [ 'mime' ] != 'image/png' && $getimagesize [ 'mime' ] != 'image/gif' ) {     $error = true;     // Некорректный формат файла. } elseif ( $getimagesize [ 'mime' ] == 'image/png' || $getimagesize [ 'mime' ] == 'image/gif' ) {     if ( $filesize < 64 || $filesize > 65536 )     {         $error = true;         // Некорректный размер файла.     } } elseif ( $getimagesize [ 'mime' ] == 'image/jpeg' ) {     if ( $filesize < 64 || $filesize > 524288 )     {         $error = true;         // Некорректный размер файла.     } } elseif ( $getimagesize [ 0 ] > 3000 || $getimagesize [ 1 ] > 3000 ) {     $error = true;     // Некорректное разрешение файла. } elseif ( $getimagesize [ 0 ] < 50 || $getimagesize [ 1 ] < 50 ) {     $error = true;     // Некорректное разрешение файла. } ?>

И тишинааа..

Да вроде все нормально...
Не понятно зачем ограничения вроде ( $filesize < 64 || $filesize > 65536 ) и ( $getimagesize [ 0 ] > 3000 || $getimagesize [ 1 ] > 3000 )....
Но это наверно для экономии ресурсов. К безопасности отношения не имеет.

Для проверки безопасности вполне достаточно:

<?php   $getimagesize = getimagesize( $_FILES['file']['tmp_name'] ) or die('Not Image');   if( !in_array( str_replace('image/','',$getimagesize['mime']), array('png','gif','jpeg') ) ) exit('Wrong mime');

Да, да. Я пока для Beta-версии ограничения..

Я вопрос задавал потому, что хотел резать картинку со всех сторон по 1px, чтобы точно, если в коде картинки были бы гадости какие, они пропадали бы. Есть смысл сделать обрезку картинки перед сохранением? В одной из старых статей Habrhabr нашёл такой вариант, не помню уж страницы, конечно.

А зачем резать, можно и просто открыть ресурс и потом сохранить его.

Он по моему имеет ввиду какуюто проверку картинок несущих в себе вредоносный код

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

Я думал, обычно этот код вначале файла вставляют..

Вообще, я сохраняю изображение обычным copy ( ). После, используя сохранённую копию в папке /temporary/, создаю новые копии разных размеров ширины и высоты в конечной папке, а после оригинал в /temporary/ удаляю.

Пример:

<?php // .. if ( $getimagesize [ 'mime' ] == 'image/jpeg' ) copy ( $_FILES [ 'file' ] [ 'tmp_name' ] , $dir.'/temporary/f'.$var [ 'id' ].'_'.$code.'.jpg' ); // .. ?>

Далее, единственное, что делаю с оргиниалом:

<?php // .. $filename = $dir.'/temporary/f'.$var [ 'id' ].'_'.$code.'.jpg'; if ( $getimagesize [ 2 ] == 2 ) $imagecreatefromjpeg = imagecreatefromjpeg ( $filename ); else if ( $getimagesize [ 2 ] == 1 ) $imagecreatefromjpeg = imagecreatefromgif ( $filename ); else if ( $getimagesize [ 2 ] == 3 ) $imagecreatefromjpeg = imagecreatefrompng ( $filename ); // .. ?>

При загрузке файлов использовать функцию copy() не рекомендуется. Для того чтобы сделать копии изображения, совсем не обязательно сохранять его в какой-то папке, затем считывать и производить операции, у вас оно уже есть загруженное, а директория /tmp ничем не хуже любой иной.

То есть лучше свести всё к:

<?php // .. $filename = $_FILES [ 'file' ] [ 'tmp_name' ]; if ( $getimagesize [ 2 ] == 2 ) $imagecreatefromjpeg = imagecreatefromjpeg ( $filename ); else if ( $getimagesize [ 2 ] == 1 ) $imagecreatefromjpeg = imagecreatefromgif ( $filename ); else if ( $getimagesize [ 2 ] == 3 ) $imagecreatefromjpeg = imagecreatefrompng ( $filename ); // .. ?>

Вы не поняли.
Да, нет смысла сперва переместить загруженный файл (а перемещать его надо, если сразу это делать, функцией move_uploaded_file(), не copy()) в некую директорию, а только затем производить с ним действия. Да, производите эти действия сразу, с файлом загруженным - $_FILES['file']['tmp_name']. Это вы поняли, а не поняли вы другое.

Зачем вам весь этот зоопарк ресурсов в зависимости от расширения?
Загрузку файла нужно начинать с проверки отсутствия ошибки - $_FILES['file']['error'], если ее нет, значит файл загружен. Далее можете проверить размер файла, и если он в пределах допустимого, то далее getimagesize($_FILES['file']['tmp_name']) вернет вам тип, если только есть необходимость сохранять файл именно такого же типа, как и исходный.

Но, это принципиально только для PNG файла (если есть необходимость сохранения в этом формате), а вот для GIF и JPEG эти параметры можно узнать и без помощи getimagesize, для этого можно получить палитру изображения, и если файл основан на палитре (GIF, PNG8), то вам такое будет возвращено, или нет (JPEG, PNG24).

Следовательно, без всякого зоопарка if ( $getimagesize [ 2 ] == 2 ) $imagecreatefrom... делаем другое:

<? if($img = imagecreatefromstring(file_get_contents($_FILES['file']['tmp_name'])) {    //производим необходимые операции над открытым ресурсом $img   //включая и сохранение файла в директории в зависимости от типа,   //о чем говорилось ранее   imagejpeg($img,$path,$quality) //оригинал   //...... и т.д.  )

При этом все что возможно было "приклеено" в конец файл останется за бортом. Используя это, кроме проверок указанных выше, проверяйте еще и действительность загрузки файла по HTTP формой (is_uploaded_file).

Да, я уже сегодня с утра сам внедрил туда ряд проверок:

<?php // .. elseif ( $_FILES [ 'postEditForumEmblemFile' ] [ 'error' ] == 1 ) // UPLOAD_ERR_INI_SIZE {     $error = true;     // Размер файла превысил максимально допустимый размер, который задан на сервере. } elseif ( $_FILES [ 'postEditForumEmblemFile' ] [ 'error' ] == 1 ) // UPLOAD_ERR_FORM_SIZE {     $error = true;     // Размер файла превысил значение, указанное в форме загрузки. } elseif ( $_FILES [ 'postEditForumEmblemFile' ] [ 'error' ] == 3 ) // UPLOAD_ERR_PARTIAL {     $error = true;     // Загружаемый файл был получен только частично' ); } elseif ( $_FILES [ 'postEditForumEmblemFile' ] [ 'error' ] == 4 ) // UPLOAD_ERR_NO_FILE {     $error = true;     // Файл не был загружен. } // .. ?>

остальное Вами посоветованное я обязательно научусь использовать и отредактирую свою функцию. Пока занялся уже другими файлами на проекте, доберусь — сделаю.

А смысл такой писанины?

Всмысле, почему не использую ||? Там, где я указал комментарии на самом деле выводится непосредственно текст ошибки )

Например:

<?php elseif ( $_FILES [ 'postEditForumEmblemFile' ] [ 'error' ] == 3 ) // UPLOAD_ERR_PARTIAL  {      $error = true;      $errorText = 'Загружаемый файл был получен только частично'; } ?>

1 - описание ошибок. Вы задумывались когда либо о том, о чем будет думать пользователь, читая ваши сообщения об ошибках? "Размер файла превысил максимально допустимый размер, который задан на сервере" - вы думаете рядовому пользователю это обязательно должно быть известно? Вполне возможно, что даже вы, пишущий код и интересующийся вопросами связанными с загрузкой файлов наступали на эти грабли - не знали о таких настройках, по крайней мере такое незнание на первых стадиях не редкость. А что же вы хотите от пользователя, который вообще не обязан этого знать? Наверное нужно сообщать пользователю о максимально разрешенном вами размере загружаемого (ориентируясь на эту установку), и сообщать пользователю, что он превысил допустимый вами, а не заставлять его гадать, что у вас там за установки на сервере. Тоже самое касается и "Размер файла превысил значение, указанное в форме загрузки", что для рядового пользователя еще более туманнее, чем первое.

"Загружаемый файл был получен только частично" и "Файл не был загружен" - и что дальше? Что делать пользователю в этом случае - рыдать, материться, подать на вас в суд...? Ведь неполная загрузка файла и вообще не загружен, может произойти и по причине сбоя сервера, значит не вываливать эти ошибки на голову нужно, а предлагать повторить загрузку, и если опять произойдет сбой, значит это не случайный сбой, а закономерность, которую надо устранять...

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

2 - обработка ошибок. Нет, не в смысле почему вы не используете ИЛИ, а почему вы вообще пишите кучу проверок там, где можно обойтись без них, тем более, что вы хотите детальной обработки ошибок. Но ведь у вас кроме ошибок самого процесса загрузки видимо возможны и другие, насколько это можно судить по вашему коду выше, например, проверка разрешения, тип файла и т.п..

Отвыкайте забивать код этими дремучими if...else...elseif... и до бесконечности. Что значит код ошибки 1, 2, 3...? Это индекс. А как можно использовать индекс? А так, что он позволит детектировать ошибки и обращаться к их описаниям автоматически, по индексу ее, другими словами не явно указывая при проверке и описывая, а анонимно.

И ведь нужно для этого всего ничего:

<? // создать массив описаний ошибок: $errors_desc = array(1=>'Error 1 desc', 'Error 2 desc', 'Error 3 desc', ... ,'Error N desc'); //проверка if(!$_FILES['file']['error']) {    //все ОК!  } else {   echo $errors_desc['$_FILES['file']['error']']; }

Это и проще, и читабельнее, и гораздо функциональнее, и вот почему. Если пользователь совершит несколько ошибок, вы же не будете обнаруживать их по одной, вываливая и обрубая работу скрипта (есть и такие ужастики примеров кода). Другими словами, при такой организации можно опять таки одной строкой произвести данную операцию. Для этого, при проверке условий загрузки, достаточно коды ошибок (а ошибкам по вашим условиям нужно тоже присвоить индексы) помещать в массив $error (а не делать эту переменную булевым значением), и если массив не пуст после проверки операции загрузки и ваших условий, то достаточно:

<? $error = $error ? '<div class=error><p>'.implode('</p><p>',array_intersect_key($errors_desc, array_flip($error))).'</p></div>' : null; //и например, перед формой выводим echo $error //и пользователь получит все при возврате формы для исправления ошибок

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

И заметьте, что в коде ничего сложного и сверх естественного нет, все просто, надо только задумываться над тем, что и для кого вы пишите, ставить себя на место пользователя, а в ситуации когда вам вместо разъяснения, на вас не обращают внимания, или хамят, думаю были.... Не стоит такие ситуации моделировать в своих скриптах, а писать компактнее поможет обязательная утренняя зарядка - три листа руководства РНР для ознакомления.

Я понимаю, что Вы советуете. Как бы объяснить ситуацию.. Я разрабатываю этот проект очень давно, и уделяю каждой строчке кода много внимания, даже слишком. То, что я здесь представлял (код), это лишь пятая часть кода в .php-файле, который вместе с другими десятью подключается ещё в один .php-файл. Тотальная обработка всевозможных ошибок — то, что я создаю везде. Проект даже ещё не запущен, а после запуска мне нужно знать, если будут ошибки, какие они будут. Многие проверки я осуществляю даже на случаи, которые крайне маловозможны. Например, что касается размера файла. У меня стоит своё ограничение, и если оно будет превышено, я обычным языком скажу "Некорректный размер файла". А то, что установленный лимит выше моего в скрипте — однозначно. Это проверка на крайную возможную ошибку. Для меня самого, нежели пользователя. О сокращении текущих существующих строк кода я думаю со временем. То есть вначале делаю обширный вариант, чтобы самому не запутаться, а уже со временем, вернувшись к этим строкам я меняю их и сокращаю. Возможно, Вы назовёте меня глупым, или зря теряющим время, но это моя натура. Пока ничего поделать не могу.

Ну почему же глупым, нет. А вот излишество в коде подобное куче условий там, где оно не только лишнее, но как раз и мешает именно сосредоточиться на главном, и не на пользу вам. Познание языка, опыт в программировании конечно же приходит со временем, а вот думалку нужно включать сразу, привыкать к этому изначально, иначе исправить потом привычки (натуру) или очень трудно, или не возможно.
Ваша задача как программиста как раз в том, чтобы учится решать оптимально, это главное, а не аккуратная череда строк кода, которые конечно же будут выполнять задачу, но крайне не эффективно. В чем же тогда смысл писать так, а потом перелопачивать? Было бы что, другое дело, а так....

Думаю, есть что.. С Наступающим! ;о)

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

Простите, что улыбаюсь сейчас, а не с серьёзным видом всё это запоминаю )

Я правильно понял, все Ваши плеяды на эту тему для того, чтобы я заменил:

<?php unset ( $error ); if ( $_FILES [ 'file' ] [ 'error' ] == 1 ) $error = 'Размер файла превысил максимально допустимый размер, который задан на сервере.'; elseif ( $_FILES [ 'file' ] [ 'error' ] == 1 ) $error = 'Размер файла превысил значение, указанное в форме загрузки.'; elseif ( $_FILES [ 'file' ] [ 'error' ] == 3 ) $error = 'Загружаемый файл был получен только частично'; elseif ( $_FILES [ 'file' ] [ 'error' ] == 4 ) $error = 'Файл не был загружен.'; if ( $error ) echo $error; ?>

этим:

<?php $errors = array ( 'Размер файла превысил максимально допустимый размер, который задан на сервере.' , 'Размер файла превысил значение, указанное в форме загрузки.' , 'Загружаемый файл был получен только частично.' , 'Файл не был загружен.' ); if ( $_FILES [ 'file' ] [ 'error' ] ) echo $errors [ $_FILES [ 'file' ] [ 'error' ] ]; ?>

?

Ну да, как пугали пользователя какой-то установкой на сервере, которая пользователю в общем-то и не нужна, так и продолжаете пугать. Вы же не для себя пишите, вы пишите для массы обычных людей, а не "программеров".
Дело не в том, как представить сами ошибки и сделать их вывод, а в том, что ваш код похож на песнь нанайца - что вижу, то и пою. А программа - это как роман, где есть главные действующие лица, мизансцены, преамбула, пролог, эпилог... Вы не пишите программу, вы пишите строки кода - это видно по первой его части вами представленной, и продолжаете в том же духе далее.
Нельзя написать книгу абы как, а потом ее переписать до шедевра. Книга - это образы и сюжеты завязанные воедино, не будет этого, не будет книги.
Вот так и ваш код - нет в нем того, что можно назвать программой, и если вы оправдываете это своими привычками, то значит нужно отвыкать от них.

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

:)
Для того, чтобы узнать вкус супа совсем не обязательно съесть целую кастрюлю его. Мне и не нужна гора ваша, я вижу как вы к ней подходите.

Суп — это уже что-то совсем не то. Вы мне ещё про поля гороховые расскажите )

Ну почему же не то? )
Программирование - это представление самой задачи и нахождение способов ее решения. Вот это как раз определяющее, а не знание конструкций языка.

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

Каждый язык описывает свои инструменты, например, в одном языке это просто "лопата", а в другом более детально - "клиновидный инструмент с острой рабочей кромкой". При этом во всех языках будет написано одно и тоже назначение этого инструмента - "копать".

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

А что значит копать? Можно нажимать ногою лопату, вынимая по кусочку почвы, последовательно выкопав ямку. А можно штыком лопаты прорубить контур, а затем сразу подвести лопату под этот кусок, убрав всю почву одним усилием - ямка готова с меньшой затратой энергии.

Вы должны учится мыслить не только конкретно, но и абстрактно, чтобы понимать более всесторонне то, над чем вы работаете. Пример с супом, это как раз то, что абстрагируясь от программирования, отвечает вашему замечанию - не обязательно видеть весь ваш код, достаточно его части, чтобы увидеть каков стиль вашего программирования, и как не эффективно вы пользуетесь лопатою.

Учебники не учат вас стилю программирования, тем более этому не учит описание самого языка. Стиль вы будете вырабатывать сами, а он будет складываться из много, но по большей части это будет ваше мышление, образ этого мышления.

Давайте проведем маленький эксперимент. Предположим, что я ваш заказчик. Вы для меня пишите эту задачу - загрузку изображений на сервер. Опускаем то, что вы уже написали - проверку ошибок при загрузке, как есть так и есть, тем более я как заказчик "тупой" в плане программирования, и не привередлив.

Я просто дополняю эту задачу новым условием, которое вы должны решить. А именно - изображения, которые я буду загружать, могут иметь поля незначащие - слева, справа, да и вообще со всех сторон. Это некие полосы цветовых оттенков, которые при загрузке вы должны вырезать из изображения.

Не надо кода, просто словами опишите то, что вы должны сделать для этого, что потребуется по вашему для этого, и по пунктам опишите как вы будете решать эту задачу. А потом сделаем "разбор полетов". Ну как, согласны?

И ещё, кстати, вопрос.

Я использую getimagesize [ 'mime' ] и filesize вместо параметров, которые уже передаёт $_FILES (size и type). В этом есть смысл, или, если пользователь нарочно передал неверный параметр MIME, то и getimagesize не поможет и покажет значение, навязанное отправляемым?

Конечно надо проверять, а не верить тому, что вам передал клиент, и это касается не только загрузки файлов, это вообще обязательное правило для всего, что вы получаете от клиента.

;о)