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

    Join Date
    Oct 2012
    Location
    Adelaide - Australia
    Posts
    167
    Rep Power
    10

    PDO - isset - Count and MySql not parsing isset?


    Probably the last "famous" question on PDO, but I have this problem remaining.

    I would like to bind the isset $ID but I can't seem to use bind or arrays and execute?

    And I also use it to compile the necessary count for pagination purposes, and is something that no matter how I configure any 'other' PDO query, the results never seem to work out?

    And I suppose the question is:

    Considering that GET $ID is wrapped as an intval, is there really any need to bind it - Because after testing with some garbage in the URL it returns nothing but an intval. Or is there an alternative considering the pagination aspect - And if so, could someone point me to it, because I can't find or do anything better that works?

    Originally Posted by wiki.hashphp.org
    Count = ONLY WORKS WITH SQL

    NOTE: Though the documentation says this method is only for returning affected rows from UPDATE, INSERT, DELETE queries, with the PDO_MYSQL driver (and this driver only) you can get the row count for SELECT queries. Keep this in mind when writing code for multiple databases.

    This is because MySQL's protocol is one of the very few that give this information to the client for SELECT statements. Most other database vendors don't bother divulging this information to the client as it would incur more overhead in their implementations.
    PDO Tutorial for MySQL Developers - Hashphp.org


    PHP Code:
        // Start This is used ony for the Count

            
    $ID = isset($_GET['ID']) ? intval($_GET['ID']) : '';

            
    $stmt $database->query("SELECT ID FROM " GAMES_TABLE " WHERE category=$ID AND active=1");

                
    $row_count $stmt->rowCount();

        
    // echo ''.$row_count.' rows selected'; // Test works fine.

                
    $totalcount $row_count;
                    
    $totalpages ceil($totalcount $max); // Works fine.

        // End This is used ony for the Count 
    Thank You, People.
    Last edited by slopalong; December 30th, 2014 at 01:33 AM.
  2. #2
  3. No Profile Picture
    Registered User
    Devshed Newbie (0 - 499 posts)

    Join Date
    Jan 2012
    Posts
    5
    Rep Power
    0
    Originally Posted by slopalong
    Considering that GET $ID is wrapped as an intval, is there really any need to bind it
    No, there is no need to bind the GET data to the query since it's already been casted to an integer, preventing malicious input attacks.

    Originally Posted by slopalong
    Or is there an alternative considering the pagination aspect - And if so, could someone point me to it, because I can't find or do anything better that works?
    Yes, your script for calculating the pagination can be slightly improved. What I would do is use SQL's COUNT() method to calculate the number of rows in the table for you. This means that you will only have to return just that number instead of pulling ID's for every row from the table and then returning that to your script.

    So your new query would look something like the following:
    Code:
    SELECT COUNT(*) FROM " . GAMES_TABLE . " WHERE category=$ID AND active=1"
  4. #3
  5. No Profile Picture
    Registered User
    Devshed Newbie (0 - 499 posts)

    Join Date
    Dec 2014
    Posts
    10
    Rep Power
    0
    First off, use empty instead of isset.

    Secondly, use round instead of ceil.

    Thirdly, if ID is empty then you need to either trigger an error or notify the user.

    And finally, use prepared statement to avoid SQL injection.
  6. #4
  7. No Profile Picture
    Registered User
    Devshed Newbie (0 - 499 posts)

    Join Date
    Dec 2014
    Posts
    10
    Rep Power
    0
    With regards to COUNT(), avoid making unnecessary SQL request and use PHP count() function instead.
  8. #5
  9. No Profile Picture
    Registered User
    Devshed Newbie (0 - 499 posts)

    Join Date
    Jan 2012
    Posts
    5
    Rep Power
    0
    Originally Posted by getmizanur
    With regards to COUNT(), avoid making unnecessary SQL request and use PHP count() function instead.
    That's incorrect. You want the processing (i.e. the counting of rows) to be done in the database because that is far quicker than querying the database for all rows, and then returning everyone row to the receiving PHP script, and the summing the total row count there. It's far less optimal to do this on the PHP-side of things.

    Originally Posted by getmizanur
    First off, use empty instead of isset.
    That doesn't particularly matter in OPs case, since he is assigning an empty value to $ID in the event of $_GET['ID'] being unset.

    Originally Posted by getmizanur
    Secondly, use round instead of ceil.
    The OP wants to use ceil() instead of round(), since ceil() will always round up. This is needed because if there is 7 rows, and the number of rows to display per page is 3, then using ceil(7/3) would give us 3 pages (i.e. page 1 will have 3 rows, page 2 will have 3 rows, and page 3 will have 1 row (to display)). If round() was used, then 7/3 would give us 2 pages, which would skip result sets.

    Originally Posted by getmizanur
    Thirdly, if ID is empty then you need to either trigger an error or notify the user.
    Not if it's an optional field, as seems that case in OPs scenario.

    Originally Posted by getmizanur
    And finally, use prepared statement to avoid SQL injection.
    OP has already mitigated any potential for SQL injection attacks by casting the GET data to an integer (using intval()), therefore he is not vulnerable to such attacks.
  10. #6
  11. No Profile Picture
    Registered User
    Devshed Newbie (0 - 499 posts)

    Join Date
    Dec 2014
    Posts
    10
    Rep Power
    0
    No, there is no need to bind the GET data to the query since it's already been casted to an integer, preventing malicious input attacks.
    You shouldn't be casting to integer either. I think it is better to use is_int() than intval().

    That's incorrect. You want the processing (i.e. the counting of rows) to be done in the database because that is far quicker than querying the database for all rows, and then returning everyone row to the receiving PHP script, and the summing the total row count there. It's far less optimal to do this on the PHP-side of things.
    I agree with you, calculating the total number of pages you need to know total rows, and fast way to do this by using SQL COUNT() function.

    Not if it's an optional field, as seems that case in OPs scenario.
    I think this is a required field instead an optional.

    OP has already mitigated any potential for SQL injection attacks by casting the GET data to an integer (using intval()), therefore he is not vulnerable to such attacks.
    This is not entirely correct, but for this scenario, it is fine.

    The OP wants to use ceil() instead of round(), since ceil() will always round up. This is needed because if there is 7 rows, and the number of rows to display per page is 3, then using ceil(7/3) would give us 3 pages (i.e. page 1 will have 3 rows, page 2 will have 3 rows, and page 3 will have 1 row (to display)). If round() was used, then 7/3 would give us 2 pages, which would skip result sets.
    I agree with you.
  12. #7
  13. Wiser? Not exactly.
    Devshed God 2nd Plane (6000 - 6499 posts)

    Join Date
    May 2001
    Location
    Bonita Springs, FL
    Posts
    6,072
    Rep Power
    4101
    Originally Posted by c3f3 Admin
    OP has already mitigated any potential for SQL injection attacks by casting the GET data to an integer (using intval()), therefore he is not vulnerable to such attacks.
    A prepared statement and bound parameter should be used rather than intval(). If someone were to make a request with garbage input that just happened to start with a number then intval() would give you that numeric prefix and ignore the rest. The proper thing to do in this scenario would be to return an error instead. Imagine if someone could make a whole bunch of random URLs to a product on your site just by adding trash after the ID in the URL. There goes your search engine ranking for duplicate content.

    You could also run into potential integer overflow problems using intval() if you happen to have very large ID numbers and 32-bit PHP. While unlikely, why not just do it properly in the first place and use a parameter in a prepared statement.


    Originally Posted by getmizanur
    You shouldn't be casting to integer either. I think it is better to use is_int() than intval().
    is_int tests if the variable is of data type int, which will be false for any request input since they are all strings (even if it contains only digits). If you want to test if a string consists of just digits, use ctype_digit or a regular expression

    Comments on this post

    • ttremain agrees : Specifically about random values to the GET[ID] causing duplicate content if not caught with error.
    Recycle your old CD's



    If I helped you out, show some love with some reputation, or tip with Bitcoins to 1N645HfYf63UbcvxajLKiSKpYHAq2Zxud
  14. #8
  15. No Profile Picture
    Registered User
    Devshed Newbie (0 - 499 posts)

    Join Date
    Jan 2012
    Posts
    5
    Rep Power
    0
    Originally Posted by kicken
    A prepared statement and bound parameter should be used rather than intval(). If someone were to make a request with garbage input that just happened to start with a number then intval() would give you that numeric prefix and ignore the rest. The proper thing to do in this scenario would be to return an error instead. Imagine if someone could make a whole bunch of random URLs to a product on your site just by adding trash after the ID in the URL. There goes your search engine ranking for duplicate content.
    Irregardless of whether you use any one of the following:
    PHP Code:
    $id intval($_GET['id']);
    $id = (int) $_GET['id'];
    $query->bindParam(1$_GET['id'], PDO::PARAM_INT); 
    The id from the GET data is going to be cast to an integer. So it doesn't make a difference whether you choose to use PDO's parametrised queries or use a PHP function (or explicit cast) with respect to your argument about letting in garbage into the query string of the URI. I do agree with you though that the OP should probably validate the integer (and return an error if needed) instead of using it's value irregardless of whether it is an integer or not.

    [QUOTE=kicken;2948999]
    You could also run into potential integer overflow problems using intval() if you happen to have very large ID numbers and 32-bit PHP. While unlikely, why not just do it properly in the first place and use a parameter in a prepared statement.
    [QUOTE]

    Like you said - an unlikely situation. But I understand your point
  16. #9
  17. No Profile Picture
    Contributing User
    Devshed Newbie (0 - 499 posts)

    Join Date
    Dec 2005
    Location
    Vancouver, WA, USA
    Posts
    425
    Rep Power
    192
    Originally Posted by slopalong
    $ID = isset($_GET['ID']) ? intval($_GET['ID']) : '';
    $stmt = $database->query("SELECT ID FROM " . GAMES_TABLE . " WHERE category=$ID AND active=1");
    This is not your original question, but if $_GET['ID'] is not set $ID will equal ''

    where as your query will be:
    SELECT ID FROM some_table WHERE category= AND active=1

    This is an invalid query.... Is this what you intended?

    I would: $ID=(int) $_GET['ID'];

    The if $ID == 0 , respond with an error.




    (I see now this was addressed some time before I saved my reply.)
    Last edited by ttremain; January 6th, 2015 at 05:29 AM.
    ttremain
  18. #10
  19. Contributing User
    Devshed Newbie (0 - 499 posts)

    Join Date
    Oct 2012
    Location
    Adelaide - Australia
    Posts
    167
    Rep Power
    10
    Well, while I've been laid-up for a couple of weeks - We have been having fun!

    Anyway......

    @c3f3 Admin - Tried your first post back some time ago and that - Didn't work either?

    Originally Posted by c3f3 Admin
    The OP wants to use ceil() instead of round(), since ceil() will always round up. This is needed because if there is 7 rows, and the number of rows to display per page is 3, then using ceil(7/3) would give us 3 pages (i.e. page 1 will have 3 rows, page 2 will have 3 rows, and page 3 will have 1 row (to display)). If round() was used, then 7/3 would give us 2 pages, which would skip result sets.
    I agree because if the page count is ODD - then it generates an extra page.

    But - I looked at this again today and constructed this little jewel...

    PHP Code:
        // Start This is used only for the Count

            
    $pdo "SELECT ID FROM " GAMES_TABLE " WHERE category=? AND active=1";
                
    $stmt $database->prepare($pdo);
                    
    $stmt->bindParam(1$ID);
                        
    $stmt->execute();

                
    $row_count $stmt->rowCount(PDO::PARAM_INT);

                
    $totalcount $row_count;
                    
    $totalpages ceil($totalcount $max);

        
    // End This is used only for the Count 
    And it's all good! and is functional whether one binds something or not.

    And tomorrow - I'll switch ceil to round and we'll go on from there.

    But thanks for ALL of the input people - even if you can't all agree.

    Edit: 17/10

    I tried "round" and if there's only one page (with the pagination I use) It doesn't show up - So I'll stick to ceil..
    Last edited by slopalong; January 17th, 2015 at 03:34 AM. Reason: some thing = something

IMN logo majestic logo threadwatch logo seochat tools logo