[ Поиск ] - [ Пользователи ] - [ Календарь ]
Полная Версия: Прокомментируйте мой код
Вежливый гость
Здравствуйте! Написал код добавления новостей на сайт по заказу одной фирмы, протестируйте, покритикуйте)))



<?php

//Подключаем конфиг-файл
include ("bd.php") ;

//Инициализируем переменные
$title = !empty ($_POST['title']) ? trim ($_POST['title']) : NULL ;

$sectionid = !empty ($_POST['sectionid']) ? (int)$_POST['sectionid'] : NULL ;

$catid = !empty ($_POST['catid']) ? (int)$_POST['catid'] : NULL ;

$text = !empty ($_POST['text']) ? trim (nl2br($_POST['text'])) : NULL ;


//Из основного текста отбираем текст для анонса и продолжения))
$introtext = substr ($text, 0, 400) ;

$fulltext = substr ($text, 400) ;

//Заменяем переносы строки на пробел в анонсе
$introtext = str_replace ("<br />", " ", $introtext) ;

//Устанавливаем дату публикации материала
$date = date ("d.m.Y /H:i/ ") ;


$images = $_FILES["images"]["name"]."|left||0||bottom||" ;

//Директория для загрузки фоток
$uploaddir = 'images/stories/';

$uploadfile = $uploaddir.basename($_FILES['images']['name']);

$surt = "{mosimage}" ;


//Отключаем "Магические кавычки"
if (get_magic_quotes_gpc ())
{
$title = stripslashes ($title) ;
$text = stripslashes ($text) ;
$introtext = stripslashes ($introtext) ;
$fulltext = stripslashes ($fulltext) ;
}





