0

i know this must be only a small bug, but i cant find it.

My function:

function del_mysql($table,$id)
{
$id = $_GET['id'];
$exec = mysqli_query($con, "delete from $table where id = '$id'");
return $exec;
}

in Code:

if ($_GET['action'] == 'delete')
{
del_mysql("awsome","$id");
}

if make in function:

$id = $_GET['id'];
echo $table;
echo $id;

i get right table and id.

Somebody see the bug? I removed already the $exec and return part and leave only mysqli_query command. but dont want to work.

user2933212
  • 303
  • 2
  • 4
  • 11
  • 11
    **Danger**: You are **vulnerable to [SQL injection attacks](http://bobby-tables.com/)** that you need to [defend](http://stackoverflow.com/questions/60174/best-way-to-prevent-sql-injection-in-php) yourself from. – Quentin Jan 08 '14 at 11:47
  • You forgot to explain what's wrong. – Álvaro González Jan 08 '14 at 11:47
  • 2
    Turn error reporting ON and you will see your errors. – Glavić Jan 08 '14 at 11:48
  • What about this line in function? $id = $_GET['id']; – Sibiraj PR Jan 08 '14 at 11:48
  • 1
    Why do you use Superglobals in your function? Using Superglobals is normally discouraged in functions and methods and also error-prone. Also *"Somebody see the bug?"* is not a valid question to ask for on Stackoverflow while you're only dumping some live code ***and not*** an isolated example that you created from scratch to demonstrate an isolated issue that you're not able to understand. – hakre Jan 08 '14 at 11:49
  • Why do you fetch `$_GET` assignment inside the function, rather than fetching the function param? – BenM Jan 08 '14 at 11:49
  • i enabled error reporting but it shows no error. >$id = $_GET['id']; is correct. if i replace it with the id in mysql db it too dont works. @quentin, thanks. i know. – user2933212 Jan 08 '14 at 11:50
  • @hakre let me to not agree with you. In a good object oriented system, you should not have more than one or two files outside a class (except templates), that run the application, everything else should be encapsulated with classes and methods, so you would need to use the superglobal somewhere in a method. If not exactly in the method where you query the DB, you will in another one to extract the values. Even the get request methods in the popular frameworks are dealing with the superglobals inside functions. – Royal Bg Jan 08 '14 at 11:52
  • @user2933212 - Then you didn't enable error reporting properly. Your code should trigger at least an *undefined variable* notice. – Álvaro González Jan 08 '14 at 11:53
  • 1
    @Royal Bg: You did not agree nor disagree with me. You honor to some code you not further specify and take it's sole existance as an argument against what I wrote. Which is obviously not the case, well designed and popular frameworks do interact with Superglobals (PHP is a framework already), however they reduce the usage of these to the bare minimum and even work without these. So the framworks are normally without superglobals, *your* bootstrap code might use them, but that's not a function nor method in the sense of my comment. So you've not given any argument, sorry. – hakre Jan 08 '14 at 11:56
  • 1
    Also, in the context of this question, the hint that you should prevent superglobals usage in your code should be an easy to follow and think about guideline which should be embraced. There is no use to tell the OP about some "popular framework" and imply this would change anything to the code-smell. – hakre Jan 08 '14 at 11:57
  • @hakre my comment was not relevant to the question (which is already answered), it may be considered offtopic, but I just saw a comment of type "using superglobal in a method is highly discouraged", and wanted to argue about this. Where, do you suggest to use the superglobal, if it's not in a method? Making a request handler will still be a function/method that deals with a superglobals and give you the correct response of `$_GET['smth']` without really using `$_GET` in your code. – Royal Bg Jan 08 '14 at 12:01
  • Even I said I'm arguing, I have no hard feelings, just want to see your oppinion about where to use the superglobal if not in a function/method? – Royal Bg Jan 08 '14 at 12:01
  • @Royal Bg: I just wanted to be techncially explicit. Sorry if that sounds rude. No hard feelings here either. And to answer your question: Well, not in a function/method. Those superglobals belong into global code (glue code, bootstrap), just to wire the system together, not inside the system. Otherwise you're not using PHP as a framework (technically this can mean it's inside a function or method, however, try to prevent that and the cases where not, make an exception to the rule, but a single one, be strict. Superglobals are expensive). – hakre Jan 08 '14 at 12:07

1 Answers1

0

The problem is that in your del_mysql function, you are referencing the connection object $con, which does not exist in the scope of the function. Either pass it into the function as a parameter like this:

function del_mysql($table, $id, $con) {

or access it as a global variable like this:

function del_mysql($table, $id) {
    global $con;

I hope that helps.

Regards, Ralfe

ralfe
  • 1,412
  • 2
  • 15
  • 25
  • 1
    Even this might be intended helpful, it does not qualify as an answer (as much as the question does not qualify as a question). Also the code is suboptimal. – hakre Jan 08 '14 at 11:51
  • 1
    @hakre, maybe it would be better to post self a answers instand of criticize other. – user2933212 Jan 08 '14 at 11:53
  • @user2933212, I'm glad I could help. As this answered your question, could I please ask that you mark the answer as correct. – ralfe Jan 08 '14 at 12:02
  • 1
    @user2933212: I showed my opinion, also your non-question has been honored with comments to a great extend already which IMHO should be the place to reflect it, not per an answer. Also I do not think that criticizing each other is something bad as it sounds with your comment. Therefore I disagree with that especially. – hakre Jan 08 '14 at 12:02
  • @hakre, it seems that my **question** does really annoy you. i am a hard beginner in PHP and wanted a quick answer for a small problem. before posting here i was using google, but couldnt find a solution. **imho** you should keep cool because its not good for your heart if you get in rage, and probably you could post a answer if you find that ralfe answer is not so good. for me, the answer is good enough and it solved my problem. anyway this script is only local so it doesnt hurt somebody if the code smells like - what you sayed - And you can now ask all your friends to press - btn, like you. – user2933212 Jan 08 '14 at 12:09
  • @user2933212: Well, no worries, I'm in absolute routine with such new-user behavior here on SO. The problem is actually you look for the quick answer that is actually the cause of not finding quick answers on SO. I wish you would understand how you prevent that with your behavior. – hakre Jan 08 '14 at 14:40