0
\$\begingroup\$

I am concerned about how certain processes of an exercise or CRUD example should be implemented in a PHP document, looking for the best practices in terms of security and development techniques of that script.

Suddenly, my question may be considered as opinion based, so I will try to be as specific as possible:

Approach

I need to make multiple processes to my database in MySQL, in a single request made to the PHP script. A person is trying to enter a section of a security company, so that each section or room of the company has a limit marked by the user:

Fictitious Database Tables

|--------------------------|---------------------------|
|         person           |          room             |
|--------------------------|---------------------------|
|id_person                 |id_room                    |
|name_person               |name_room                  |
|document_person           |code_room                  |
|date_admission_person     |number_room                |
|id_document_type_person   |capacity_room              |
|id_blood_type_person      |current_capacity_room      |
|id_room_person            |id_state_room              |
|--------------------------|---------------------------|

The capacity of the room is 10 people, and the current capacity of the room is 8 people.

Process

  • Validate if the room has the capacity to register.
  • Enter the record of a person in a table called person.
  • Update the current value of the room you are visiting current_capacity_room.

There are three different procedures that must be performed in the same script:

  1. READ
  2. CREATE
  3. UPDATE

Sample Script

<?php
    //Declaration of system headers
    header("Context-type: application/json;");
    //includin connection with the db
    require '../Connection/connection.php';
    //Declaration of sessions
    session_start();
    $idCompany = $_SESSION['user']['id_company'];
    //Decoding data by POST method
    $_POST = json_decode(file_get_contents('php://input'), true);
    //Declaration array for encode JSON format
    $result= array();
    //Evaluate if the connection it's ok
    if ($mysqli)
    {
      if(isset($_POST) && !empty($_POST))
      {
          //Object format to UTF8
          $mysqli->set_charset('utf8');
          //Capturing variables
          $namePerson           = $_POST['name_person'];
          $documentPerson       = $_POST['document_person'];
          $idDocumentTypePerson = $_POST['id_document_type_person'];
          $idBloodTypePerson    = $_POST['id_blood_type_person'];
          $idRoomPerson         = $_POST['id_room_person'];

          /*
          *****************************************************************************
          *****************************************************************************
                 PREPARE QUERY TO VERIFY THE CAPACITY OF THE ROOM
          *****************************************************************************
          *****************************************************************************
          */

          /* Query SQL */
          $consult = $mysqli->prepare("SELECT *
                                          FROM room
                                         WHERE id_room = ?
                                           AND current_capacity_room < capacity_room
                                           AND id_company = ?");

          /* check if the query was prepared correctly */
          if ($consult === false)
          {
             $result['message'] = "Failed to prepare the query.";
          }

          /*assign the first "?"*/
          $consult->bind_param('ii', $idRoomPerson, $idCompany);

          /* check if the query was executed correctly */
          if ($consult->execute() === false)
          {
            $result['message'] = "Error checking the execution of the query.";
          }

          /* Here we get the record (if there is one) */
          if ($consult->fetch() !== true)
          {
            $result['message'] = "The room does not have enough capacity to register.";
          }
          else
          {
            $consult->next_result(); // Dump the extra resultset.
            $consult->free_result(); // Does what it says.

            /*
            *****************************************************************************
            *****************************************************************************
                                 REGISTER THE PERSON IN THE SYSTEM
            *****************************************************************************
            *****************************************************************************
            */

            $consult_sql = "INSERT INTO `person`(`id_person`, `name_person`, `document_person`,
                                                   `date_admission_person`, `id_document_type_person`, `id_blood_type_person`,
                                                   `id_room_person`)
                             VALUES (NULL, ?, ?, CURDATE(), ?, ?, ?)";


            $insert_person = $mysqli->prepare($consult_sql);
            /*
               * Use the method bind_param() to capture the variables in the Database
            */
            $insert_person->bind_param('ssiii', $namePerson,
                                                $documentPerson,
                                                $idDocumentTypePerson,
                                                $idBloodTypePerson,
                                                $idRoomPerson);

            //Execute query
            if ($insert_person->execute())
            {
              $insert_person->next_result(); // Dump the extra resultset.
              $insert_person->free_result(); // Does what it says.
              /*
                 * Verify if there were affected rows
                 * Decide the status of the message with a ternary operator
              */
              /*
              *****************************************************************************
              *****************************************************************************
                          UPDATE THE CAPACITY OF THE ROOM IN THE SYSTEM
              *****************************************************************************
              *****************************************************************************
              */
              $consult_sql = "UPDATE `room`
                                  SET `current_capacity_room` = current_capacity_room + 1
                                WHERE id_room` = ?;";

              $update_room = $mysqli->prepare($consult_sql);
              /*
                 * Use the method bind_param() to capture the variables in the Database
              */
              $update_room->bind_param('i', $idRoomPerson);

              //Execute query
              if ($update_room->execute())
              {
                $insertedRows = $update_room->affected_rows;
                $message = ($insertedRows > 0) ? "The person in the system has been correctly registered." : "It was not possible to register the person, try again later." . $update_room->error;
                $result['message'] = $message;
              }
              else
              {
                $result['message'] = "An error has occurred: " . $mysqli->error;
              }

            }
            else
            {
              $result['message'] = "An error has occurred: " . $mysqli->error;
            }
          }
      }
      else
      {
          $result['message'] = "The request with the respective POST method is inappropriate for the URL visited.";
      }
    }
    else
    {
        $result['message'] = "The connection could not be established.";
    }
    echo json_encode($result);
   ?>

Questions

  1. Is this the correct way to perform the multiple processes shown in the script?
  2. If a process fails, what is expected is that at some point modifications are made to the database, but does not allow it to be an integral way. Let me explain: It is consulted, it is registered but it is not updated by x or and reason, the script failed, so at the time it would not register the increase in the room. What would be the ideal way to guarantee the executions and if one fails to undo the changes?
  3. What is there to improve in the script?
  4. In terms of security, how vulnerable can it be?

Improvements

  1. By A. Cedano's suggestions I made the respective change die('SQL Error:'.$query->error); to be controlled by a message in JSON.

  2. The conditional <= a < of the first query is changed since, being less or equal, it would enable the person to pass, leaving the current_capacity_room in a value greater than the capacity_room at the moment of updating. That is, if the visit were at its maximum capacity (12) and was to consult for the access of another person, being 12 <= a 12 would enable the step and would increase one person leaving the current_capacity_room in 13.

\$\endgroup\$

closed as off-topic by Toby Speight, 200_success, t3chb0t, 23fc9a62-56de-47fb-97b4-737890, Heslacher Nov 29 '18 at 5:17

This question appears to be off-topic. The users who voted to close gave these specific reasons:

  • "Code not implemented or not working as intended: Code Review is a community where programmers peer-review your working code to address issues such as security, maintainability, performance, and scalability. We require that the code be working correctly, to the best of the author's knowledge, before proceeding with a review." – 200_success, 23fc9a62-56de-47fb-97b4-737890, Heslacher
  • "Lacks concrete context: Code Review requires concrete code from a project, with sufficient context for reviewers to understand how that code is used. Pseudocode, stub code, hypothetical code, obfuscated code, and generic best practices are outside the scope of this site." – Toby Speight, t3chb0t
If this question can be reworded to fit the rules in the help center, please edit the question.

  • \$\begingroup\$ Who is A. Cedano? And why a client should be concerned about prepare() function's failure? \$\endgroup\$ – Your Common Sense Nov 28 '18 at 17:20
  • \$\begingroup\$ You are asking how to ensure consistency when multiple SQL operations are involved, but your code currently doesn't do that correctly, which makes your question off-topic for Code Review. (See the help center.) Hint: you want to use transactions. \$\endgroup\$ – 200_success Nov 28 '18 at 19:18
2
\$\begingroup\$

In general, there is A LOT of unnecessary and useless code, the whole stuff could be written in a few lines

<?php
header("Context-type: application/json;");
require '../Connection/connection.php';
session_start();
$idCompany = $_SESSION['user']['id_company'];
$_POST = json_decode(file_get_contents('php://input'), true);
$result= array();

if($_POST)
{
    $consult = $mysqli->prepare("SELECT 1 FROM room WHERE id_room = ? 
        AND current_capacity_room < capacity_room AND id_company = ?");
    $consult->bind_param('ii', $_POST['id_room_person'], $idCompany);
    $consult->execute();
    if ($consult->fetch() !== true)
    {
        $result['message'] = "The room does not have enough capacity to register.";
    } else {
        $sql = "INSERT INTO `person`(`id_person`, `name_person`, `document_person`,
           `date_admission_person`, `id_document_type_person`,
           `id_blood_type_person`, `id_room_person`)
            VALUES (NULL, ?, ?, CURDATE(), ?, ?, ?)";
        $stmt = $mysqli->prepare($consult_sql);
        $insert_person->bind_param('ssiii', $_POST['name_person'],
                                            $_POST['document_person'],
                                            $_POST['id_document_type_person'],
                                            $_POST['id_blood_type_person'],
                                            $_POST['id_room_person']);
        $insert_person->execute();
        $result['message'] = "The person in the system has been correctly registered.";
    }
}
echo json_encode($result);

Basically, to improve this code, you must

  • move setting the db charset into connection.php and also set the proper error reporting for mysqli. See a correct example for connection.php in my article, How to properly connect to Mysql database using mysqli.
  • set up a proper error handler for your code. A a simple error handler for JSON API can be seen also in my other article.
  • as a result, remove all laborious error checking code blocks
  • remove the code that is just useless, like
    • if ($mysqli) an error handler should handle this
    • isset($_POST) it makes no sense to test if a variable is set if it was defined two lines above
    • !empty($_POST) same
    • $consult->next_result() makes no sense. there is no extra resultset
    • $consult->free_result() useless
    • ... etc.
  • such a prolific commenting is also considered a bad practice. A skilled in Engish programmer could tell that a code require '../Connection/connection.php'; does include a connection with the db.

But what you should really think of is a race condition when inbetween your select and insert queries squeezed another select query and you will have 9 spaces occupied. Answers can be found on Stack Overflow.

\$\endgroup\$
  • \$\begingroup\$ You're right, but I have a little question, what can I do with the query that update data on the same script that you've written? \$\endgroup\$ – jecorrales Nov 28 '18 at 19:22
  • \$\begingroup\$ Sorry I don't quite get what you mean \$\endgroup\$ – Your Common Sense Nov 28 '18 at 19:47
  • \$\begingroup\$ Look my code and I've written three different operations, READ, INSERT and UPDATE. In your script you just wrote two of them (READ and INSERT)... So part, I'm trying to improve my code and how to ensure consistency when multiple SQL operations are involved? \$\endgroup\$ – jecorrales Nov 28 '18 at 19:53
  • \$\begingroup\$ Ah sorry. Indeed I missed it. To ensure the consistency transactions are used. They guarantee that wither all or none statements were successfully executed. \$\endgroup\$ – Your Common Sense Nov 28 '18 at 19:56
  • 1
    \$\begingroup\$ I've got an example for PDO transactions, for mysqli it's pretty much the same. But really it's off topic already. Try to google your question \$\endgroup\$ – Your Common Sense Nov 28 '18 at 20:10

Not the answer you're looking for? Browse other questions tagged or ask your own question.