» WAP Мастер Форум   https://forum.wapinet.ru/index.php
» PHP   https://forum.wapinet.ru/viewforum.php?id=3
» Учимся модернизировать скрипты на конкретном примере   https://forum.wapinet.ru/viewtopic.php?id=5

Admin » 2007.11.03 03:27

Имеется скрипт сервиса гостевых книг. При первом взгляде сразу бросается в глаза его не оптимизированность. Т.е. расходуется слишком много системных ресурсов.
Например имеется код

Код:

1
2
3
4
5
6
echo "<?xml version=\"1.0\" encoding=\"UTF-8\"?>\n";
echo "<!DOCTYPE wml PUBLIC \"-//WAPFORUM//DTD WML 1.2//EN\" \"http://www.wapforum.org/DTD/wml12.dtd\">\n";
echo "<wml>\n";
echo "<head><meta http-equiv=\"Cache-Control\" content=\"no-cache\" forua=\"true\"/></head>\n";
echo "<card id=\"index\" title=\"Сервис Гостевых\">\n";
echo "<p align=\"center\" mode=\"wrap\">\n";

если его переписать следующим образом

Код:

1
2
3
4
5
6
print '<?xml version="1.0" encoding="UTF-8"?>
<!DOCTYPE wml PUBLIC "-//WAPFORUM//DTD WML 1.2//EN" "http://www.wapforum.org/DTD/wml12.dtd">
<wml>
<head><meta http-equiv="Cache-Control" content="no-cache" forua="true"/></head>
<card id="index" title="Сервис Гостевых">
<p align="center" mode="wrap">';

то это даст небольшой прирост производительности, а главное код воспринимается легче, ИМХО.

Далее, в скрипте используется постоянное соединение с MySql, я думаю все же лучше заменить mysql_pconnect на mysql_connect
заменяем

Код:

1
$link = @mysql_pconnect ($MySQL_Hostname, $MySQL_Username, $MySQL_Password);

на

Код:

1
$link = @mysql_connect ($MySQL_Hostname, $MySQL_Username, $MySQL_Password);

Так же можно изменить имена переменных на более короткие, и вообще сделать их константами.
Это так же даст небольшой прирост производительности
В файле functions.php видим функцию транслита, сделано с помощью 65 функций str_replace
Этот код так же можно переписать следующим образом

Код:

1
2
3
4
5
6
7
function trans($tr)
{
return str_replace(
array('YA','Ya','ya','yee','YO','yo','Yo','ZH','zh','Zh','Z','z','CH','ch','Ch','SH','sh','Sh','YE','ye','Ye','YU','yu','Yu','JA','ja','Ja','A','a','B','b','V','v','G','g','D','d','E','e','I','i','J','j','K','k','L','l','M','m','N','n','O','o','P','p','R','r','S','s','T','t','U','u','F','f','H','h','W','w','q','Y','y','C','c','X','x'),
array('Я','Я','я','ые','Ё','ё','Ё','Ж','ж','Ж','З','з','Ч','ч','Ч','Ш','ш','Ш','Э','э','Э','Ю','ю','Ю','Я','я','Я','А','а','Б','б','В','в','Г','г','Д','д','Е','е','И','и','Й','й','К','к','Л','л','М','м','Н','н','О','о','П','п','Р','р','С','с','Т','т','У','у','Ф','ф','Х','х','Щ','щ','ь','Ы','ы','Ц','ц','Х','х'),
$tr);
}

точно то же самое с заменой смайлов в файле convert_to_smiles.php

Admin » 2007.11.03 03:31

далее смотрим довольно интересный файл filtro.php приведу его содержание здесь

Код:

1
span style="color: #0000BB"><?phpfunction filtro($a){$a = str_replace('$','$$',$a);$a = stripslashes($a);$a = htmlspecialchars($a,ENT_QUOTES);$a = mysql_escape_string($a);$a = strtr($a,array(chr("0")=>"",chr("1")=>"",chr("2")=>"",chr("3")=>"",chr("4")=>"",chr("5")=>"",chr("6")=>"",chr("7")=>"",chr("8")=>"",chr("9")=>"",chr("10")=>"",chr("11")=>"",chr("12")=>"",chr("13")=>"",chr("14")=>"",chr("15")=>"",chr("16")=>"",chr("17")=>"",chr("18")=>"",chr("19")=>"",chr("20")=>"",chr("21")=>"",chr("22")=>"",chr("23")=>"",chr("24")=>"",chr("25")=>"",chr("26")=>"",chr("27")=>"",chr("28")=>"",chr("29")=>"",chr("30")=>"",chr("31")=>""));$a = addslashes($a);$a = trim($a);return $a;}?>

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