if (isset ($_POST['Ok']))
{
if ($title and $text and $sectionid and $catid and $_FILES["images"]["name"])
{

copy($_FILES['images']['tmp_name'], $uploadfile) ;
$res = mysql_query ("INSERT INTO `jos_content` (`title`, `introtext`, `fulltext`, `sectionid`, `catid`, `state`, `created`, `images`)
VALUES ('"
.mysql_real_escape_string ($title)."',
'"
.$surt.$date.mysql_real_escape_string ($introtext)."',
'"
.mysql_real_escape_string ($fulltext)."',
'"
.$sectionid."',
'"
.$catid."', 1, NOW(), '".$images."')") ;

if ($res)
{
echo "<span style='color: green'>Материал успешно добавлен</span>" ;

}
}

elseif (empty ($title) or empty ($text) or empty ($sectionid) or empty ($catid) or $_FILES["images"]["name"])
{
echo "<span style='color: #cc0000'>Заполните все поля</span>" ;
}
}





?>



<form action="" method="post" enctype="multipart/form-data">

<p><label>
Введите заголовок</label><br />
<input
type="text" name="title" value="<?php echo htmlspecialchars ($title) ; ?>"/>
</p>

<p><label>
Введите текст статьи</label><br />
<textarea
cols="50" rows="25" name="text"><?php echo nl2br(htmlspecialchars ($text)) ; ?></textarea>
</p>

<p><label>
Загрузите первое фото</label><br />
<input
type="file" name="images" /></p>

<p><label>
Выберите раздел</label><select size="1" name="sectionid">
<option
value=""></option>
<option
value="1">Новости</option>
</select>
</p>

<p><label>
Выберите категорию</label><select size="1" name="catid">
<option
value=""></option>
<option
value="1">Политика</option>
<option
value="2">Общество</option>
</select>
</p>



<p><input
type="submit" value="Добавить материал" name="Ok" />

</form>

<p><a
href="pan2.php">Просмотреть все новости за день</a></p>

<p><a
href="pan4.php">Просмотреть главные новости</a></p>




Спустя 5 минут, 28 секунд (29.10.2010 - 10:44) ApuktaChehov написал(а):
Чего то я не нашел у вас, проверку изображений.
Если вы ее не сделали, то к вам на сайт можно залить любой файл php и сделать все что заблагорассудиться, кому угодно.

Спустя 1 минута, 11 секунд (29.10.2010 - 10:45) Guest написал(а):
дело в том, что доступ к этому файлу тока с одного ip, поэтому я проверку не делал. а как код сам?

Спустя 10 минут, 31 секунда (29.10.2010 - 10:56) ApuktaChehov написал(а):
А как вы заблокировали доступ к файлу?

Вот это зачем?
elseif (empty ($title) or empty ($text) or empty ($sectionid) or empty ($catid) or $_FILES["images"]["name"])

Вы же при первом условии проверяете все переменные.
Если первое условие не выполнится, тогда 100%, как минимум одна из переменных пуста. Достаточно просто else.

Еще хромает оформление кода, много лишних фигурных скобок, которые можно опустить.

Спустя 1 минута, 36 секунд (29.10.2010 - 10:57) Guest написал(а):
Цитата (ApuktaChehov @ 29.10.2010 - 07:56)
Еще хромает оформление кода, много лишних фигурных скобок, которые можно опустить.

почему? что именно не правильно? а код сам рабочий? или бред?

Спустя 8 минут, 59 секунд (29.10.2010 - 11:06) ApuktaChehov написал(а):
Вот тут можно сделать так:
if( isset($_POST['Ok']) )
{
if($title and $text and $sectionid and $catid and $_FILES["images"]["name"])
{
copy($_FILES['images']['tmp_name'], $uploadfile);

$res = mysql_query ("INSERT INTO `jos_content` (`title`, `introtext`, `fulltext`, `sectionid`, `catid`, `state`, `created`, `images`)
VALUES (
'"
. mysql_real_escape_string($title) . "',
'"
. $surt . $date . mysql_real_escape_string($introtext) . "',
'"
. mysql_real_escape_string($fulltext) . "',
'"
. $sectionid . "',
'"
. $catid ."',
1,
NOW(),
'"
.$images . "'
)"
) ;

if($res)
echo '<span style="color: green">Материал успешно добавлен</span>';
}
else
echo '<span style="color: #cc0000">Заполните все поля</span>';
}


Код более читабельный и чистый, хотя функционал сохранен.

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

Спустя 47 секунд (29.10.2010 - 11:07) Basili4 написал(а):
ИМХО Код вполне на уровне.

замечание на счет elseif озвучено.

мне не очень понравилась конструкция if ($res)
мне лично ближе if ($res===fasle)
{
// Все плохо. пишем в лог и паникуем
}
else
// Все хорошо

да и проверяйте что вам там за файл передали иначе XSS
лучше всего это сделать с помощью imagecreatefromjpeg ну или другой если тип фапйла не jpg.

Суть в том что если изображение создаться то это картинка если нет попытка записать скрипт на сервер.


Спустя 59 секунд (29.10.2010 - 11:08) Guest написал(а):
а почему не идеален?

Спустя 54 секунды (29.10.2010 - 11:09) Guest написал(а):
спасибо, Basili4

Спустя 36 секунд (29.10.2010 - 11:10) Basili4 написал(а):
вместо copy
используй
move_uploaded_file

Спустя 19 секунд (29.10.2010 - 11:10) ApuktaChehov написал(а):
Хотя бы потом, что нет проверки на пришедшие изображения.
Да и move_uploaded_file я забыл, Basili4 - дело говорит.

Спустя 3 минуты, 29 секунд (29.10.2010 - 11:13) Guest написал(а):
просто загружать фото будет тока админ сайта, вот... поэтому

Спустя 35 секунд (29.10.2010 - 11:14) ApuktaChehov написал(а):
А если ломанут? все.. капец сайту?

Спустя 36 секунд (29.10.2010 - 11:15) Guest написал(а):
1. Сделать проверку изображения

2. Убрать elseif

3. Заменить copy на move_uploaded_file


и все вроде, да?

Спустя 52 секунды (29.10.2010 - 11:15) Guest написал(а):
я в .htaccess файле сделал доступ к этому файлу тока для одного ip. никто ж не сможет с другого ip подступиться к этому файлу?

Спустя 56 секунд (29.10.2010 - 11:16) Basili4 написал(а):
с другим ip нет

Спустя 2 минуты, 4 секунды (29.10.2010 - 11:18) ApuktaChehov написал(а):
Думаете IP сложно подделать? Ну это я уже в крайности ухожу. Не думаю, что кто то будет вас ломать. Мы указали на недочеты, исправлять их или нет, уже ваше дело.

Спустя 8 секунд (29.10.2010 - 11:19) Guest написал(а):
поэтому я и не сделал проверку изображения. не думаю, что их админ будет загружать шелл, снифер и т.д.))))) не мазохист

Спустя 1 минута, 13 секунд (29.10.2010 - 11:20) Guest написал(а):
ApuktaChehov, спасибо за такие замечания. приветствуется! нужно рассматривать все...

там тока три недочета, которые я указал?

Спустя 1 минута, 23 секунды (29.10.2010 - 11:21) Basili4 написал(а):
ApuktaChehov
чтоб поделать ip нужно знать какой он должен быть.

Спустя 2 минуты, 55 секунд (29.10.2010 - 11:24) ApuktaChehov написал(а):
Basili4 - Ясен перец. При желании и это узнается.


Ну, да, вроде бы только 3. Хотя сейчас придут суровый эксперты, еще чего-нибудь накопают. wink.gif

Спустя 54 секунды (29.10.2010 - 11:25) Guest написал(а):
я сам знаю, что там по мелочи много всего, но зато это все пашет) не охота с мелочами копошиться
Быстрый ответ:

 Графические смайлики |  Показывать подпись
Здесь расположена полная версия этой страницы.
Invision Power Board © 2001-2024 Invision Power Services, Inc.