-3

We are building up a doctor appointment system where patients can book and cancel appointments. We are making sure they cancel their appointments with their phone number and a message goes to them. However, in this case any phone which is typed, the message goes. Disregarding if the patient's name is there or not.

For example, if any patient in the database does not have any phone number of "9845654362" but in the cancel appointment page, I type "9845654362" in the form, the message will go to the sim. How can an appointment be cancelled if its not even there? How do we resolve this?

This is the php code for the design:

<html>
<body>
    <div class="logo">
<img src="logo1.png" height="50px" width="50px" style="float:left">
                    <a  style="font-size:40px; font-weight:bold;color:teal" >MUKUND ORTHOPAEDIC CENTER</a>
                </div>
                <br></br>
                <meta charset="utf-8">
                <meta name="viewport" content="width=device-width, initial-scale=1">
                <link rel="stylesheet" href="https://maxcdn.bootstrapcdn.com/bootstrap/3.4.1/css/bootstrap.min.css">
                <script src="https://ajax.googleapis.com/ajax/libs/jquery/3.5.1/jquery.min.js"></script>
                <script src="https://maxcdn.bootstrapcdn.com/bootstrap/3.4.1/js/bootstrap.min.js"></script>
                </head>
                <body>
                    <nav class="navbar navbar-inverse">
                        <div class="container-fluid">
                          <ul class="nav navbar-nav">
                            <li class="active"><a href="maincancel.php">Back</a></li>
                            <li class="active"><a href="abc.html">Home</a></li>
                          </ul>
                        </div>
                        </nav>
<?php


// Create connection
$conn=mysqli_connect("localhost","root","","bookingcalendar");// Check connection
if (!$conn) {
  die("Connection failed: " . mysqli_connect_error());
}

$phone= $_POST['phone'];
// sql to delete a record
$sql = "DELETE FROM bookings WHERE phone='$phone'";

if (mysqli_query($conn, $sql)) {
  echo "<h3 align=\"center\" >Your slot has been cancelled successfully!";
} else {
  echo "Error deleting record: " . mysqli_error($conn);
}

mysqli_close($conn);
$ph=(int)$phone;
      
$curl = curl_init();
$msg="Dear '$name', your appointment  with Dr.Phaniraj is cancelled for '$date'";
curl_setopt_array($curl, array(
  CURLOPT_URL => "https://www.fast2sms.com/dev/bulk?authorization=urFfV4mgSqRyNAH7M9cItCjedvYo5h8x6aDsLip3wKTO1GkzEXZYFspaQL1MAkjiPWy9GrCw34Kov5tx&sender_id=CHKSMS&message=".urlencode($msg)."&language=english&route=p&numbers=".$ph,
  CURLOPT_RETURNTRANSFER => true,
  CURLOPT_ENCODING => "",
  CURLOPT_MAXREDIRS => 10,
  CURLOPT_TIMEOUT => 30,
  CURLOPT_SSL_VERIFYHOST => 0,
  CURLOPT_SSL_VERIFYPEER => 0,
  CURLOPT_HTTP_VERSION => CURL_HTTP_VERSION_1_1,
  CURLOPT_CUSTOMREQUEST => "GET",
  CURLOPT_HTTPHEADER => array(
    "cache-control: no-cache"
  ),
));
$response = curl_exec($curl);
$err = curl_error($curl);

curl_close($curl);

if ($err) {
  echo "cURL Error #:" . $err;
} //else {
  //echo $response;
//}

      

    

 ?>


   






<br><h3><center><b>Click here to Reschedule Appointment:</b></center></h3>
<center><button><a href="index.php">Click here</a></button></center>

enter code here