Код:

1
span style="color: #0000BB"><?phpfunction filtro($a){$a = str_replace('$','$$',$a);$a = mysql_escape_string(trim(stripslashes(htmlspecialchars($a,ENT_QUOTES))));$a = strtr($a,array(chr(0)=>'',chr(1)=>'',chr(2)=>'',chr(3)=>'',chr(4)=>'',chr(5)=>'',chr(6)=>'',chr(7)=>'',chr(8)=>'',chr(11)=>'',chr(12)=>'',chr(14)=>'',chr(15)=>'',chr(16)=>'',chr(17)=>'',chr(18)=>'',chr(19)=>'',chr(20)=>'',chr(21)=>'',chr(22)=>'',chr(23)=>'',chr(24)=>'',chr(25)=>'',chr(26)=>'',chr(27)=>'',chr(28)=>'',chr(29)=>'',chr(30)=>'',chr(31)=>''));return trim(addslashes($a));}?>

1) Цифры следует писать без кавычек, что мы и сделали
2) Если читабельность кода не сильно пострадает, то лучше несколько функций обьеденить в одну конструкцию, тоже сделали
3) Убрали из замены символы chr(9), chr(10), chr(13) - это символы табуляции, они вполне могут пригодиться в дальнейшем и опасности в себе не несут.
4) Еще можно добавить функцию nl2br - она заменит переносы строк на <br />

Т.к. в переменной $a могут находиться довольно много символов, то обрабатывать ее регулярным выражением ее не будем. Но если это, например, пароль или логин, то лучше проверять их регулярными выражениями. Об этом чуть позже.

Admin » 2007.11.03 03:34

Дальше будем изучать "основной" файл скрипта - index.php
В начале кода видим следующую конструкцию

Код:

1
2
3
$ref = $_GET["ref"];
...
$ref=rand(10000,1000000);

Спрашивается, зачем $ref = $_GET["ref"];, если дальше мы все равно присвоим переменной $ref значение от rand(10000,1000000); ?
$ref = $_GET["ref"]; убираем.
Еще дальше видим код

Код:

1
2
3
4
$id = $_GET["id"];
$id = filtro($id);
$id = intval($id);
$id = filtro($id);

здесь абсолютно не нужно! Ведь далее мы все равно переменной $id присвоим целочисленное значение с помощью intval
Заменяем весь этот код на

Код:

1
$id = intval($_GET['id']);

Смотрим дальше

Код:

1
$result=mysql_query("select * from users2 where id='$id' limit 1;");

Здесь помимо проблем с оптимизацией видим проблемы со стилем... ( хотя, смотря на этот скрипт, мы это видим постоянно =D )
Переписываем так

Код:

1
$result = mysql_query('SELECT * FROM `users2` WHERE id = '.$id.' LIMIT 1');

Поскольку ранее переменной $id мы присвоили тип int, то и передавать эту переменную следует как int, так же SQL комманды принято писать В ВЕРХНЕМ РЕГИСТРЕ. Очень советую этого придерживаться.

Admin » 2007.11.03 03:40

Код:

1
$data=mysql_fetch_array($result);

т.к. в дальнейшем не предполагается использовать нумерованный массив, а только ассоциативный, заменяем mysql_fetch_array на mysql_fetch_assoc

далее файл регистрации - reg.php

Код:

1
2
3
4
5
6
7
$title = filtro($_POST["title"]);
$msgs = filtro($_POST["msgs"]);
$pass = filtro($_POST["pass"]);
$email = filtro($_POST["email"]);
$link = filtro($_POST["link"]);
$sitename = filtro($_POST["sitename"]);
$razdel = filtro($_POST["razdel"]);

Очень интересно, все переменные обрабкатываются функцией filtro, которую мы рассмотрели ранее.
Но ведь не бывает таких универсальных решений, которые бы корректно обрабатывали и e-mail, и пароли, и ссылки, и просто какой-нибудь текст! В данном случае этот код необходимо переписать обязательно!
Я бы сделал примерно так

Код:

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
$title = substr(filtro($_POST['title']),0,25);
$sitename = substr(filtro($_POST['sitename']),0,25);
$razdel = substr(filtro($_POST['razdel']),0,10);
$msgs = substr(intval($_POST['msgs']),0,2);
 
