#1
  1. Contributing User
    Devshed Newbie (0 - 499 posts)

    Join Date
    Sep 2012
    Posts
    204
    Rep Power
    2

    Putting Query in a function (cleanliness )


    Hi.

    On my page i have a query which returns all the content. Now i'm just putting this query at the top of the document like this

    Now i'm wondering.. is this best practice? Should i put it in a functions file? and if yes how do i do that? you would be of great help. Thanks


    PHP Code:
    <?php 

        
    include_once('connect.php');

        
    $fields_select $DBH->query('SELECT * FROM content');
        
    $fields_select->setFetchMode(PDO::FETCH_ASSOC);
        
    $fields $fields_select->fetchAll();

    ?>
    <!DOCTYPE html>
    <html>
    <head>
  2. #2
  3. --
    Devshed Expert (3500 - 3999 posts)

    Join Date
    Jul 2012
    Posts
    3,911
    Rep Power
    1045
    Hi,

    yeah, I'd make a function -- or a database class even, which would take care of the connection stuff as well.

    However, there are several other issues, which are somewhat more important:
    • Do not use this horrible "SELECT *" stuff. It's inefficient and potentially dangerous. Select specific columns. Are your even sure you need the whole table?
    • If PDO::FETCH_ASSOC is your prefered fetching mode, set it in the PDO options so that you don't have to repeat it for each and every query.
    • Do not use fetchAll() unless you absolutely need to have the rows in an array. If you just want to iterate over the rows once, use the PDO statement directly, it's iterable. This is much more efficient.
  4. #3
  5. Contributing User
    Devshed Newbie (0 - 499 posts)

    Join Date
    Sep 2012
    Posts
    204
    Rep Power
    2
    Originally Posted by Jacques1
    Hi,

    yeah, I'd make a function -- or a database class even, which would take care of the connection stuff as well.

    However, there are several other issues, which are somewhat more important:
    • Do not use this horrible "SELECT *" stuff. It's inefficient and potentially dangerous. Select specific columns. Are your even sure you need the whole table?
    • If PDO::FETCH_ASSOC is your prefered fetching mode, set it in the PDO options so that you don't have to repeat it for each and every query.
    • Do not use fetchAll() unless you absolutely need to have the rows in an array. If you just want to iterate over the rows once, use the PDO statement directly, it's iterable. This is much more efficient.
    Hi.

    Thanks for the good info!

    How should i implement the PDO iterator then?

    PHP Code:
    foreach($q->fetch() as blabla ) ? 
    now im using this to fetch the results of the Fetchall array

    PHP Code:
        <?php foreach($fields as $field) : ?>
                    <h2><?= $field['field'?></h2><small><?= $field['field_info'?></small>
                    <textarea id="redactor_<?= $field['field'?>" class="redactor" name="<?= $field['field'?>"><?= $field['value'?></textarea>
                <?php endforeach ?>
  6. #4
  7. --
    Devshed Expert (3500 - 3999 posts)

    Join Date
    Jul 2012
    Posts
    3,911
    Rep Power
    1045
    Originally Posted by notflip
    How should i implement the PDO iterator then?

    PHP Code:
    foreach($q->fetch() as blabla ) ? 
    No fetch(), just loop directly over the PDOStatement object returned by query(), prepare() etc.:
    PHP Code:
    $query $DBH->query('
        SELECT
            something
        FROM
            content
    '
    );
    foreach (
    $query as $row) {
        
    var_dump($row);



    Originally Posted by notflip
    now im using this to fetch the results of the Fetchall array

    PHP Code:
        <?php foreach($fields as $field) : ?>
                    <h2><?= $field['field'?></h2><small><?= $field['field_info'?></small>
                    <textarea id="redactor_<?= $field['field'?>" class="redactor" name="<?= $field['field'?>"><?= $field['value'?></textarea>
                <?php endforeach ?>
    You really need to escape your values before outputting them. Your code is wide open to JavaScript injections.
    PHP Code:
    function html_escape($raw_input) { 
        return 
    htmlspecialchars($raw_inputENT_QUOTES ENT_HTML401'UTF-8');     // important! don't forget to specify ENT_QUOTES and the correct encoding 

  8. #5
  9. Contributing User
    Devshed Newbie (0 - 499 posts)

    Join Date
    Sep 2012
    Posts
    204
    Rep Power
    2
    Thank you, I see the array is not necessary now.

    For the escaping.. I didn't know you have to sanitize client-side for javascript..

    I put the function but i wonder what values i need to 'escape' with that function? thanks!
  10. #6
  11. --
    Devshed Expert (3500 - 3999 posts)

    Join Date
    Jul 2012
    Posts
    3,911
    Rep Power
    1045
    Any variable you want to output on any HTML page must be run through htmlspecialchars first. If you don't do that, your visitors can inject HTML into your page, especially script elements to execute malicious JavaScript code.

    Check the link in my signature. The keyword is "cross-site scripting" or "XSS".
  12. #7
  13. Contributing User
    Devshed Newbie (0 - 499 posts)

    Join Date
    Sep 2012
    Posts
    204
    Rep Power
    2
    Originally Posted by Jacques1
    Any variable you want to output on any HTML page must be run through htmlspecialchars first. If you don't do that, your visitors can inject HTML into your page, especially script elements to execute malicious JavaScript code.

    Check the link in my signature. The keyword is "cross-site scripting" or "XSS".
    Allright! I can now even send "<script src=" ... " to my database, and if i get it back it's shown correctly using htmlspecialchars..

IMN logo majestic logo threadwatch logo seochat tools logo