-3

I have a piece of code, where every 20 mins, a system task runs a SQL query, adding to one of my database tables (tickets). I have if if statement thats supposed to find than number (tickets) and use it to allow someone to "buy" items from my store. It allows the user to buy more items, than they have tickets for (bringing their balance in to the negatives) like its just completely skipping my else statement. Is there something im doing wrong?

also thought i would add, the "tickets" column is INT and not like, VARCHAR or anything

the thing is also, it worked perfectly a few hours ago, so im not sure if i added something by accident, but now its not working

Be watching, the problem here is "if ($rewards > 4)"

http://prntscr.com/g79q3p

<div class="box">
    <div class="contentHeader headerGreen">
        <div class="inside">
            5 Tickets
        </div>
    </div>
    <ul>
        <img src="/swf/c_images/album1584/2015HA.gif" align="right">
        <li>25k Credits</li>
        <li>25k Pixels</li>
        <li>5 Diamonds
    </ul>
    <?php
        if(isset($_POST['buy1'])) {

            $getRewards = mysql_query("SELECT tickets FROM users WHERE id = '" . $_SESSION['user']['id'] . "'");
            while($rewards = mysql_fetch_assoc($getRewards)) {

                if($rewards > 4) {
                    mysql_query("UPDATE users SET tickets = tickets-5 WHERE id = '" . $_SESSION['user']['id'] . "'") or die(mysql_error());
                    mysql_query("UPDATE users SET credits = credits+25000 WHERE id = '" . $_SESSION['user']['id'] . "'");
                    mysql_query("UPDATE users SET activity_points = activity_points+25000 WHERE id = '" . $_SESSION['user']['id'] . "'");
                    mysql_query("UPDATE users SET vip_points = vip_points+5 WHERE id = '" . $_SESSION['user']['id'] . "'");
                    echo '                 
                        <div class="alert alert-sucess">
                            <strong>Well Done!</strong> Your items have been added.
                        </div>';
                }

                else if($rewards < 5) {
                    echo '                 
                        <div class="alert alert-error">
                            <strong>Error:</strong> You Do NOT Have Enough Tickets To Buy This
                        </div>';

                }

                /*else {
                 echo '                 
                    <div class="alert alert-error">
                        <strong>Error:</strong> You Do NOT Have Enough Tickets To Buy This
                    </div>';                       
                }*/

            }
        }
    ?>
    <form method='post'>
        <input type="submit" value="Buy" name="buy1" style="width: 138px;cursor: pointer;" />
    </form>
