Gfrktyl

I have this code to display a counter on the side of <i class="fas fa-bell mr-3"></i>. I want to know if this code is good on security and perfomance.

I just started using jquery and ajax, i had heard people saying that someone could disable the javascript and do bad things. What you guys think about my code?

 <div>

    <ul class="navbar-nav textoPerfilDesk dropMenuHoverColor">

        <li class="nav-item dropdown pr-2 dropleft navbarItem ">

        <a class="nav-link dropdown-toggle-fk" href="#" id="navbarDropdownMenuLink" role="button" data-toggle="dropdown" aria-haspopup="true" aria-expanded="false">

            <i class="fas fa-bell mr-3"></i>

        </a>

        <div class="dropdown-menu dropdown-menu-fk py-3" aria-labelledby="navbarDropdownMenuLink">

            <a class="dropdown-item dropMNitemNT" href="um-link">

            <span class="d-flex">

                <img class="imgNT" src="img/1.jpg">

                <span class="pl-2 pt-1">

                    titutlo

                </span>

            </span>

            </a>

        </div>

        </li>

    </ul>

    <span class="text-white divCountNT" id="datacount"></span>

  </div>

script:

<script>

  $(document).ready(function(){

     var intervalo, carregaDiv;

     (carregaDiv = function(){

        $("#datacount").load('select.php', function(){

           intervalo = setTimeout(carregaDiv, 1000);

        });

     })();

     $('.fa-bell').on('click', function (){

        clearTimeout(intervalo);

        $.ajax({

           url: "update.php",

           complete: function(){

             setTimeout(carregaDiv, 1000);

           }

        });

     });

  });

</script>

select.php

<?php

require_once 'db.php';



if(!isset($_SESSION))session_start();



if(isset($_SESSION['userid'])) {

  $userid = $_SESSION['userid'];

}



    $status = 'unread';



    $sql = $conn->prepare("SELECT * FROM noti WHERE status = :status AND 

    userid = :userid");

    $sql->bindParam(':userid', $userid, PDO::PARAM_INT);

    $sql->bindParam(':status', $status, PDO::PARAM_STR);

    $sql->execute();

    $countNT = $sql->rowCount();



    echo $countNT;



    $conn = null;



    ?>

update.php

<?php

require_once 'db.php';



if(!isset($_SESSION))session_start();



if(isset($_SESSION['userid'])) {

  $userid = $_SESSION['userid'];

}



$status = 'read';

$sql = $conn->prepare("UPDATE noti SET status = :status WHERE userid = :userid");

$sql->bindParam(':user_id', $userid, PDO::PARAM_INT);

$sql->bindParam(':status', $status, PDO::PARAM_STR);

$sql->execute();

$countNT = $sql->rowCount();



echo $countNT;



$conn = null;

?>

JAVASCRIPT SECURITY

Javascript is running on the client, and is therefore under full control of the user. It can be disabled, inspected, manipulated, and everything else that can done in a programming language. You knew this, didn't you?

Javascript is, almost by definition, insecure. Things that have to do with the security of your site, like validating passwords, should not be done in Javascript. And in your code you don't do anything security related in Javascript. All you do is set a timer running and call two PHP scripts. No risks there.

PHP SECURITY

The PHP scripts are another matter. Here is where things really happen, and you should implement your security measures here. Even though these scripts implement AJAX calls, they can be executed by anybody.

You seem to have users, that can log in. Their user ID is stored in $_SESSION['userid']. I notice that you don't do anything, in your PHP scripts, when this ID is absent. You still execute the database queries. That is a bad idea.

When the two current PHP scripts are called, without an user ID, they will probably just perform database queries that are invalid. No real harm done. But you shouldn't rely on just pure luck. Good security should leave no doubts about what will happen.

I therefore propose I slight change to your code. Instead of writing this:

if (isset($_SESSION['userid'])) {

  $userid = $_SESSION['userid'];

}

you could write this:

if (!isset($_SESSION['userid'])) die('Not logged in.');

$userid = $_SESSION['userid'];

this means that the PHP scripts will halt execution when there's no user, as they should.

PERFORMANCE

You code is evidently not very efficient. Polling the database every second does not scale very well. There are other ways to do this. For instance with web sockets: https://developer.mozilla.org/en-US/docs/Web/API/Websockets_API ( you would use a combination of the tools mentioned there). Updates will be quicker, without polling.

For now polling will probably be fine for you, after all you're still learning Jquery and that is a challenge in itself. It takes time to understand how everything hangs together.