if(!eregi("^[[:alnum:]][a-z0-9_^\.-]*@[a-z0-9^\.-]+\.[a-z]{2,6}$", $_POST['email']))
{exit('Error');}
else
{$email = trim($_POST['email']);}
 
if(!preg_match("/^http:\/\/+[a-zA-Z_0-9-^\.]+\.[a-z0-9]{2,6}+$/i", $link))
{exit('Error');}
else
{$link = trim($_POST['link']);}
 
$pass = trim($_POST['pass']
if(!preg_match("/^([a-zA-Z_0-9]+)$/i", $pass))
{exit('Error');}
else
{
if(strlen($pass) < 4)
{exit('Error');}
else
{$pass = substr($pass),0,25);}
}
}


$title = substr(filtro($_POST['title']),0,25);
$sitename = substr(filtro($_POST['sitename']),0,25);
$razdel = substr(filtro($_POST['razdel']),0,10);

Здесь переменные фильтруются через функцию filtro
$msgs = substr(intval($_POST['msgs']),0,2); - это количество сообщений в гостевой на страницу, соответственно цифра, поэтому используем intval
Далее регулярки
if(!eregi("^[[:alnum:]][a-z0-9_^\.-]*@[a-z0-9^\.-]+\.[a-z]{2,6}$", $_POST['email']))
{exit('Error');}
else
{$email = trim($_POST['email']);}

if(!preg_match("/^http:\/\/+[a-zA-Z_0-9-^\.]+\.[a-z0-9]{2,6}+$/i", $link))
{exit('Error');}
else
{$link = trim($_POST['link']);}

$pass = trim($_POST['pass']
if(!preg_match("/^([a-zA-Z_0-9]+)$/i", $pass))
{exit('Error');}
else
{
if(strlen($pass) < 4)
{exit('Error');}
else
{$pass = substr($pass),0,25);}
}

E-mail и ссылка фильтруются через часто используемые регулярные выражения, можете скопировать их и использовать в своих скриптах.
Пароль подвергнется жестким репрессиям, в нем можно использовать только буквы латинского алфавита, цифры и знаки - и _
Так же пароль должен быть не менее 4 символов.

Admin » 2007.11.03 03:41

Код:

1
2
3
4
5
if($title == "") $error = $error."<strong>Вы не ввели заголовок в гостевой!!</strong><br/>";
if($msgs == "") $error = $error."<b>Вы не ввели кол-во сообщений на страницу!</b><br/>";
if($pass == "") $error = $error."<b>Вы не ввели пароль!!</b><br/>";
if($link == "") $error = $error."<b>Вы не ввели адрес вашего сайта!</b><br/>";
if($razdel == "") $error = $error."<b>Вы не ввели разделитель сообщений!!</b><br/>";

в данном случае лучше заменить конструкцию if($var == "") $error = $error."..."; на if(!$var) $error = $error.'...';
Т.к. если оставить как было, если ввести к примеру заголовок 0, то проверка пройдет успешно, хотя в базу данных НИЧЕГО не занесется, а нам нужно чтобы заголовок был.

Код:

1
2
3
4
5
6
7
8
$msgs = intval($msgs);
$title=str_replace('$','$$',$title);
$msgs=str_replace('$','$$',$msgs);
$link=str_replace('$','$$',$link);
$pass=str_replace('$','$$',$pass);
$email=str_replace('$','$$',$email);
$razdel=str_replace('$','$$',$razdel);
$sitename=str_replace('$','$$',$sitename);

Оставляем следующее

Код:

1
2
3
$title=str_replace('$','$$',$title);
$razdel=str_replace('$','$$',$razdel);
$sitename=str_replace('$','$$',$sitename);

И то только потому что мы работаем с WML

Admin » 2007.11.03 03:42

Панель администрирования из данного скрипта будет рассмотрена чуть позднее, если у Вас есть еще какие-либо замечания, то обязательно пишите! =)

Admin » 2007.11.03 03:46

Продолжим =)

Еще такое замечание, в скрипте везде явно прописан путь на главную сайта, но лучше для таких целей использовать переменную $_SERVER['HTTP_HOST'];

Файл admin.php из админки... лично я бы вообще там все переписал... но поскольку мне за это не никто не заплатит ограничусь несколькими замечаниями =)

