-1
$pdo = $db_con->prepare("INSERT INTO agents (Agent_ID,Agent_Name,Agent_Branch) VALUES (:1, :2, :3)");
$pdo->bindParam(':1', $id);
$pdo->bindParam(':2', $agent);
$pdo->bindParam(':3', $branch);
$pdo->execute();

No errors and this does work

$db_con->exec("INSERT INTO agents (Agent_ID,Agent_Name,Agent_Branch) VALUES ('fd','dd','d')");

By the way is the first method more secure or does it not matter?

Guesser
  • 1,769
  • 3
  • 25
  • 52
  • The first method is more secure since the bindParam protects you from Sql Injections. – Andresch Serj Apr 09 '14 at 11:56
  • You `bindValue()` unless you're binding a return value from the statement, and use names like `:id`, `:agent` etc. for your placeholders. I'm guessing that just numeric identifiers are invalid. You can also set PDO to use exceptions for your errors so an exception is thrown on error. Otherwise you can use `errorInfo()` on your statement to get more information. This would be useful for debugging. – MatsLindh Apr 09 '14 at 11:57
  • 1
    duplicate of http://stackoverflow.com/questions/15990857/reference-frequently-asked-questions-about-pdo#15990858 – Your Common Sense Apr 09 '14 at 12:01
  • The errorInfo shows this Array ( [0] => 00000 [1] => [2] => ) – Guesser Apr 09 '14 at 12:01
  • Not using numbers made no difference – Guesser Apr 09 '14 at 12:03
  • It's a good practice to name your params by their meaning, so :1 would be :agent_id etc. Make your code readable for others, man. – Rafał Walczak Apr 09 '14 at 12:05
  • What's not readable about a sequence of numbers, to me it's more readable as there is less repetition - dude. – Guesser Apr 09 '14 at 12:08
  • Set your query in a try / catch block and tell us if there 's an exception thrown and if so, what does the message say? – Marcel Apr 09 '14 at 12:18
  • @Marcel there is no point in try / catch blocks here. To have a message thrown one don't need such a block. one need an exception – Your Common Sense Apr 09 '14 at 12:19
  • Well it turns out $agent was an undefined variable, that broke the PDO statement but didn't produce any errors, which is a bit annoying. – Guesser Apr 09 '14 at 12:22
  • undefined variables do not broke PDO statements. – Your Common Sense Apr 09 '14 at 12:30
  • to make PHP produce errors on undefined variables **you** have to set it up so – Your Common Sense Apr 09 '14 at 12:30
  • the way described in the answer I linked to – Your Common Sense Apr 09 '14 at 12:30
  • Sorry but that is what is happening, all I have to do is add "A" to the end of $agent and the statement doesn't work, and it also doesn't produce an error, I don't mean the PHP undefined variable error, there is no error from errorInfo(); – Guesser Apr 09 '14 at 12:38
  • _No errors and this does work_... Sure? Have you enabled all error levels with `error_reporting(E_ALL)`? Also, do: `$pdo->setAttribute(PDO::ATTR_ERRMODE, PDO::ERRMODE_EXCEPTION);`, then you'll have full information about what is happening... – Henrique Barcelos Apr 09 '14 at 13:24
  • That displays 'Agent_Name' cannot be null when $agent is undefined, so that is what is breaking it then. – Guesser Apr 09 '14 at 16:01

1 Answers1

0

Have you considered using ? placeholder instead of :<n>?

Since you are not giving descriptive names to your placeholders, there is no point of doing like your are. I could not find anything at the docs that suggests it's possible to name parameters as integers.

My suggestion:

$pdo = $db_con->prepare("INSERT INTO agents (Agent_ID,Agent_Name,Agent_Branch) VALUES (?, ?, ?)");
// Params are 1-indexed!!!
$pdo->bindParam(1, $id);
$pdo->bindParam(2, $agent);
$pdo->bindParam(3, $branch);
$pdo->execute();

Since you are just ignoring the parameter type (which is OK in most cases), you'd better do:

$pdo = $db_con->prepare("INSERT INTO agents (Agent_ID,Agent_Name,Agent_Branch) VALUES (?, ?, ?)");
$pdo->execute(array($id, $agent, $branch));

Then all three parameters will be treated as strings.


About the question of which one is more secure, if $id, $agent, and $branch are information provided by the user, the more secure approach is the first one, since it uses prepared statements and therefore you'll be warded against SQL Injection at least. If these data come from a "reliable source" (ex.: is hard-coded into you application), then there is no difference between them in terms of security a priori.

However, if someone (anyone) has access to that data, he could change it, making your system vulnerable. Therefore, the wiser option is to always use prepared statements. This is "more secure" than nothing, but is not secure at all, there are several other issues that prepared statements don't treat to take in account.

Henrique Barcelos
  • 7,670
  • 1
  • 41
  • 66
  • he is naming his parameters – Your Common Sense Apr 09 '14 at 13:39
  • your theory on separating data to "more secure" and "less secure" is essentially wrong – Your Common Sense Apr 09 '14 at 13:41
  • He is "naming" with integers, that means no naming at all... And sorry, just saying is wrong and downvote is not enough... – Henrique Barcelos Apr 09 '14 at 13:42
  • Sorry but just to say "you are not naming your parameters" and post it as answer is not enough – Your Common Sense Apr 09 '14 at 13:48
  • What is the point of naming parameters with integers? This is not even supported... – Henrique Barcelos Apr 09 '14 at 13:51
  • Why do you think it is not supported? – Your Common Sense Apr 09 '14 at 13:58
  • there is no "trusted" or "safe" data. Every data is unsafe and have to be properly formatted. – Your Common Sense Apr 09 '14 at 14:01
  • Fair enough, I edited the answer to explain better what I meant at first. – Henrique Barcelos Apr 09 '14 at 14:10
  • You made it quite worse. One would say that prepared statement is not secure, which is wrong again. – Your Common Sense Apr 09 '14 at 14:22
  • What I said is that prepared statements should not be the only line of defense. – Henrique Barcelos Apr 09 '14 at 14:28
  • @Your Common Sense That is not true, data can be sanitised prior to INSERT and should be anyway. If I use $_REQUEST to get say a phone number, I will use a regex and preg_replace to filter out anything but spaces and numbers, the resulting variable is then totally safe for INSERT and does not need further sanitisation. – Guesser Apr 09 '14 at 16:06
  • @user1209203 you just don't understand. this regex you are talking about has nothing to do with SQL. Learn more and eventually you will understand – Your Common Sense Apr 09 '14 at 16:12
  • @Your Common Sense If your so knowledgeable then please explain it, the REGEX in this case can do the same job, how is an integer & spaces INSERT in any way a issue for SQL? – Guesser Apr 09 '14 at 16:16
  • @YourCommonSense ok so now I know where all the confusion has been, I thought bindParam was sanitising the data, the answer to this question http://stackoverflow.com/questions/4364686/how-do-i-sanitize-input-with-pdo is number one on google for 'pdo sanitisation' although it's not wrong it looked to me that bindParam was key to doing the sanitisation as there is no mention of where the sanitisation actually is occurring, maybe this answer needs editing? – Guesser Apr 09 '14 at 16:57
  • Prepared statements only helps against SQL injection because you separate data from statements, sanitisation has nothing to do with it... – Henrique Barcelos Apr 09 '14 at 17:23
  • Ok, then that whole question I posted is misleading. – Guesser Apr 09 '14 at 20:48