Reputation: 253
I have the following PHP function:
public function signup() { $mysql = mysqli_connect(HOSTNAME, USERNAME, PASSWORD, DATABASE); if (mysqli_connect_errno($mysql)) { $this->viewModel->set("pageTitle", "Signup"); $this->viewModel->set("message", "There was an error connecting to the server."); return $this->viewModel; } if ($result = $mysql->query("SELECT id FROM mailinglist WHERE email='" . $this->email . "';")) { if ($result->num_rows == 0) { $mysql->query("INSERT INTO mailinglist (email) VALUES ('" . $this->email . "');"); $this->viewModel->set("message", "Great! Thanks for signing up " . $this->email . "."); } else { $this->viewModel->set("message", "You are already signed up for updates!"); } } else { $this->viewModel->set("message", "There was an error adding you the mailing list."); } $this->viewModel->set("pageTitle", "Signup"); return $this->viewModel; }
Which runs fine and returns exactly what I want, however, if I try to use mysqli_real_escape_string() to my queries, it doesn't work. That is, the following code
public function signup() { $mysql = mysqli_connect(HOSTNAME, USERNAME, PASSWORD, DATABASE); if (mysqli_connect_errno($mysql)) { $this->viewModel->set("pageTitle", "Signup"); $this->viewModel->set("message", "There was an error connecting to the server."); return $this->viewModel; } $query = $mysql->real_escape_string("SELECT id FROM mailinglist WHERE email='" . $this->email . "';"); if ($result = $mysql->query($query)) { if ($result->num_rows == 0) { $query = $mysql->real_escape_string("INSERT INTO mailinglist (email) VALUES ('" . $this->email . "');"); $mysql->query($query); $this->viewModel->set("message", "Great! Thanks for signing up " . $this->email . "."); } else { $this->viewModel->set("message", "You are already signed up for updates!"); } } else { $this->viewModel->set("message", "There was an error adding you the mailing list."); } $this->viewModel->set("pageTitle", "Signup"); return $this->viewModel; }
does not work. It is not a problem with the connection and I have tried using mysqli_real_escape_string() instead of $mysql->real_escape_string() but neither of them work. Can anyone see what is wrong with this code?
Upvotes: 0
Views: 129
Reputation: 6922
Don't do this, use prepared statements. They are safer and more reliable. You still need to sanitize the data for proper value and cross site scripting to name a few risks you'll still encounter. Escaping your data is one way of preventing SQL injection, but it's not full proof. Prepared statements tell the DB server to assume the incoming data is insecure and just accept it, and do not process it like a concatenated string. The database takes your parameters like they are variables for a statement rather than a part of the statement.
Here's how you can change yours to be a prepared statement:
$stmt=$mysql->prepare("SELECT id FROM mailinglist WHERE email=?");
$stmt->bind_param('s',$this->email);
$result=$stmt->execute();
if ($result) {
if ($result->num_rows == 0) {
$stmt=$mysql->prepare("INSERT INTO mailinglist (email) VALUES (?)");
$stmt->bind_param('s', $this->email);
$stmt->execute();
$this->viewModel->set("message", "Great! Thanks for signing up " . $this->email . ".");
} else {
$this->viewModel->set("message", "You are already signed up for updates!");
}
} else {
$this->viewModel->set("message", "There was an error adding you the mailing list.");
}
Upvotes: 2
Reputation: 3907
You have to escape data not the whole query.
public function signup() {
$mysql = mysqli_connect(HOSTNAME, USERNAME, PASSWORD, DATABASE);
if (mysqli_connect_errno($mysql)) {
$this->viewModel->set("pageTitle", "Signup");
$this->viewModel->set("message", "There was an error connecting to the server.");
return $this->viewModel;
}
$query = "SELECT id FROM mailinglist WHERE email='" .$mysql->real_escape_string( $this->email ). "'"
if ($result = $mysql->query($query)) {
if ($result->num_rows == 0) {
$query = "INSERT INTO mailinglist (email) VALUES ('" . $mysql->real_escape_string($this->email) . "')";
$mysql->query($query);
$this->viewModel->set("message", "Great! Thanks for signing up " . $this->email . ".");
} else {
$this->viewModel->set("message", "You are already signed up for updates!");
}
} else {
$this->viewModel->set("message", "There was an error adding you the mailing list.");
}
$this->viewModel->set("pageTitle", "Signup");
return $this->viewModel;
}
Upvotes: -1