-1

I am having some issue that is facing me with running the following dismiss functions,

So page with an alert bootstrap that I have named as notifications-success.php goes as follows:

<?php 
$root = realpath(str_replace('\\', '/', $_SERVER['DOCUMENT_ROOT']) ); 
include ($root . '/insights/ss/onix.php'); 

$result = mysqli_query($mysqli,"select * from notifications where seen = 0");
    if ($result)
    {      
    if($result->num_rows) {
        while($row = mysqli_fetch_assoc($result))
        {?> 
    
<div class='alert alert-success alert-dismissible' role='alert' style='margin-left:-12px;'>
 <button type="button" class="close" onClick="updateId('<?php echo $row['id'];?>')" data-dismiss="alert" aria-label="Close" style="float:left!important; border:0; background:none;"><span aria-hidden="true">&times;</span></button>

<strong><span class="text-success" style="margin-top:-50px;"><i class='fa fa-check'></i> &nbsp; &nbsp; &nbsp; File has been moved successfully</strong><br>To confirm reading this message please press x button </span></div>

<?php       }
    }
   
}
                  ?>
                  
                  <script>
function updateId(id)
{
    var xmlhttp = new XMLHttpRequest();
    xmlhttp.open("GET", "dismisssuccess.php?id=" +id, true);
    xmlhttp.send();
}
</script>

Action file which is dismisssuccess.php goes as follows:

<?php 
if(isset($_GET['id']) && !empty($_GET['id']))
{
    $id = $_GET['id'];
    $ip = getenv('REMOTE_ADDR');
$root = realpath(str_replace('\\', '/', $_SERVER['DOCUMENT_ROOT']) ); 
include ($root . '/insights/ss/onix.php'); 

    $update = "UPDATE notifications SET seen = 1 , seenby = '$ip' WHERE id = '".$id."'";

    if (mysqli_query($mysqli, $update))
    {
              echo "success";   
 
            

    } 
    else 
    {
        echo "There is some error";
    }
    die;
}
?>

Now when I press x , the update statement doesn't actually run, meanwhile, when i open dismisssuccess file by http with relevant id it works fine with no error and does the update required, also works fine only when I change the table to be update.

Does anyone have clue what could be possible reason behind this issue?

Thank you in advance

  • What have you tried to resolve the problem? Where are you stuck? Also, be warned that your `UPDATE` query is widely open for SQL injection – Nico Haase Oct 21 '21 at 09:45
  • Your generated HTML markup is incorrectly nested .. one portion begins `
    To confirm reading this message please press x button
    `
    – Professor Abronsius Oct 21 '21 at 09:46
  • Also - you ajax function does nothing with any returned content so why bother generating any in php? – Professor Abronsius Oct 21 '21 at 09:49
  • **Warning:** You are wide open to [SQL Injections](https://php.net/manual/en/security.database.sql-injection.php) and should use parameterized **prepared statements** instead of manually building your queries. They are provided by [PDO](https://php.net/manual/pdo.prepared-statements.php) or by [MySQLi](https://php.net/manual/mysqli.quickstart.prepared-statements.php). Never trust any kind of input! Even when your queries are executed only by trusted users, [you are still in risk of corrupting your data](http://bobby-tables.com/). [Escaping is not enough!](https://stackoverflow.com/q/5741187) – Dharman Oct 21 '21 at 10:51
  • @NicoHaase what i have tried are 1. running action file using http and target id and update stmt have worked only seperately from notification file 2. changing target table to be updated also have made my code to be fully working – Ryham Ali Maher Abouelnour Oct 21 '21 at 11:57
  • my database is protected by another service so my aim is not to work on securing sql injection as this is already handled by another service, and so I only write down simply in that way as in above – Ryham Ali Maher Abouelnour Oct 21 '21 at 12:01

1 Answers1

0

Tweak the PHP & HTML so that the nesting is correct and assign a new dataset attribute to the button rather than the inline event handler.

<?php

    $root = realpath(str_replace('\\', '/', $_SERVER['DOCUMENT_ROOT']) ); 
    include ($root . '/insights/ss/onix.php'); 

    $result = mysqli_query($mysqli,"select * from notifications where seen = 0");
        if ($result){
            if($result->num_rows) {
                while($row = mysqli_fetch_assoc($result)){
?> 
    
<div class='alert alert-success alert-dismissible' role='alert' style='margin-left:-12px;'>
    <button type="button" class="close" data-id="<?=$row['id'];?>" data-dismiss="alert" aria-label="Close" style="float:left!important; border:0; background:none;">
        <span aria-hidden="true">&times;</span>
    </button>

    <strong>
        <span class="text-success" style="margin-top:-50px;">
            <i class='fa fa-check'></i>
            &nbsp;&nbsp;&nbsp;File has been moved successfully
        </span>
    </strong>
    <br>
    To confirm reading this message please press X button 
</div>

<?php
           }
        }
    }
?>

Use an externally registered event handler and why not use the fetch api ~ appears slightly shorter and is a better api moving forwards.

<script>
    function updateId(e){
        e.stopPropagation();
        let id=e.target!=e.currentTarget ? e.target.parentNode.dataset.id : e.target.dataset.id;
        fetch( 'dismisssuccess.php?id='+id )
            .then(r=>r.text())
            .then(text=>console.log(text))
    }
    document.querySelectorAll('div[role="alert"] button[data-id]').forEach(bttn=>bttn.addEventListener('click',updateId))
</script>

Within the PHP you really, really should use a prepared statement when dealing with user supplied data - otherwise all your hard work could be undone by one malicious user!

<?php 

    if( !empty( $_GET['id'] ) ){

        $id = $_GET['id'];
        $ip = getenv('REMOTE_ADDR');
        
        $root = realpath(str_replace('\\', '/', $_SERVER['DOCUMENT_ROOT']) );
        include ($root . '/insights/ss/onix.php'); 

        $sql='UPDATE `notifications` SET `seen`=1, `seenby`=? where `id`=?';
        
        $stmt=$mysqli->prepare($sql);
        $stmt->bind_param('ss',$ip,$id);
        $stmt->execute();
        $rows=$stmt->affected_rows;
        $stmt->close();
        
        exit( $rows ? 'Success' : 'There is some error' );
    }
?>
Professor Abronsius
  • 33,063
  • 5
  • 32
  • 46