-3

I am using the PHP below with a form that I am using to determine whether a user has authorized access to the next page.

However, I have to fill out the form at least twice for the information to go through. If I enter in the correct information on the first attempt, it will say give the appropriate error statement I coded in, then if I put literally anything into the form and hit submit, it will go through.

I can put incorrect information a million times and it won't go through, but if I put the correct info once, I have to hit submit then fill out the form again with any information and hit submit for a second time, then I will be taken to the next page. I removed the if (isset ($_COOKIE) block completely to see if it made a difference, but it did the exact same thing.

<?php
include_once "database.php";
session_start();
if (isset($_POST['submit'])) {
    if (isset($_COOKIE['first'])){
        setcookie("first",'',time()-3600,"/");
        setcookie("last",'',time()-3600,"/");
        setcookie("city",'',time()-3600,"/");
    }
    $first=$_POST["first"];
    $last=$_POST["last"];
    $city=$_POST["city"];
    setcookie("first",$_POST["first"],0,"/");//cookie expires after browser closes
    setcookie("last",$_POST["last"],0,"/");//cookie expires after browser closes
    setcookie("city",$_POST["city"],0,"/");//cookie expires after browser closes
    clearstatcache();
    $sql="SELECT * FROM invited WHERE FIRSTNAME='".$_COOKIE['first']."' AND LASTNAME='".$_COOKIE['last']."' AND CITY='".$_COOKIE['city']."'";
    $found=mysqli_query($conn,$sql);
    echo "$first\n\n$last\n\n$city";
    if (mysqli_num_rows($found)){
        $success="<script>window.location.href='rsvp.php'</script>";
    }
    else{
        $fail="ERROR, Could not find";
    }
}
?>

HTML Form:

        <form action="" method="POST">
            <p>Enter Family Name (as it appears on your invitation): </p>
            <input class="entry" type="text" name="first" placeholder="FIRST NAME" required><br><br>
            <input class="entry" type="text" name="last" placeholder="LAST NAME" required><br><br>
            <input class="entry" type="text" name="city" placeholder="CITY (ONLY)" required><br><br>
            <input type="submit" onclick="getCookie()" name="submit">
        </form>
Sal
  • 1,471
  • 2
  • 15
  • 36
  • Can you show us the markup for the form? – Jay Blanchard Jul 12 '18 at 18:38
  • 3
    What are the cookies for? (Don't say to transport the posted values to the next page now - would be nonsense, since you have already started a session, so any data needing that kind of transport should hitch a ride there.) What's `clearstatcache` doing there, there's not a single file system operation going on anywhere - are we just throwing in stuff we somehow "hope might help", or what? And why is this wide open to SQL injection? Time to go through some beginner's tutorials first of all IMHO. – CBroe Jul 12 '18 at 18:41
  • I included the session afterwards, the cookies were set initially, and for the same reason that you mentioned. I wasn't able to use the session properly, and I forgot to remove the line that started it. Same thing with clearstatcache. – Sal Jul 12 '18 at 18:43
  • 3
    [Little Bobby](http://bobby-tables.com/) says ***[your script is at risk for SQL Injection Attacks.](http://stackoverflow.com/questions/60174/how-can-i-prevent-sql-injection-in-php)***. Even [escaping the string](http://stackoverflow.com/questions/5741187/sql-injection-that-gets-around-mysql-real-escape-string) is not safe! – Jay Blanchard Jul 12 '18 at 18:43
  • I think you don't understand what `if (mysqli_num_rows($found)){` will return. – Jay Blanchard Jul 12 '18 at 18:45
  • What does it return? Originally, I had $found in the if statement alone, however it kept returning as true. As such, after googling a bit, I saw a post that said to use mysqli_num_rows. I'm not necessarily looking for how many rows there are with the said data, just need to know if there is one. – Sal Jul 12 '18 at 18:48
  • _"I wasn't able to use the session properly"_ - well then I would first and foremost suggest you make the effort and see one thing properly through the end - read up on the necessary basics, _debug_ what doesn't work - instead of jumping ship at the first sign of trouble & moving on to the next thing. Sessions really aren't that hard to get, and if you fall into the usual beginner traps (headers already sent, ...), then a bit of proper research should get you out of those in no time. – CBroe Jul 12 '18 at 18:50
  • 3
    $_COOKIE values aren't altered by setcookie(). You should not rely on them for the same request. – Devon Bessemer Jul 12 '18 at 18:51
  • @Sal `mysqli_num_rows()` [Returns number of rows in the result set.](https://secure.php.net/manual/en/mysqli-result.num-rows.php). Not `true`/`false` Boolean. Check for a result equal to or greater than zero `if (mysqli_num_rows($found) >= 1)` – Sam Jul 12 '18 at 18:53
  • Note: The object-oriented interface to `mysqli` is significantly less verbose, making code easier to read and audit, and is not easily confused with the obsolete `mysql_query` interface. Before you get too invested in the procedural style it’s worth switching over. Example: `$db = new mysqli(…)` and `$db->prepare("…")` The procedural interface is an artifact from the PHP 4 era when `mysqli` API was introduced and should not be used in new code. – tadman Jul 12 '18 at 19:06
  • Instead of JavaScript to do a redirect, just send a `Location:` header and set status 301 ("redirect"). – tadman Jul 12 '18 at 19:07

1 Answers1

0

Sal, you mustn't rush into this. Let's first understand what your setcookie function actually does:

It sets cookies, right? Sure.

However, if you read on to the second paragraph you would then understand that:

Once the cookies have been set, they can be accessed on the next page load with the $_COOKIE array. Cookie values may also exist in $_REQUEST.

So these two blocks of code are wasteful repetition of code:

if (isset($_COOKIE['first'])){
    setcookie("first",'',time()-3600,"/");
    setcookie("last",'',time()-3600,"/");
    setcookie("city",'',time()-3600,"/");
}

//...

setcookie("first",$_POST["first"],0,"/");//cookie expires after browser closes
setcookie("last",$_POST["last"],0,"/");//cookie expires after browser closes
setcookie("city",$_POST["city"],0,"/");//cookie expires after browser closes

Since they are running in the same run time instance they should not be available until the next load, or the next POST you request. Which I believe would cause the weird "bug" (if you wanna call it that) in your form.

To set a cookie, you would use setcookie(). But that's not what you are doing. What you are trying to do is modify their values. Which is much simpler:

$_COOKIE['first'] = $_POST['first'];
// ... and so on

If you are trying to unset() the $_COOKIE then your if clause is okay.


May I recommend, however, a simpler approach?

You are already using $_SESSIONs. So, why create the hurdle of cookie management when you can easily manage sessions via PHP? The value would still be accessible on your rsvp.php page, and the browser will automatically "forget" the session at closing (unless you say otherwise). I mentioned that because of the comments in your code: //cookie expires after browser closes

To use the sessions simply assign values, they are just an array after-all.

$_SESSION['first'] = $_POST['first'];

That will free up 9 lines of code you currently have.

Sam
  • 2,856
  • 3
  • 18
  • 29
  • Thanks a lot, I switched over from cookies to session, and it worked, I'm not entirely sure why it didn't in the first place. Now I'm running into another issue, when I load up rsvp.php and try mysqli_num_rows($_SESSION('found')) where the session variable is the result of the initial query, I get this error: Warning: mysqli_num_rows(): Couldn't fetch mysqli_result in – Sal Jul 12 '18 at 19:47
  • @Sal, don't store your mysql query on your session. Perform the sql query there, and send the results as an array – Sam Jul 12 '18 at 19:49