Jump to content

mac_gyver

Staff Alumni
  • Posts

    5,352
  • Joined

  • Days Won

    173

Everything posted by mac_gyver

  1. do you have any specific questions about any of the points that were made? if you are just starting out, i recommend that you start with one form field of one type, e.g. text since it is the most common type, and get your code fully working and tested with that single form field. then, pick one field of a different type, and repeat for each type of field. then you can worry about all the code needed for the rest of the fields.
  2. addition points about the posted code - keeping the form data as a set in a php array variable and dynamically validating/processing the data will eliminate most of that code. i see you have an array to hold errors, but are not using it. by definition all post/get/cookie data are strings, regardless of the value they hold, and using is_string() on them will always be true. if you use simple ? positional prepared query place-holders and use implicit binding, by supplying an array to the ->execute([...]) call, all the database specific code will be simplified. if you set the default fetch mode to assoc when you make the PDO connection, you don't need to specify it in each fetch statement. don't copy variables to other variables for nothing. just appropriately name and use the original variables that data is in. don't run multiple queries to get different pieces of data from the same table and row. list out the columns you are SELECTing in a query. you can use the PDO fetchAll() method instead of looping to build an array of the fetched data. when you loop to build the multi-row insert query VALUE terms, you can build the array of inputs that you will supply to the ->execute([...]) call in the same loop. also, if you build the terms in an array, then implode the array with the ',' character, you can eliminate the $i variable and the conditional logic. for the SELECT COUNT(*) ... query, you can just use the fetchColumn() method to get the count value. the INSERT INTO photos... query should be prepared once, before the start of the looping, then just call the execute([...]) method with each set of values as an array to the execute call inside the loop. you would only catch database exceptions for user recoverable errors, such as when inserting/updating duplicate or out of range submitted values. for all other database errors, just let php catch and handle the exceptions, where php will use its error related settings to control what happens with the actual error information (database statement errors will 'automatically' get displayed/logged the same as php errors.)
  3. the code for any page should be laid out in this general order - initialization post method form processing get method business logic - get/produce data needed to display the page html document you should keep the form data as a set in a php array variable, then operate on elements in this array variable throughout the rest of the code. this will eliminate the need to create discrete variables for every field and will let you do things like trim all the data at once, with one single line of code, dynamically validate the data, using a data driven design, and dynamically process the data, so that you don't need to write out repetitive code for every field. you should first trim, mainly so that you can detect if all white-space characters were entered, then validate all input data before using it, storing user/validation errors in an array using the field name as the main array index. to enforce user permissions, you would test at the earliest point in whatever operation you are performing if there is a logged in user and if that user has permission to perform the current operation and is the owner or an administrator of the data being being operated on. in your current code, the permission test for ownership of the place data is somewhere near the middle of the code. this should be right after the point where you have determined that there is form data and that the submit_token is valid. the post method form processing code should - detect if a post method form was submitted before referencing any of the form data. detect if the total size of the form data exceeded the post_max_size setting. if this occurs, both the $_POST and $_FILES data will be empty and there's no point in trying to use any of the form data because it won't exist. trim all the form data at once. verify the submit_token. determine ownership of the data being edited. validate all the input data. after the end of the validation logic, if there are no errors (the array holding the user/validation errors is empty), use (process) the form data. if the processing of the form data could produce duplicate errors for data that must be unique, you would detect this and add errors to the array holding the user/validation errors letting the user know what was wrong with the data that they submitted. after processing the form data, if there are no errors, redirect to the exact same url of the current page to cause a get request for that url. this will prevent the browser from trying to resubmit the form data if that page is browsed away from and back to or reloaded. every redirect needs an exit/die statement to stop php code execution. if you want to display a one-time success message, store it in a session variable, then test, display, and clear that session variable at the appropriate location in the html document. if there are errors at item #7 or #9 on this list, the code will continue on to display the html document, test for and display any user/validation errors, redisplay the form, populating field values with the submtited form data. any value you output in a html context should have htmlentities() applied to it to help prevent cross site scripting. to allow you to initially query for the existing data, then use the submitted form data to populate the form fields, you would only query to get the existing data if the form has never been submitted, then fetch the data into the same php array variable that has already been mentioned for holding the form data.
  4. here's a different approach - <?php echo '<pre>'; print_r($_POST); echo '</pre>'; ?> <img src="btn/colorpicker/darkred.png" data-color='darkred' onClick="pic_color(this)" class="pointer clrimg"> <img src="btn/colorpicker/yellow.png" data-color='yellow' onClick="pic_color(this)" class="pointer clrimg"> <img src="btn/colorpicker/purple.png" data-color='purple' onClick="pic_color(this)"class="pointer clrimg"> <script> function pic_color(el) { // set the form field to hidden document.getElementById('name_pref').type = 'hidden'; // set the form field value to the current element's data-color value document.getElementById('name_pref').value = el.dataset.color; // get the src attribute from the current element and set the pref_img src attribute document.getElementById('pref_img').setAttribute('src',el.getAttribute('src')); // display the pref_img element document.getElementById('pref_img').style.display = 'block'; } </script> <form method='post'> <input type="text" name="name_pref" id='name_pref' style="font-size: 24px;" value=""> <img style="display:none" id='pref_img' class="pointer clrimg"> <br> <input type='submit'> </form>
  5. since you must test the ['error'] element of the uploaded file information, before using any of the file data for a correctly uploaded file, you would test the ['error'] value to see if no file was selected - UPLOAD_ERR_NO_FILE (Value: 4), to skip the file processing code. your post method form processing code should detect if a post method form was submitted, then test if both the $_POST and $_FILES arrays are empty. if they are, this indicates that the total size of the form data exceeded the post_max_size setting. in this case, you would setup a message for the user letting them know that the form data was too large (likely due to the size of the uploaded file) and could not be processed. after you have detected that there is form data, you would test the ['error'] element of the uploaded field information, then proceed based on which value it is. for errors that the user has control over, you would setup and display unique and helpful error messages. for errors that the user doesn't have any control over, you would setup a generic failure message for the user and log the actual error information so that you know what is causing uploads to fail.
  6. when you make the connection using the PDO extension - name the connection variable $pdo or similar so that you can search/distinguish between code that has and has not been converted. set the character set to match your database table's character set, so that no character conversion occurs over the connection (this should always be done, but is rarely set.) set the error mode to exceptions for all the statements that can fail (this is the default setting now in php8+) set emulated prepared queries to false (you want to run real prepared queries whenever possible.) set the default fetch mode to assoc (so that you don't need to specify it in each fetch statement.) since you are probably doing this because you want to convert queries that have external, unknown, dynamic values being put directly into them, into prepared queries, converting from a non-prepared query to a prepared query is simple, when using the PDO extension - remove, and keep for later, the php variables that are being put into the sql query statement. remove any quotes that are around the php variables, any {}, and any extra quotes and concatenation dots that were used to get the php variables into the sql query statement. put a ? place-holder into the sql query statement where each php variable was at. call the PDO ->prepare(...) method. supplying it with the sql query statement. this returns a PDOStatement object. call the PDOStatement ->execute([...]) method, supplying it with an array consisting of the variables you removed in step #1. for a query that returns a result set, fetch the data from the query into an appropriately named php variable. the three commonly used fetch methods are - fetch() (fetch one row at a time), fetchAll() (fetch all the rows at once), and fetchColumn() (fetch a single column from a single row.)
  7. this is a follow-on error, because the prepare() call failed, and you don't have any error handling to detect and stop execution upon an error. you always need error handling for statements that can fail. for database statements that can fail - connection, query, exec, prepare, and execute, the simplest way of adding error handling, without adding code 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 (database statement errors will 'automatically' get displayed/logged the same as php errors.) the exception to this is when inserting/updating duplicate or out of range user submitted data. in this case, you would catch the exception, detect if the error number is for something your code is designed to handle, then setup a unique and helpful message for the user letting them know what was wrong with the data that they submitted. for all other error numbers, just re-throw the exception and let php handle it. to enable exceptions for errors for the mysqli extension, add the following line of code before the point where you make the database connection (note: this is the default setting now in php8+) - mysqli_report(MYSQLI_REPORT_ERROR | MYSQLI_REPORT_STRICT);
  8. the 1st parameter in FIND_IN_SET() would be the column you are trying to match, e.g. FIND_IN_SET(id,?) (no quotes around the name) the 2nd parameter is the set to search in. it is a (string) consisting of a comma separated list of values. you would just implode() the array of values using a comma, and supply that as the prepared query input value. if the job status and/or the array of ids are statically defined in your code, and don't come from external, unknown, dynamic values, why are you going through the trouble of using a prepared query? just put the static values in the sql query statement.
  9. why, exactly, do you think you need to do this? longer-version: you are telling us what you are trying to make work, not what the overall problem is that you are trying to solve. the point of looping over the result from a SELECT query is to use that data somehow. what does this have to do with the if() conditional statement? btw - using mysqli_real_escape_string() on the static string 'Active' is meaningless, and using it in a context that is not a literal string value, e.g. the IN (...) list of values, doesn't provide any protection against sql injection in the value, because there's no string to escape out of. to provide protection against sql injection in this case, you either need to cast each of the ids as integers, or more simply use a prepared query, since it provides protection for all data types, without needing you to use a different protection method for each type. you were using the much simpler and more modern PDO extension, with prepared queries. in one of your previous threads. why have you now devolved to using the overly complicated and inconsistent mysqli extension? if you read my last reply in that thread, you will find that you can use a single prepared query place-holder if you use FIND_IN_SET() instead of an IN() comparison, which requires a place-holder for each value in the list.
  10. what conditions are necessary for your post method form processing code to run? have you checked what values they are? you can use echo '<pre>'; print_r($_POST); to see what the submitted post data is. next, you should NOT attempt to detect if the submit button isset(). there are cases where it won't be. you should instead detect if a post method form was submitted. one such case is if the total size of the form data exceeds the post_max_size setting on the server. in this case, both the $_POST and $_FILES arrays will be empty. after you detect if a post method form was submitted, you should detect this condition and setup a message for the user that the form data was too large (typically due to the size of the uploaded file) and could not be processed. you should also not combine the tests for multiple inputs together into one conditional statement, as this makes error checking and handling/logging difficult. if the CSRF token doesn't match, you would want to display, during development, or log, when on a live/public server, information about the occurrence of that error. there's a bunch of issues with the code that should be addressed - don't output database statement - connection, query, prepare, and execute errors onto a live web page. after you add the line of code that @Barand posted above, to use exceptions for errors for the mysqli extension, remove any existing conditional database error handling logic you have since it will no longer get executed upon an error. don't use the root database user without a password for your applications. create a specific database user, with a password. with only the database permissions that your application needs. delete the sanitize function you have now. you should only trim, then validate input data. htmlspecialchars() is an output function. it is applied when you output data in a html context, right before you output the data. do not use it on input data in your post method form processing code. stripslashes(), when it was needed, was conditionally applied, when magic_quotes_gpc was ON. the need to do this was removed from php a long time ago (deprecated as of PHP 5.3.0 and removed in PHP 5.4. in 2012) client side validation is a nicety for legitimate visitors. you must (trim, then) validate all input data on the server before using it. don't create discrete variables for every input. this is just a waste of typing. instead, keep the (POST) form data as a set, in a php array variable, then use elements in this array variable throughout the rest of the code. once you do item #5 on this list, you can trim all the input data at once using one single line of code. validate each input separately, storing user/validation errors in an array using the field name as the main array index. what you are doing now, by echoing and exiting on each validation error, the user will need to keep resubmitting the form until all the validation errors have been corrected. the uploadProfilePhoto() function should setup a unique and helpful error message for each different problem that will prevent the uploaded file from being used, i.e. don't make the user guess why something they did, failed. after the end of all the validation logic, if there are no errors (the array holding the user/validation errors will be empty), use the submitted form data. the username and email columns must not contain duplicate values. these two columns must be defined as unique indexes. you would then have exception try/catch logic for this insert query, where you would test if the error number is for a duplicate index error, then setup a message for the user (add it to the array holding the user/validation errors), letting them know what was wrong with the data that they submitted. for all other error numbers, just re-throw the exception and let php handle it. after the end of using the submitted form data, if there are still no errors, you would redirect to the exact same url of the current page to cause a get request for that page. this will prevent the browser from trying to resubmit the form data if the page is reloaded or browsed away from and back to. to display a one-time success message, store it in a session variable, then test, display, and clear that session variable at the appropriate location in the html document. if there are errors at item #9 or #11 on this list, the code will continue on to display the html document, where you would test for and display any errors in the array holding the user/validation errors, redisplay the form, and populate the form field values/selected options with the existing data, so that the user doesn't need to keep reentering data over and over upon each validation error. it is at this point where you would apply htmlentities() to the values being output on the web page. in most cases, there's no need to close prepared statements, free up result sets, of close database connections, since php automatically destroys all the resources on a page when your script ends. you need to validate your resulting web pages at validator.w3.org there are errors concerning the <label> tags, the 1st required option choice, stray markup, ... to get a form to submit to the same page it is on, simply leave out the entire action attribute.
  11. $result is a mysqli_result object. it will only be an empty() (false) value when the query fails due to an error. a query that matches zero rows is a successful query, not an error. if all you are doing is testing if a value exists in the database table, without wanting to fetch and use the matching data, use a SELECT COUNT(*) ... query, then fetch and test the count value. if you are going to use the fetched data, just fetch and test the value returned by the fetch statement. if this is for deciding if you are going to insert new data or update existing data, the column in question in the database table should be defined as a unique index, then you would just attempt to insert/update the row of data and test if a duplicate index error (number) occurred. next, don't put external, unknown, dynamic values directly into an sql query statement, where any sql special character in a value can break the sql query syntax, which is how sql injection is accomplished. use a prepared query instead.
  12. why are you using an obsolete and no longer supported php version? as long as your code isn't using any previously deprecated features that have been removed in php8+, most straight-forward code should work as is under php8+. it is more likely that the currently supported versions of php have had the mysqli extension built to use the mysqlnd driver. temporarily switch to one of the php8+ versions and check the phpinfo() output to see if/which database extensions are listed in the mysqlnd section, API extensions. if you post your code, someone could advise how to convert it to use the PDO extension, or could possible do it for you out of shear frustration at how simple the PDO extension is to learn and use over the mysqli extension.
  13. because you only posted a small snippet of the problem, you got a small snippet of a solution. if you want a complete top down solution, you will need to post all the code that demonstrates the extent and limits of this problem. at a minimum, the code for each part of the query will need to be re-written so that it produces the sql query syntax, with the prepare query place-holders in it, the type format string for that part of the query, and an array of the input parameters for that part of the query. you can then merge the dynamically built parts together to produce the compete set of data needed to prepare, bind the input parameters, and execute the query. all the cases where there is conditional logic inside the sql query. e.g. the ternary operator, will need to be rewritten so that the logic can conditionally merge the three pieces of data for each part of the query, together in the correct order. have you actually used the mysqli extension with a prepared query so that you know what the end result will need to be?
  14. just because the mysqlnd driver is loaded, doesn't mean that mysqli (or PDO_mysql) will use it, if mysqli wasn't built to use the mysqlnd driver. the mysqli section, Client API library version, would read - mysqlnd 7.4.33, and the last line in the mysqlnd section, API extensions, would list mysqli. to get this to work, your php installation, regardless of php version (5.3+), must be built to include the mysqlnd driver (which it apparently has been) and the mysqli extension must be built to use the mysqlnd driver (which it has not been.) i'll ask/suggest this again -
  15. as of php8 - either update to php8+ or use a strict comparison ===
  16. it doesn't seem like when the search is for a numerical value, that the name columns should be searched? will any of the name columns ever start with a numerical value? based on the snippets of the implied problem, i would implement this like the following - $opsrch['op_fullname'] = "CONCAT(TRIM(op_firstname),' ',TRIM(op_lastname)) as op_fullname"; $opsrch['sort'] = "op_firstname, op_lastname"; // the search input from wherever it is coming from $srch = '9999'; // a numerical value $srch = 'adam h'; // a string value // since mysqli requires variables in the bind_param statement, produce the search with a trailing wild-card in its own variable $sterm = "$srch%"; $types = []; // array to hold the type format strings $params = []; // array to hold the input variables $where_terms = []; // array to hold the where terms if(!is_numeric($srch)) { // do a name search $types[] = 's'; $params[] = $sterm; $where_terms[] = "CONCAT(TRIM(op_firstname),' ',TRIM(op_middlenames),' ',TRIM(op_lastname)) like ?"; $types[] = 's'; $params[] = $sterm; $where_terms[] = "CONCAT(TRIM(op_firstname),' ',TRIM(op_lastname)) like ?"; $types[] = 's'; $params[] = $sterm; $where_terms[] = "CONCAT(TRIM(op_middlenames),' ',TRIM(op_lastname)) like ?"; $types[] = 's'; $params[] = $sterm; $where_terms[] = "op_lastname like ?"; } else { // do an id search $types[] = 'i'; $params[] = $srch; $where_terms[] = "op_id= ?"; $types[] = 's'; $params[] = $sterm; $where_terms[] = "op_id like ?"; } // build the sql query $sql = "SELECT op_id, {$opsrch['op_fullname']}, op_role FROM prs_op WHERE ".implode(' OR ',$where_terms)." ORDER BY {$opsrch['sort']} LIMIT 40"; // examine the result echo $sql; echo '<br>'; echo implode($types); echo '<pre>'; print_r($params);
  17. you can build the sql query syntax with the same usage as now, with the $opsrch elements. what would change when converting to a prepared query will involve putting ? place-holders in the sql query statement where each value is at, building a type format string, and an array of variables that correspond to the place-holders. if your code is structured so that you are building parts of the query separately, you would build separate type format strings and arrays of input variables for each section, then conditionally concatenate the separate parts of the type format string and conditionally use array_merge() to produce the complete array of inputs that are supplied to the bind_param() statement. another subtle detail, any wild-card characters must be concatenated with the value in the variable, since the bind_param() uses references to the variables.
  18. in your example, does $opsrch['srch'] contain all that WHERE ... sql that also contains the $srch value and if so, where is $opsrch['srch'] built relative to the code you have posted? what php version will this be used with, because as of php8.1, you can simply supply an array of values to the ->execute([...]) call end eliminate all the explicit input binding and the type format string. all you would need to do is build an array with the input values that corresponds to the sql that is being built. edit: as long as all the values being a string data type are okay. i.e. won't work with a LIMIT clause which requires integers. If you are not using php8.1+, to convert this to a prepared query, using mysqli, you need to build the type format string and an array of input parameters that matches the sql that is being built.
  19. the OP's screen prints show something interesting. libmysql is specifically listed, though it is not clear if this is just how the database client is being listed or if both of those are present. on the offending hosting, what does the mysqli section in the output from a phpinfo() statement in a .php script file show? how much overall database specific code do you have? converting to the much simpler and more modern PDO extension would eliminate this problem completely.
  20. your resulting html markup is broken, due to something like the out of date border = attribute value. you need to use modern markup and you need to validate the resulting web pages at validator.w3.org
  21. if you post all the code, less the database connection credentials, someone can provide suggestions on what to start checking. if there's nothing obvious that the code is doing, then you would need to start by profiling/timing the execution of the query(ies) when executed via the php script, to see how long they are taking. however, just based on the symptom, it is likely the php code is doing something extremely inefficient, such as running queries inside of loops, operating on sets of data discreetly, rather than to use php array functions to operate on the set all at once, ...
  22. typical PDO connection code - $DB_HOST = ''; // database host name or ip address $DB_USER = ''; // database username $DB_PASS = ''; // database password $DB_NAME = ''; // database name $DB_ENCODING = 'utf8mb4'; // db character encoding. set to match your database table's character set. note: utf8 is an alias of utf8mb3/utf8mb4 $options = [PDO::ATTR_ERRMODE => PDO::ERRMODE_EXCEPTION, // set the error mode to exceptions (this is the default setting now in php8+) PDO::ATTR_EMULATE_PREPARES => false, // run real prepared queries PDO::ATTR_DEFAULT_FETCH_MODE => PDO::FETCH_ASSOC // set default fetch mode to assoc ]; $pdo = new pdo("mysql:host=$DB_HOST;dbname=$DB_NAME;charset=$DB_ENCODING",$DB_USER,$DB_PASS,$options); php only sends two things to the database server when using prepared queries, 1) the sql query statement, with (optional) positional ? place-holders in it, and 2) an execute command, with an (optional) list of values corresponding to the place-holders. when using true prepared queries, supplying an array to the ->execute([...]) call, preserves the data type of the values, so, this works correctly for strings, numbers, boolean, null, and blob data (when it is less then the max packet size setting.) typical prepared query - $sql = "some sql statement with positional ?, ... place-holders in it where values are to be evaluated when the query is executed"; $stmt = $pdo->prepare($sql); $stmt->execute([ a_value_for_each_place-holder, ... ]); // for queries which return result sets, fetch the data here // see the fetch(), fetchAll(), and fetchColumn() methods // php also has some useful fetch modes, which can do things like pivot/index data by the first column selected, return a single-column one-dimensional array when using fetchAll(), ... you are likely to see example code posted on the web that is using exception try/catch logic around all the database statements that interact with the database server - connection, query, exec, prepare, and execute. ignore this. it is unnecessary clutter that does nothing useful. the only time your code should catch and handle a database exception is for user recoverable errors, such as when inserting/updating duplicate or out of range data. in these cases, you would have try/catch logic (specifically dealing with the ->execute() call), test if the error number is for something that your code is designed to handle, and setup an error message for the user letting them know what was wrong with the data that they submitted. for all other error numbers, just re-throw the exception and let php handle it. in all other cases, simply do nothing, and let php catch and handle the database exceptions, 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.)
  23. forget about using the overly complicated and inconsistent mysqli extension. some additional problems - the procedural and OOP notation have different error responses for the same problem. things that should be and are fatal errors with OOP notation, are just warnings using procedural notation, and the procedural code continues to run, resulting in follow-on errors, not directly related to the problem. some of the features that were added to the mysqli extension, to try to make it more usable, are dependent on the mysqlnd driver being used (and they just corrected one of these but not the others.) if you don't manage your php build, you cannot guarantee that the mysqlnd driver will be used, resulting in code that is not portable between php installations. whoever defined the mysqli result fetch_all default fetch mode didn't know what they were doing. the fetch mode refers to the rows of data in the 'all' array, not the all array indexes. the default fetch mode for every other fetch statement is BOTH. the default fetch mode for the fetch_all statement is numeric, meaning you won't get associative indexes for the rows of data unless you explicitly set the fetch mode in the fetch statement. when you use the PDO extension - set the character set to match your database table's character set, so that no character conversion takes place over the connection. (you should do this for all database extensions, but it is rarely done.) set the error mode to exceptions, so that all the statements that interact with the database server use exceptions (this is now the default setting in php8+ for both the PDO and mysqli extensions.) set emulated prepared queries to false. you want to use real prepared queries whenever possible. set the default fetch mode to assoc, so that you don't need to specify it in each fetch statement. when using prepared queries, use implicit binding, by simply supplying an array to the ->execute([...]) call.
×
×
  • 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.