Created
August 20, 2014 07:09
-
-
Save jdeniau/0c3df8a498787dc30078 to your computer and use it in GitHub Desktop.
Classe model
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
<?php | |
function search($date = null, $departure = null, $arrival = null) | |
{ | |
if ($departure == null || $arrival == null) | |
return null; | |
if ($date != null) { | |
$day_time = strtotime($date); | |
$day_time_end = $day_time + 86400; | |
$sql = 'SELECT * FROM `ride` as r, `user` as u | |
WHERE r.`user_id` = u.`id` | |
AND r.`towndeparture` LIKE "%'.urldecode($departure).'%" | |
AND r.`townarrival` LIKE "%'.urldecode($arrival).'%" | |
AND r.`timestamp` >= "'.$day_time.'" | |
GROUP BY `id_ride`'; | |
} | |
else | |
$sql = 'SELECT * FROM `ride` as r, `user` as u | |
WHERE r.`user_id` = u.`id` | |
AND r.`towndeparture` LIKE "%'.urldecode($departure).'%" | |
AND r.`townarrival` LIKE "%'.urldecode($arrival).'%" | |
AND r.`timestamp` > NOW() + 3600 | |
GROUP BY `id_ride` ;'; | |
$query = $this->db->query($sql); | |
if ($query->num_rows() > 1) { | |
$result = array(); | |
foreach ($query->result() as $row) | |
$result[] = $row; | |
return $result; | |
} | |
if ($query->num_rows() == 1) | |
return array($query->row()); | |
return null; | |
} |
Injection SQL
Je n'ai pas l'intégralité du code, étant donné que ce sont des paramètres qui sont passés, cela dit, le fait qu'il fasse un urldecode
me laisse penser que ça vient d'un paramètre GET
ou POST
.
Dans le pire des cas, il aurait du utiliser quelque chose comme mysql_real_escape_string, et mieux, utiliser un paramètre PDO
Paramètre inutilisé
Ligne 9
$day_time_end
n'est jamais utilisé.
Return null
Ligne 33
C'est un peu étrange de retourner null lorsqu'il n'y a pas de résultat, plutôt que tableau vide, qui fait plus de sens (c'est bien une liste de 0 départ).
Refactoring
Voila comment je l'aurais fais moi:
<?php
function search($departure, $arrival, $date = null)
{
if (!$date) {
$dayTime = time() + 3600;
} else {
$dayTime = strtotime($date);
}
$sql = 'SELECT * FROM `ride` as r, `user` as u
WHERE r.`user_id` = u.`id`
AND r.`towndeparture` LIKE "%:departure%"
AND r.`townarrival` LIKE "%:arrival%"
AND r.`timestamp` >= ":dayTime"
GROUP BY `id_ride`';
$params = ['departure' => $departure, 'arrival' => $arrival, 'dayTime' => $dayTime];
$preparedQuery = $this->db->prepareQuery($sql, $params); // pseudo code
$query = $this->db->query($preparedQuery);
$result = [];
foreach ($query->result() as $row) {
$result[] = $row;
}
return $result;
}
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
if > 1, else if 1 else...
Ligne 25
A cet endroit, il fait quelque chose d'assez bizarre. Traduit en français, ça donnerait:
La deuxième ligne ne sert en fait à rien, puisqu'elle ferait la même chose que la première.