1) Переменные $id и $mid - должны быть целым числом, следовательно используем intval и все! Так же они к нам приходят только через GET запрос, следовательно заменяем REQUEST на GET. Это же касается всех остальных переменных. Если они передаются через POST - пишем POST, если через GET - пишем GET. Это несколько усложнит потенциальные попытки взлома каким-нибудь нехорошим человеком (хакером)

Код:

1
2
$id = intval($_GET['id']);
$mid = intval($_GET['mid']);

2) Вот этот код убрать:

Код:

1
$ref = $_REQUEST["ref"];

3) Вот это тоже убрать:

Код:

1
2
3
global $addr;
...
global $agent;

4) Вот это

Код:

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
if(mysql_affected_rows()==0)
{
echo <<<END
<?xml version="1.0" encoding="UTF-8"?>
<!DOCTYPE wml PUBLIC "-//WAPFORUM//DTD WML 1.1//EN" "http://www.wapforum.org/DTD/wml_1.1.xml">
<wml>
<card id="search" title="Gpanel">
<p align="center">
Пароль неверен
</p>
</card>
</wml>
END;
return 0;
}

лучше переписать следующим образом

Код:

1
2
3
4
5
6
7
8
9
10
11
12
if(mysql_affected_rows()==0)
{
exit('<?xml version="1.0" encoding="UTF-8"?>
<!DOCTYPE wml PUBLIC "-//WAPFORUM//DTD WML 1.1//EN" "http://www.wapforum.org/DTD/wml_1.1.xml">
<wml>
<card id="err" title="Error" ontimer="../in.php"><timer value="1"/>
<p align="center">
Пароль неверен
</p>
</card>
</wml>');
}

<<<END
...
END;
Не очень читабельная конструкция. И для лучшей совместимости с мозгом перепишем ее.

5) Все переменные в скрипте проверяются всего 1 функцией - filtro. Как было сказано ранее, не существует функций, которые бы корректно проверяли ВСЕ данные. К каждой переменной необходим свой подход, к целочисленным - intval ( кстати не забываем что, скажем, -5 это тоже число, только отрицательное, это надо учитывать ;), если в переменной должна содержаться какая-либо типизованная информация, то проверяем ее регулярными выражениями (E-mail, URL и т.д.), переменные в которых может содержаться большое кол-во символов, например сообщение в гостевой книге, можно фильтровать функциями типа filtro, а можно придумать соответствующее регулярное выражение, это уже рещать Вам.

Admin » 2007.11.03 03:46

Вобщем здесь я привел лишь основные недоработки этого скрипта и если копать его глубже, то наверняка найдется еще много недоработок.
Знаете как определяется настоящий срок и для написания какого-либо скрипта?
Предпологаемый срок * 2 + 3 дня + 1 ночь
Вот так вот =) я это к тому, что любой скрипт можно дорабатывать и дорабатывать, было бы желание =)

Casper » 2008.01.22 15:20

Помоги пожалуйста с загруз-центром. Там почему то индексная страница ужасно долго загружается, а с Opera Mini вообще вместо станицы вот это получается -
Error

Could not locate remote server  http://dowmloads.smartinet.org/ Вот индекс http://wap.smartinet.org/downfile.zip

Gemorroj » 2008.01.23 12:35

Вобщем я начал переписывать, потом забросил, т.к. думаю бесполезно это, чтобы добиться приличного прироста скорости следует весь скрипт переписывать. Я так понял он на register_globals On работает? Еще там ошибки в HTML коде. Ну и вообще код корявый.

Casper » 2008.01.23 17:02

Да, php_value register_globals On
Очень-очень жаль=(

Ekcnepm » 2008.07.29 18:24

Паш,помоги со скриптом,в смысле с PHP.
Значит так..есть папка theme в ней файл темы default.php.есть в корне два файла http://cmswap.ru/config.txt http://cwswap.ru/index.txt нужно чтобы $theme передавалась в индеексный файл как тема ..я в ПХП еще не очень..как сделать?

Gemor » 2008.07.29 23:19

какое отношение ЭТОТ вопрос имеет к ЭТОЙ теме?

САНЯ » 2008.07.30 01:57

Как писать грабб

Gemorroj » 2008.07.30 03:15

а ЭТОТ!? =(((

denich » 2008.07.30 18:24

гг

Vliker » 2009.11.11 16:38

Admin, почему не выложил подправленный скрипт сервис гостевых книг гг?