Jump to content

mac_gyver

Staff Alumni
  • Posts

    5,374
  • Joined

  • Days Won

    173

Posts posted by mac_gyver

  1. php's error_reporting should (always) be set to E_ALL. when learning, developing and debugging code/query(ies), display_errors should be set to ON. when on a live/public server, display_errors should be set to OFF and log_errors should be set to ON. these setting should be in the php.ini on your system so that ALL php errors will get reported (putting the settings in your code won't cause fatal parse errors to be reported since your code never runs to cause the settings to take effect) and so that they can be changed in a single place. while you are making changes to the php.ini, set output_buffering of OFF. stop and start your web server to get any changes made to the php.ini to take effect and confirm that the settings actually got changed by using a phpinfo() statement in a .php file that you request through the web server.

    next, when you make the PDO connection -

    1. set the character set to match your database tables. so that no character conversion occurs over the connection.
    2. as already mentioned, set the error mode to exceptions (this is actually the default now in php8, but set it anyways), so that all the rest of the PDO statements, after the connection (which always uses exceptions for errors), will also use exceptions for errors.
    3. set emulated prepared queries to false, you want to run real, not emulated, prepared queries.
    4. set the default fetch mode to assoc, so that you don't need to specify it in each fetch statement.

    since you are just getting started with the PDO extension, save yourself a lot of typing with prepared queries by using simple positional ? prepared query place-holders and use implicit binding, by supplying an array of input values to the ->execute([...]) call.

  2. web servers are stateless. they don't know or care about anything outside of the current http request they are servicing. any data submitted to a page or any result calculated on a page, unless persistently stored in a common accessible location, e.g. a database, file, common memory cache,... doesn't exist once the server-side code on any page ends.

    you are making an application where there are x winners calculated at some interval or on some set of submissions. you must persistently store the result of each 'round' so that anyone visiting the 'result' page(s) can view the results.

  3. 7 minutes ago, phppup said:

    Aside from instances with a unique parameter,should I be doing this (try & catch) regularly with all my INSERT statements?

    no. someone already went to great detail specifically stating when to use error handling in your application code and when not to.

  4. no. not only does it take two queries and more code, but when you try to execute a SELECT query to find if data doesn’t exist, a race condition exists where multiple concurrent instances of your script will all find that the value doesn’t exist and attempt to insert it. the database is the last step in the process. it must enforce uniqueness. since the database must do this, you might was well eliminate the initial SELECT query, which can tell your code the wrong answer.

  5. the point of the catch code is to test the error number and just handle specific error number(s).  getMessage() is the error text and won't do you any good for testing the error number. these are both OOP methods, not procedural code.

    BTW - getCode() is the sqlstate, which groups together several similar errors, and is not the actual error number.

  6. 44 minutes ago, phppup said:

    are rejected and need to be re-submitted of the $username already exists?

    yes. but since your form processing code and form are on the same page and you are safely repopulating the form field values with the submitted form data, this should not be a problem. just correct the username value and resubmit the form.

  7. yes, you should only make one database connection. your main code would do this in an 'initialization' section, then pass it as a call-time parameter into any function that needs it. this will eliminate the repeated connection code. Don't Repeat Yourself (DRY.) next, there's generally no need to close a database connection in your code since php will close it when your script ends.

    you should separate the database specific code, that knows how to query for and fetch data, from the presentation code, that knows how to produce the output from that data. to do this just fetch the data from any query into a php variable. functions should return their result to the calling code, so that the result can be used in any context.

    to produce the output, both for the main layout and for the dynamic sections within a page, you should use a template system. either just using simple php code or use one of the 3rd party template systems. you should use 'require' for things your page must have for it to work.

    since odbc_fetch_array is available (which is why you were asked in a previous thread what database server type you were using), you should use that everywhere, instead of using multiple odbc_result statements. always use the simplest logic that accomplishes a task. Keep It Simple (KISS.) 

  8. 2 minutes ago, phppup said:

    How is this method better than using IF statements?

    you ALWAYS need error handling for statements that can fail. the simplest way of adding error handling for all the database statements that can fail - connection, query, prepare, and execute, WITHOUT adding logic at each statement, is to use exceptions for errors and in most cases simply let php catch and handle the exception, where php will use its error related settings to control what happens with the actual error information  via an uncaught exception error (database statement errors will 'automatically' get displayed/logged the same as php errors.) this saves you from having to write out a ton of conditional statements where they won't do your application any good.

    the only database statement errors that are recoverable by the user on a site are when inserting/updating duplicate or out of range user submitted values. this is the only case where your application should handle database statement errors, and since you are using exceptions for database statement errors, this is the only case where your application code should have a try/catch block.

    your catch code would test if the error number is for something that your code is designed to handle (a duplicate index error number is 1062),  and setup a message for the user telling them exactly what was wrong with the data they they submitted. for all other error numbers, just rethrow the exception and let php handle it. if there's only one column defined as a unique index, a duplicate index error would mean the duplicate was in the column. if you have more than one unique index, you would execute a SELECT query within the catch code to find which column(s) contain duplicate values.

  9. 6 hours ago, hendrikbez said:

    This is only on my private laptop, I am the only one that will use this.

    that doesn't matter. the main point of using prepared queries is to prevent any sql special characters in a value from breaking the sql query syntax (which is how sql injection is accomplished.) currently, a value that contains a ' (singe-quote) will break the sql query syntax and produce an error, rather than to allow the UPDATE query to execute. what if the user on your site has a name that contains a ' or wants to use one in their username? using a prepared query also simplifies the sql query syntax, making it easier to write a query that doesn't contain typos.

    6 hours ago, hendrikbez said:

    Do not understand what you are saying here.

    a header() statement doesn't stop php code execution. you must use an exit/die statement with/after a header() redirect to stop php code execution. currently (before the last posted code), all the rest of the code on the page executes every time it gets requested, even if the user isn't logged in.

    6 hours ago, hendrikbez said:

    did not know you can use it only one time.

    it's not about using them only one time (you can use them any number of times.) this is about writing code that doesn't contain a lot of unnecessary clutter that you want someone else to look at and help you with. when you post code that contains as much as 2x the amount of typing in it that is necessity, it makes it harder for the people who would help you. it also takes your time typing and fixing mistakes in all the unnecessary typing.

    the points that were made about the php code are coding practices that will help make the code secure, simplify it, help reduce mistakes, ...

    here are some more points about just the posted php code -

    1. error_reporting should always be set to E_ALL and the setting should be in the php.ini on your system, so that you can change it in a single place.
    2. don't use output buffering to make bad code work. find and fix whatever is preventing your code from working. using output buffering also makes debugging harder because it hides non-fatal php errors and any debugging echo output you use, when you execute a header() redirect.
    3. the only value you should store in a session when the user logs in is the user_id. query on each page request to get any other user information (which you are doing) such as the username, permissions, ... this insures that any changes made to the other user information will take effect on the next page request. 
    4. don't copy variables to other variables for nothing. this is a waste of typing time. just use the original variables.
    5. don't unconditionally destroy the session. the session can hold things other then the logged in user_id. the session user_id will either be set or it won't. you don't need to create a bunch of variables and logic to detect if there is a logged in user or not.
    6. your should build the sql query statements in a php variable. this makes testing easier and help prevent typo mistakes by separate the sql query syntax as much as possible from the php code.
    7. don't use variables ending in numbers. this just makes more work for you keeping track of what you are doing in your code.
    8. you should list the columns you are selecting in queries. this help make your code self-documenting and helps prevent mistakes.
    9. outside of the log in code, you should NOT select and fetch passwords.
    10. don't use a loop to fetch data from a query that will match at most one row of data. just fetch the data.
    11. don't store passwords in plain text. use php's password_hash() and password_verify().
    12. don't attempt to detect if a form submit button is set. there are cases where it won't be. instead detect if a post method form was submitted.
    13. one case where the submit button won't be set is if you upload a file that exceeds the post_max_size setting. in this case, both the $_POST and $_FILES arrays will be empty. your post method form processing code must detect this and setup a message for the user that the size of the post data was too large and could not be processed.
    14. after you have detected that there is $_FILES data, you must test the ['error'] element and only use the uploaded file information if there was not an upload error (the errors are listed in the php documentation.)
    15. your post method form processing code should trim then validate all $_POST inputs before using them.
    16. apply htmlentities() to any dynamic value when you output it on a web page to help prevent cross site scripting.

    finally, these points just apply to the php code. your makeup is not working due to the mistakes in the html document. you need to clean up the html document so that it only loads each external element once and is valid markup.

    • Like 1
  10. the code on any php page should be laid out in this general order -

    1. initialization
    2. post method form processing
    3. get method business logic - get/produce data needed to display the page
    4. html document

    if you build any or all of these sections using separate .php files, use 'require' for things that your page must have.

    the html document that you produce must be valid and should not repeatedly load the same external elements. i recommend that you validate the resulting html output at validator.w3.org  the current result contains a large number of errors, which may account for the incorrect operation in the browser. also, have you checked the browser's developer console for errors?

     

  11. or you could just convert to the much simpler, better designed, and more modern pdo extension. 

    in your previous thread, i posted an example showing what your SELECT query would look like using the pdo extension. here's what the UPDATE query in this thread would look like -

     

    $stmt = $pdo->prepare("UPDATE accounts SET userbalance = ?, totaldonationdc = ?, userlevel = ?, userexperience = ?, totaleventjoined = ? WHERE id = ?");
    $stmt->execute([ $userbalance, $totaldonationdc, $userlevel, $userexperience, $totaleventjoined, $_SESSION['id'] ]);

    after you have made a connection using the pdo extension, converting existing msyqli based code to pdo mainly involves removing statements, copy/pasting the list of variables being bound into an array in the ->execute([...]) call, changing fetch statements (see pdo's fetch(), fetchAll() and sometimes fetchColumn()), and using differently spelled methods for things like last insert id and affected rows.

    • Like 1
  12. 41 minutes ago, alexandre said:

    command out of sync

    the most common cause is executing a query that returns a result set but not fetching all the data from that result set. this ties up the connection so you cannot run another query. therefore, if a query returns a result set, simply fetch all the data from the query in to an appropriately named  php variable, freeing up the connection for use by other queries, then test/use that variable throughout the rest of the code.

    • Like 1
  13. On 9/4/2022 at 10:47 AM, mac_gyver said:

    don't store redundant data in multiple locations. you are storing the user's id in the donationclashdetails. that's all the user information you need. you can get any other user information via the user's id.

    the SUM() query to get the total for any/each user should be grouping by the user's id (which is the participationid column). the usernames column should not even be in this table.

    this programing practice falls under the subject of data normalization and will make your queries as fast as possible, reduces the storage requirements, and makes the code/queries needed to manage the data as simple and straightforward as possible.

  14. 29 minutes ago, alexandre said:

    Warning: Undefined variable $participationid in C:\xampp\htdocs\container\donation-clash\donation-clash.php on line 70

    Warning: Undefined variable $participationid in C:\xampp\htdocs\container\donation-clash\donation-clash.php on line 83

    there is no code setting the variable $participationid in the last posted code. you are now seeing those two errors because you eliminated the fatal error, which stopped execution, that was occurring at the incorrect bind_result() statement.

    the sql syntax error is because the SELECT query syntax is incorrect, in that you have a * followed by a column name in the SELECT list.

  15. the error is occurring on line 63. based on the last code you posted, the bind_result() nearest that line is the one after the INSERT/REPLACE query, which doesn't have a result set to fetch data from. if you are removing the bind_result() at about line 16 in the last posted code, that one does belong there because it is associated with a SELECT query, which does have a result set.

    i seriously recommend that before you waste any more time on this, that you switch to the much simpler PDO extension.

  16. since the error mentions the bind_result() statement, the cause of this error has already been stated -

    6 hours ago, mac_gyver said:

    the $stmt->store_result() and $stmt->bind_result() after the INSERT query don't belong there and may be producing errors.

    an INSERT (now a REPLACE) query doesn't have a result set, so there's nothing for bind_result to bind to. at this point we are just repeating information already given. you need to spend more time on your own trying to solve problems.

  17. 3 hours ago, alexandre said:

    reduce the risk of such threat by using prepared queries

    prepared queries provide fool-proof protection against sql special characters in a data value breaking the sql query syntax, which is how sql injection is accomplished. you are already using prepared queries with the mysqli extension. however, as stated, the PDO extension is much simpler and more consistent.

    for the SELECT query that's getting the current user's row of data, this what it looks like using mysqli (with the if(){} and ->close() removed) -

    $stmt = $con->prepare("SELECT userbalance, totaleventjoined, totalpifcoingained, totaldonationdc, userlevel, userexperience FROM accounts WHERE id = ?");
    $stmt->bind_param('i', $_SESSION['id']);
    $stmt->execute();
    $stmt->bind_result($userbalance, $totaleventjoined, $totalpifcoingained, $totaldonationdc, $userlevel, $userexperience);
    $stmt->fetch();

    and here's what that looks like using PDO -

    $stmt = $pdo->prepare("SELECT userbalance, totaleventjoined, totalpifcoingained, totaldonationdc, userlevel, userexperience FROM accounts WHERE id = ?");
    $stmt->execute([ $_SESSION['id'] ]);
    $user_data = $stmt->fetch();

    there's no need for explicit binding of inputs or for the result. you can directly fetch data from a prepared PDO query exactly the same as for a non-prepared query.

×
×
  • Create New...

Important Information

We have placed cookies on your device to help make this website better. You can adjust your cookie settings, otherwise we'll assume you're okay to continue.