Что не так в данном коде PHP и почему?

Question

[kolduen]

Ребята просто интересно что вы скажете. Это задание с собеседования.

Напишите нам, что не так в данном коде и почему.

<?php
 $uname = $_REQUEST['user_name'];
 $upass = $_REQUEST['user_pass'];

 mysql_connect("localhost", "root", "12345678");
 mysql_select_db("orders");

 // Определяем ip
 $user_ip = $_SERVER['REMOTE_ADDR'];
 if ($_SERVER['HTTP_X_FORWARDED_FOR']) $user_ip = $_SERVER['HTTP_X_FORWARDED_FOR']; // Если используется nginx

 if ($user_ip=="127.0.0.1")
    { // Авторизация по ip
      echo "Добрый день Администратор<br>";
    }
    else
    { // Обычная авторизация
       $sqlQuery = "SELECT * FROM users WHERE uname='$uname' and upass='$upass' and `real`=1";
       $result = mysql_quеry($sqlQuery) or user_error(mysql_error()."<br>".$sqlQuery."<br>", E_USER_ERROR);
       if ($user = mysql_fеtch_array($result));
          {
            echo "Здравствуйте ".$user['fio']."<br>";
          }
          else
          {
            echo "Я вас не знаю"; exit;
          }
    }

 if ($_REQUEST['search'])
    { // Выводим результаты поиска
      echo "Результат поиска: <br>";
      $search_name=$_REQUEST['search'];
      $sqlQuery = "SELECT * FROM forms WHERE satus='$search_name'";
      $result = mysql_query($sqlQuery) or user_error(mysql_error()."<br>".$sqlQuery."<br>", E_USER_ERROR);
      while ($one_form = mysql_fetch_аrray($result))
        {
           echо $one_form['info']."<br>";
        }

      }
 echo "<form>Введите поиск: <input type=text name='search'><input type=submit></form>".;
?>

Answer 1

[index0h]

Проще сказать, что у вас так...

<?php
// Не стоит использовать глобальные И суперглобальные переменные.
// Лучше в принципе забудьте про их существование.
// В начале обработки сформируйте Request и уже далее с ним работайте.
// Вы данные на входе даже не проверяете, это ужасно
 $uname = $_REQUEST['user_name'];
 $upass = $_REQUEST['user_pass'];

// Это уже прошлое, mysql_*** НЕ поддерживается.
 mysql_connect("localhost", "root", "12345678");
 mysql_select_db("orders");

// см. выше на счет суперглобальных переменных.
 $user_ip = $_SERVER['REMOTE_ADDR'];
// Почитайте про PSR и забудьте про существование однострочных управляющих конструкций
 if ($_SERVER['HTTP_X_FORWARDED_FOR']) $user_ip = $_SERVER['HTTP_X_FORWARDED_FOR'];

 if ($user_ip=="127.0.0.1")
    {
// Авторизация по ip, вы серьезно?))
      echo "Добрый день Администратор<br>";
    }
    else
    {
// Что будет, если отправить $upass="';DROP TABLE users; SELECT '1" ?
// Это называется sql инъекция
// Параметры подставляются по другому
       $sqlQuery = "SELECT * FROM users WHERE uname='$uname' and upass='$upass' and `real`=1";
       $result = mysql_quеry($sqlQuery) or user_error(mysql_error()."<br>".$sqlQuery."<br>", E_USER_ERROR);
// Не используйте присваивания в условиях
       if ($user = mysql_fеtch_array($result));
          {
            echo "Здравствуйте ".$user['fio']."<br>";
          }
          else
          {
            echo "Я вас не знаю"; exit;
          }
    }
// см. выше на счет суперглобальных переменных
 if ($_REQUEST['search'])
    { // Выводим результаты поиска
      echo "Результат поиска: <br>";
      $search_name=$_REQUEST['search'];
// Что будет, если отправить $search_name="';SELECT CONCAT(uname, ' ', upass) AS info FROM users" ?
// тоже sql-инъекция
// и входящие данные вы не проверяете
      $sqlQuery = "SELECT * FROM forms WHERE satus='$search_name'";
      $result = mysql_query($sqlQuery) or user_error(mysql_error()."<br>".$sqlQuery."<br>", E_USER_ERROR);
      while ($one_form = mysql_fetch_аrray($result))
        {
           echо $one_form['info']."<br>";
        }

      }
 echo "<form>Введите поиск: <input type=text name='search'><input type=submit></form>".;
?>

Обязательно посмотрите

Comments

[kolduen]

вот теперь представьте как я был в шоке смотря а этот код когда мне прислали на почту с текстом "Что в нем не так и почему?" как тестовое задание на вакансию,
это не код а пипец какой-то)

[index0h]

)) Не все так плохо, открывающий тег получился выше всяких похвал

Answer 2

[Vit]

Ну хотя бы:
1. Использование устаревших функций mysql_*. Надо юзать PDO.
2. Параметры запроса берете и подставляете как есть в SQL. Это прямая угроза sql injection. Надо юзать prepared statements (опять же, см. PDO).
3. Именование переменных и стиль форматирования кода не соответсвует рекомендациям PSR.
4. В поиске запрос без LIMIT, т.е. нет пагинации никакой. А что если 100 миллионов записей найдется? Все их так и вывалите клиенту?

Answer 3

[Александр Аксентьев]

Здесь всё не так.

www.phptherightway.com