- 01
- 02
- 03
- 04
- 05
- 06
- 07
- 08
- 09
- 10
- 11
- 12
- 13
- 14
- 15
- 16
- 17
- 18
- 19
- 20
- 21
- 22
- 23
- 24
- 25
- 26
- 27
- 28
- 29
- 30
case"register": // если do=register, выводим регистрацию
if (isset($login) && isset($pass1) && isset($pass2)) {
if (!empty($login) && !empty($pass1) && !empty($pass2)) {
$users=get_serial('users');
$reallogin=$login;
$login=md5(strtolower($login));
if (!$users[$login]) {
if (strlen($pass1)>=4) {
$pass1=md5($pass1);
$pass2=md5($pass2);
if ($pass1==$pass2) {
$users[$login]=array();
$users[$login]['login']=htmlspecialchars($reallogin);
$users[$login]['pass']=$pass1;
set_serial($users,'users');
$error="Вы успешно зарегистрированны";
header("Refresh:3;url=".$_SERVER['PHP_SELF']);
}else {
$error="Ошибка: Пароли не совпадают";
}
}else {
$error="Ошибка: Минимальная длина пароля 4 символа";
}
} else {
$error="Ошибка: Такой пользователь уже существует";
}
}else {
$error="Ошибка: Обязательные поля нужно заполнить";
}
}
Lure Of Chaos 21.08.2011 16:00 # +6
1. один большой index.php на все случаи жизни, с главным "циклом жизни" switch-case
2. условноисключающая валидация if-else
3. сначала проверка на isset - и тут же на !empty
4. непоследовательный API в get_serial('users') и set_serial($users,'users')
5. куча ненужных md5 без засолки
6. куча лишних "выделений", как в 5ой или 12ой строке
7. двойные кавычки *WALL* для простых строк
8. НАСТОЯЩИЙ логин! хотя, может, и имеет смысл кодировать в базу и логин?
9. 17ая строка заслуживает особого внимания
9.1. редирект не через Location, а через Refresh
9.2. редирект со всеми GETными параметрами - в случае form method="GET" вероятно К.З.
9.3. и КОНТРОЛЬНЫЙ: PHP_SELF уязвимость....
я ничего не забыл? )))
7ion 22.08.2011 01:30 # +1
>сначала проверка на isset - и тут же на !empty
Ого, а у нас теперь существование переменной - уже знак того, что она не пуста?
>КОНТРОЛЬНЫЙ: PHP_SELF уязвимость....
Что? Гугл про уязвимость в $_SERVER['PHP_SELF'] знает только одну страницу - эту.
Lure Of Chaos 22.08.2011 11:03 # 0
if(isset($_REQUEST['login'])) $login=$_REQUEST['login'];
и куча подобного (может, $_GET или $_POST)
конструкция empty также не кидает нотисов, если переменная не определена, поэтому можно приравнять "не существует" и "пуста"
7ion 22.08.2011 12:24 # 0
Lure Of Chaos 22.08.2011 18:47 # 0
7ion 22.08.2011 19:25 # 0
Lure Of Chaos 22.08.2011 20:35 # 0
Vasiliy 22.08.2011 08:22 # 0
guest 26.08.2011 20:09 # 0
Lure Of Chaos 26.08.2011 20:10 # 0
CPPGovno 26.08.2011 22:21 # 0
Lure Of Chaos 21.08.2011 16:08 # 0
This is obvious 21.08.2011 17:17 # +1
http://govnokod.ru/2545
7ion 22.08.2011 01:31 # +5
eth0 22.08.2011 11:05 # +1
Но главная беда тут в юзабилити. За такие интерфейсы, где одно опущенное поле показывает одну ошибку и сбрасывает форму - надо убивать.
Проверять нужно все значения в форме, диагностические сообщения должны быть для каждого поля, форма сбрасываться не должна (возможно, за исключением поля "пароль", но это вопрос спорный).
Lure Of Chaos 22.08.2011 11:23 # +1
roman-kashitsyn 22.08.2011 11:27 # +1
Сидимо, где-то глубоко в мозгу автора отпечаталось, что работать с паролями напрямую - зло, нужно всегда использовать хэши. На всякий случай.
rO_ot 23.08.2011 14:23 # 0
с намеком:)
RaZeR 24.08.2011 12:14 # 0