</div>
GrumpyCrouton
  • 8,486
  • 7
  • 32
  • 71
  • 2
    __everyone__ can post code here. SO please, __make an effort__ and paste it. – u_mulder Aug 11 '17 at 14:57
  • 3
    `var_dump($rewards)`. It looks like it's an array, not a value that you're thinking of. – aynber Aug 11 '17 at 14:58
  • tried several times, was just formatting it weird. T-T – Trevor Stanley Aug 11 '17 at 14:58
  • 1
    Just post it and someone will edit. Or indent with four spaces to make it correct – Andreas Aug 11 '17 at 15:00
  • There, I'll be nice today and put the code in for you. – aynber Aug 11 '17 at 15:02
  • 1
    **Please**, don't use `mysql_*` functions for new code. They are no longer maintained and the community has begun the [deprecation process](http://news.php.net/php.internals/53799), and `mysql_*` functions have been officially removed in PHP 7. Instead you should learn about [prepared statements](https://en.wikipedia.org/wiki/Prepared_statement) and use either `PDO` or `mysqli_*`. If you can't decide, [this article will help to choose your best option](http://php.net/manual/en/mysqlinfo.api.choosing.php). – GrumpyCrouton Aug 11 '17 at 15:16
  • [Little Bobby](http://bobby-tables.com/) says **[you are at risk for SQL Injection Attacks](https://stackoverflow.com/q/60174/)**. Learn about [Prepared Statements](https://en.wikipedia.org/wiki/Prepared_statement) for [MySQLi](http://php.net/manual/en/mysqli.quickstart.prepared-statements.php). Even **[escaping the string](https://stackoverflow.com/q/5741187)** is not safe! I recommend `PDO`, which I [wrote a function for](https://stackoverflow.com/a/45514591) to make it extremely **easy**, very **clean**, and way more **secure** than using non-parameterized queries. – GrumpyCrouton Aug 11 '17 at 15:16

1 Answers1

0

Please see how mysql_fetch_assocs works.

It returns an array. Does not matter how many fields you specify in sql - it is always an array.

Also do print_r and echo of your variables to see what is inside when it does not work.

  if ($rewards["tickets"] > 4)
  {

  }
  else if ($rewards["tickets"] < .... 
Paul Spiegel
  • 30,925
  • 5
  • 44
  • 53
Dimash
  • 581
  • 1
  • 5
  • 13
  • $getRewards = mysql_query("SELECT tickets FROM users WHERE id = '".$_SESSION['user']['id']."'"); isnt it already selecting "tickets" though?? – Trevor Stanley Aug 11 '17 at 15:05
  • 1
    http://php.net/manual/ru/function.mysql-fetch-assoc.php There examples, please read examples attentivly. Yes, you told to select only tickets, but it does not matter, it acts always the same if you select one or several fields. Less fields - faster sql query, but it represented same way in php – Dimash Aug 11 '17 at 15:07
  • Also guys, you allready not first, do you know that you can just do print_r($rewards) to see what is inside?! It is called - debugging – Dimash Aug 11 '17 at 15:10
  • Also note, that your else most likely will never happen because of esle if < 5, because any interger less than 5 will be in this statement. – Dimash Aug 11 '17 at 15:11
  • Well, Dimash, i had actually just recently added the else if, to try to see if it would help any, it was previously just an else, i was troubleshooting i guess you could say, im new, and learning, and ive literally spent hours trying to get this fixed. when i already had it working at one point. and actually, you were right mate, adding the ['tickets'] got it to work. i appreciate it. and BTW, i hit the up arrow? – Trevor Stanley Aug 11 '17 at 15:14
  • Yeah ) But for some reason I got -2 ) Lol, do always print_r and echo of all variables to see what is inside, It will help a lot. – Dimash Aug 11 '17 at 15:15
  • 1
    Code only answers are generally not well welcomed, probably why you got downvotes. – GrumpyCrouton Aug 11 '17 at 15:15
  • @Dimash If you put your first comment here in the answer itself, it would make it a higher-quality answer (although it would probably still be good to explicitly mention that an array, not a single value, is returned) – Patrick Q Aug 11 '17 at 15:17
  • 1
    I'm not going to downvote for this but you should probably not be teaching people things about `mysql_*` functions because they are no longer maintained and the community has begun the [deprecation process](http://news.php.net/php.internals/53799), and `mysql_*` functions have been officially removed in PHP 7. Instead you should teach about [prepared statements](https://en.wikipedia.org/wiki/Prepared_statement) and use either `PDO` or `mysqli_*`. If you can't decide, [this article will help to choose your best option](http://php.net/manual/en/mysqlinfo.api.choosing.php). – GrumpyCrouton Aug 11 '17 at 15:21
  • 1
    You can downvote if you want, but your notice is not directly related. Code is there and it works, nothing will happen with it if not to update. User even using mysql instead mysqli_, so the right way will be to teach user clean variables from sql injections and not telling him which tool to use. I personally have project with old mysqli functions, I guess it is not clever to redo those project, just stays on old versions. – Dimash Aug 11 '17 at 15:24
  • @Dimash I'm not saying tell them what to use, I'm saying tell them what _not_ to use, as `mysql_*` is deprecated, and being removed (and has been removed, as said), it should not be used in new code at all. – GrumpyCrouton Aug 11 '17 at 15:27
  • well, the xampp that im using uses php 5.x.x something... so it doesnt really matter that much, as to thats what came WITH the project that im doing. Thank you for the notice @GrumpyCrouton but i know already :) – Trevor Stanley Aug 11 '17 at 15:33
  • @TrevorStanley The notice is an attempt to stop you from trying to use this code in a production environment, as it's not safe. I'm not sure why you want to learn old methods that are deprecated and removed from new versions of PHP, as if you ever upgrade your system all this "new code" that you're writing will need to be rewritten. Not to mention the security aspects of the code. You can do what you want, but if I didn't try to explain to you that you shouldn't do it, then I would fee guilty about it. – GrumpyCrouton Aug 11 '17 at 15:35
  • 1
    " I'm not sure why you want to learn old methods that are deprecated and removed from new versions of PHP" Thats not the case, i have a project, thats hundreds of pages worth of "Depricated" code. Im not going to sit here, and go through every single page, just to go from mysql_* to mysqli_*. nor am i "trying to learn old methods" something that was already made, was broken, and i wanted to know how to get it back to its "less-broken" state. is that a problem? I said thank you :) "Thank you for the notice @GrumpyCrouton but i know already :)" – Trevor Stanley Aug 11 '17 at 15:39
  • @TrevorStanley Again you can do what you want, I'm not trying to stop you from using this code in a local project, just remind me to never use your project/website if you publish it, I would rather my information stay safe, and I wish you luck when people attack you with SQL Injection. – GrumpyCrouton Aug 11 '17 at 15:44
  • @GrumpyCrouton Funny discussion ) Still, problem of sql injection is not directly related to mysql methods, it is related to variables escaping. – Dimash Aug 11 '17 at 15:48
  • @TrevorStanley with all respect, Trevor, you are not right, person is not a Dick, and you can just went silent after his last statement not to argue. He just telling what he things and maybe it is clever to warn your client about sql injections. I see nothing dicky in his statements, please control your speach. – Dimash Aug 11 '17 at 15:50
  • Nah thats fine @Dimash just felt that way personally because i see "and I wish you luck when people attack you with SQL Injection" as a way to be passive aggressive. Sorry everyone <3 – Trevor Stanley Aug 11 '17 at 15:51