2

As I have to fetch data multiple times so I created select function and where function is it write way to way to use and also in point of view of SQL Injection , this way is not good ? Please guide

function where($column, $value) {
  return "AND $column = :$column";
}

function fetchCategory($where, $data1) {
  // Create a PDO instance
  $db = Database::newInstance();
              
  // Build the SELECT statement with a WHERE clause
  $sql1 = "SELECT * FROM category WHERE 1=1 $where";
              
  // Execute the SELECT statement with bound parameters
  $row1 = $db->read($sql1, $data1);
              
  // Return the result set
  return $row1;
}

$where = where('cat_id', $value->parent_id);
$data1 = array(':cat_id' => $value->parent_id);
$result = fetchCategory($where, $data1);

if ($result) {
  // Fetch the data from the result set
  $data['Dis_05']= $result[0]->category;
} else {
  // No data was found
  echo "No data found";
}
Your Common Sense
  • 156,878
  • 40
  • 214
  • 345
Madisson
  • 35
  • 5
  • 1
    I'm not 100% sure what you are trying to do here, but you can't use parametrized queries for column names. – bassxzero Jan 06 '23 at 09:30
  • @bassxzero this not good? – Madisson Jan 06 '23 at 09:35
  • 1
    "right way" is subjective, so we can't answer that. If you think making things like the `where` function is useful for you, then please go ahead and do it. And since we cannot see your `read()` function we cannot tell you if it's safe from SQL injection or not. – ADyson Jan 06 '23 at 10:05
  • 1
    @bassxzero I see no attempt to parameterise a column name here. It's unclear what you're referring to. – ADyson Jan 06 '23 at 10:05
  • https://phpdelusions.net/pdo/common_mistakes#select – Your Common Sense Jan 06 '23 at 10:17
  • @ADyson I was asking how can guide SQL Injection in this function .Can you guide? – Madisson Jan 06 '23 at 10:17
  • 3
    Writing just two functions, fetchCategoryById($id) and fetchCategoryByAnotherColumn($column) will be WHOLE WORLD better than this clumsy unreliable home-brewed query builder – Your Common Sense Jan 06 '23 at 10:20
  • 2
    Although I understand the urge to remove the repetitions from code, the approach you choose is wrong on so many levels from readability to reusability. Having good intentions in mind, you are about to create a MONSTER that, in the end, will be 10 harder to use than just plain SQL – Your Common Sense Jan 06 '23 at 10:22
  • 2
    One of the worst premises here is the "fetch data multiple times" idea. In reality, it will be used with 1 or 2 different queries. But you will spend incomparable amount of work trying to create a universal solution... for 2 queries – Your Common Sense Jan 06 '23 at 10:24
  • 1
    `I was asking how can guide SQL Injection in this function`...I know. But you didn't show any PDO code so we can't tell you. SQL injection can only happen when you build and execute a query. That must be happening inside your `read()` function, which you didn't show us. – ADyson Jan 06 '23 at 10:44
  • 1
    But you can read about how to prevent SQL injection in many places already, such as [How can I prevent SQL injection in PHP?](https://stackoverflow.com/questions/60174/how-can-i-prevent-sql-injection-in-php), [How to include a PHP variable inside a MySQL statement](https://stackoverflow.com/questions/7537377/how-to-include-a-php-variable-inside-a-mysql-statement), https://phpdelusions.net/pdo and many other places. – ADyson Jan 06 '23 at 10:45
  • 1
    The most we can tell you is that you appear to be using parameters correctly in your SQL string. But we don't know what you're doing after that. – ADyson Jan 06 '23 at 10:54
  • 1
    P.S. YourCommonSense is right...I can see you're trying to write some sort of re-usable function to make it simpler to run different queries. But really your system is not very flexible, it only allows for one parameter in the WHERE clause. It's not sufficiently re-usable to be useful beyond a very small number of queries. So, it makes your code harder to read and harder to debug, without much benefit. I would suggest either sticking to writing queries in your normal way using standard PDO prepared statements, or moving entirely to using an existing ORM module which you could find online. – ADyson Jan 06 '23 at 10:56
  • 1
    @Madisson may I invite you to this room? https://chat.stackoverflow.com/rooms/250892/php-help I like the idea but I want to offer you another implementation – Your Common Sense Jan 06 '23 at 12:40

0 Answers0