Spoorthi Kulkarni
  • 9
  • 3
  • 2
    Hello and welcome to Stackoverflow! Please read [How To Ask](https://stackoverflow.com/help/how-to-ask) and then provide some code to produce [repex](https://stackoverflow.com/help/minimal-reproducible-example). It will be easier for us to debug what is wrong with your code, or what's causing this behavior. – Saud Apr 10 '21 at 05:42
  • This doesn't make a lot of sense. You seem to be deleting from a "phone no" table. That doesn't sound like appointments. So it's unclear how you think this code is cancelling any appointments? And it's unclear what the process flow is supposed to be? Is the idea that the user simply type their phone number and it would cancel any appointments related to that phone number? Or is typing the phone number the end of a longer cancellation process? Either way, the code does not check if the phone number relates to any real appointments before sending the SMS. You need a select query for that. – ADyson Apr 10 '21 at 06:51
  • Thanks for the update. Your code seems to assume there would only ever be one booking with that specific phone number. That seems unlikely - what if the person has more than one appointment (either in future or in the past)? What if two members of a family share the same phone number? You need to cancel a specific appointment by its specific ID, not by identifying it through a value which could exist in more than one row of the table. – ADyson Apr 10 '21 at 08:21
  • We have put the indication that each patient should book the appointment with a different phone number. What if they don't remember their id or have misplaced it? Also, PHONENO was a view I had created in my database so that I get all the phone numbers at one place. – Spoorthi Kulkarni Apr 10 '21 at 08:24
  • 1
    Ok but what if they don't? Unless you enforce that with a rule in the database you can't prevent duplicates. Also what if the same person books more than one appointment? Then it's totally legitimate for the same number to appear twice in your table (you can't expect people to have 2 or more phone numbers!). But probably they don't want to cancel _all_ their appointments! There should be a way to search for the correct appointment either by ID or by other personal details as well (e.g. name, date of birth, zip code etc.). – ADyson Apr 10 '21 at 09:24
  • Also who is accessing the site - the patients, or the doctor's staff? If it's the patients then you should have a login system - if you simply allow someone to cancel an appointment using a phone number or a name then there is no security - anyone who knows their phone number or other details could mess with their data. If this is a medical appointments system usable directly by patients then I think you have some pretty serious issues with privacy and data protection. – ADyson Apr 10 '21 at 09:27
  • This issue has been resolved so I guess nothing more to discuss about. – Spoorthi Kulkarni Apr 10 '21 at 10:16
  • 1
    Perhaps, but you sound defensive so I'll assume you've understood my points, but you don't want to discuss them, and that you're going to go away and fix them yourself. Otherwise I wouldn't want to be the patient who has to use this application... I don't know what country or market you are targeting this at, but be aware that in many places, medical software has to go through many layers of regulatory and security approval before it can be used by any registered health service. Your app and apparent level of knowledge are currently nowhere near what would be expected in those jurisdictions. – ADyson Apr 10 '21 at 10:54
  • We have designed the website according to what the client needs. And the client has successfully accepted it so peace. – Spoorthi Kulkarni Apr 10 '21 at 13:52
  • Probably the client hasn't realised the issues with it yet (probably because either the requirements or the testing, or both) were insufficiently thorough. The client is not always correct. You don't think it's a problem that your code could delete the wrong appointments? To be clear this is not a personal criticism of you, I just don't think this code is going to withstand the onset of the real world. – ADyson Apr 10 '21 at 14:48
  • Also the query is vulnerable to basic SQL injection attacks. If you're not aware of standard security stuff like that then you need to have your whole application reviewed in detail by a more experienced professional. Medical records + security problems / bugs = big lawsuit. I'm trying to warn you, so that you can avoid that sort of disaster. – ADyson Apr 10 '21 at 14:59
  • Thanks for the warn. The issue has been resolved as told before. You could have send the code as I have received if you were totally interested into coding and not criticism. – Spoorthi Kulkarni Apr 10 '21 at 15:06
  • `The issue has been resolved`...yes that original issue has been resolved, but none of the ones I've just pointed out have. It's not my job to re-write your whole application, I just try to help a little bit. See https://stackoverflow.com/questions/60174/how-can-i-prevent-sql-injection-in-php for detailed guidance on how to fix the SQL injection problem. The rest is an issue for you to work out a more logical UX process for your application to follow, and ensure appropriate security throughout. I'm sure this isn't the only script with similar issues. Commission a thorough review as I advised. – ADyson Apr 10 '21 at 15:23

1 Answers1

1

There are several problems in code. First of all never use raw inputs from users. You can't trust with an input from the user. Always use Prepared Statements to avoid SQL injections.

Now let's get back to your code. The problem is in your process flow. Breaking down your logic step by step:

  • Getting input from the user.
  • Deletion with respect to that input.
  • Useless check for whether the record has been deleted or not.
  • After that you're sending your SMS.

As you can see you are sending SMS after your if condition check. This means whether the record exists or not the text message will always be sent to the user.

Here's how you can do it:

<?php
// Create connection
$mysqli = new mysqli("localhost", "id16390447_mukund", "Go?QuMVC^Yx8}QzV", "id16390447_das");

// Check connection
if ($mysqli->connect_errno) {
    echo "Failed to connect to MySQL: ".$mysqli->connect_error;
    exit();
}

// sql to delete a record
$phone = $_POST['phone'];

// Using prepared statements to avoid SQL injections...
$stmt = $mysqli->prepare("DELETE FROM PHONENO WHERE phone = ?");
$stmt->bind_param("s", $phone); // put i if you're storing your phone number as an integer
$stmt->execute();

if($stmt->affected_rows){
    $ph=(int)$phone;

    $curl = curl_init();
    $msg="Your appointment with Dr.Phaniraj is cancelled.";
    curl_setopt_array($curl, array(
        CURLOPT_URL => "https://www.fast2sms.com/dev/bulk?authorization=urFfV4mgSqRyNAH7M9cItCjedvYo5h8x6aDsLip3wKTO1GkzEXZYFspaQL1MAkjiPWy9GrCw34Kov5tx&sender_id=CHKSMS&message=".urlencode($msg)."&language=english&route=p&numbers=".$ph,
        CURLOPT_RETURNTRANSFER => true,
        CURLOPT_ENCODING => "",
        CURLOPT_MAXREDIRS => 10,
        CURLOPT_TIMEOUT => 30,
        CURLOPT_SSL_VERIFYHOST => 0,
        CURLOPT_SSL_VERIFYPEER => 0,
        CURLOPT_HTTP_VERSION => CURL_HTTP_VERSION_1_1,
        CURLOPT_CUSTOMREQUEST => "GET",
        CURLOPT_HTTPHEADER => array(
            "cache-control: no-cache"
        ),
    ));

    $response = curl_exec($curl);
    $err = curl_error($curl);

    curl_close($curl);

    echo "<h3 align=\"center\" >Your slot has been cancelled successfully!";
} else {
    echo "Error deleting record: ".$stmt->error;
}

$stmt->close();

All you need to do is put your SMS send code into the if condition, so that it will only execute if the record is deleted. I've used prepared statements with OOP approach, you can use procedural if you want.

Saud
  • 859
  • 1
  • 9
  • 23