-
Posts
5,535 -
Joined
-
Days Won
191
Everything posted by mac_gyver
-
while these points probably have nothing to do with the performance problem (with the exception of the invalidate markup), the code needs to be refactored, corrected, simplified, and cleaned up - this code is filled with unnecessary copying of variables to other variables and repetitive code that only differs in a variable name/title. just use the original variables that data is in and use a template/function/data-driven design instead of writing out code for every value. you want to always report all php errors. when learning, developing, and debugging, you want to display all php errors. when on a live/public server, you want to log all php errors. the error related settings should be in the php.ini on your system, so that you can set or change them at a single place. use 'require' for things your code must have for it to work and be consistent in what you use. include/require are not functions. the () around the file name do nothing. leave them out. the session variable with the user id should either be set and contain the id or it should not be set. don't bother testing if it is an empty string. it should never be one. many things have have ids. the session variable holding the user id should be uniquely named, something like user_id. there's no good reason to destroy the whole session. a session can contain more than just the logged in user's id. all inputs need to be trimmed, then validated before being used. if 'required' input(s) are not valid, don't run the code that's dependent upon them. every redirect needs an exit/die statement to stop php code execution. currently, all the code on the page runs, even if there is no logged in user. don't use or die(...) for error handling. use exceptions for database statement error handling (which is the default setting now in php8+) and in most cases simply let php catch and handle any database exception. mysqli_error() requires the connection variable as a parameter. this problem will go away when you switch to using exceptions for database errors. use a prepared query when supplying external, unknown, dynamic values to a sql query when it gets executed. if it seems like using the mysqli extension is overly complicated and inconsistent, especially when dealing with prepared queries, it is. this would be a good time to stitch to the much simpler and more modern PDO extension. while there should be a user matching the logged in user id, if there isn't, all the code dependent on there being a user should not be executed. a true value returned by mysqli_query() only indicates that the query executed without error. it doesn't mean that the query matched a row of data. you should instead test that the fetch statement returned a true value. also, all the rest of the code is dependent on there being fetched data. if there isn't, none of the remaining code should be executed. when mapping an input value to an output value, don't write out conditional logic for every value. use a 'mapping' array instead. there are a number of html markup errors. you should validate the resulting html at validator.w3.org don't repeat yourself (DRY) don't repeat conditional tests, just put everything in one conditional test. multiple spaces in html markup render as a single space. if you do have a need for multiple spaces, you would use a instead of escaping double-quotes in the php produced markup, use single-quotes. as to the performance problem, you need to profile the execution of the code, i.e. measure the time it takes for different sections of code to execute. i would start by timing the execution of the code from the start up to the QR code, the QR code, the generation of the html markup, and each of the main dompdf statements. once you know where the greatest amount of time is being taken, you can concentrate on finding and fixing the performance problems in that section of code.
-
you would need to post an sql dump with some sample data and the logo file to get specific help. a likely performance problem is using .png for the QR image and putting that image in several places in the document. if you convert it to a jpeg image, it should render more quickly as the jpeg format is natively supported by dompdf. to produce a jpeg for the QR image, see this link - https://phpqrcode.sourceforge.net/examples/index.php?example=711
-
if both arms_name_long and arms_name_short must be unique, you would want to detect which one or both are duplicates and display a separate and specific message in the appropriate span tag(s). as to how to do this, your database design must enforce uniqueness. both of those database table columns should be defined as unique indexes. you would then just attempt to insert the row of data and in the exception error handling, detect if a duplicate index error number (1062) occurred. It is at this point where you would build and execute a SELECT query to find which of the values are duplicates. here's a laundry list of things you should/should not be doing in this code - the for='...' attribute in the <label ...> tag must match the corresponding form field's id='...' attribute OR if you put the closing </label> tag after the form field, you can leave out the for='...' attribute entirely and if not used for anything else, leave out the id='...' attribute. for problems like item #1, you need to validate your resulting web pages at validator.w3.org any value you output in a html context needs to have htmlentities() applied to it to help prevent cross site scripting. if you use the null coalescing operator (??), it will simplify things like the value='...' attribute logic - value='<?=htmlentities($_POST['arms_long_name']??'',ENT_QUOTES)?>' the ids you use in a database context should be generated via autoincrement primary index columns, not using php random numbers. if you did have some need to generate unique random numbers and store them in a database table, you would need to insure uniqueness by defining the column holding them as a unique index and perform a duplicate index error check as described above for the arms_name_long and arms_name_short values. 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. you should trim all input data before validating it, mainly so that you can detect if values where all white-space characters. once you do item #6 on this list, you can trim all the data at once using a single php array function. you should apply any ucwords() and strtoupper() modification to values only when you output them. the value you output in the form field value='...' attribute should be the trimmed, validated value, not the raw field value. when you make the database connection, you need to - set the character set to match your database table's character set, set the emulated prepared query setting to false (you want to run real prepared queries whenever possible), and set the default fetch mode to assoc (so that you don't need to specify it in each fetch statement.) i'm assuming that the ALTER TABLE... query is temporarily in the code and will be removed? btw - the ->query() method call executes the query. you don't need an additional ->execute() call and i'm not sure what doing so in this case will accomplish. if you use simple ? prepared query place holders and simply supply an array of input values to the ->execute([...]) call, it will greatly reduced the amount of typing you have to do for each sql query. as already discussed, you would first attempt to insert the data, then in the exception error handling, detect if the query produced a duplicate index error number. if it did, you would then execute a SELECT query to find which column(s) contain the duplicate values. for any other error number, you would simply rethrow the exception and let php handle it. you should not output raw database statement errors on a live/public server, as this gives hackers useful information when they internationally do things that trigger errors. if you only catch and handle duplicate (and out of range) database query exceptions in your code, and let php catch and handle all other database exceptions, php will 'automatically' display/log the raw database statement errors the same as its error related settings are setup to do for php errors.
-
Php Require or Include inside of IF/ELSE statement
mac_gyver replied to meOmy's topic in PHP Coding Help
this is nonsense code that has been floating around on the web for a long time. the only thing it is doing that is proper is trimming the value. stripslashes, when it was needed, should have been conditionally applied only when magic_quotes_gpc was on, but the (bad) feature in php that needed stripslashes to be applied to input data was removed from php in 2012, over a decade ago. htmlspecialchars is an output function. it is applied to data being used in a html context. it is not used on input data that your script will use for database operations as it modifies the value. so, even if this function only trimmed the data, the proper place to use it would have been before validating the data, so that you would be validating the trimmed values. -
Php Require or Include inside of IF/ELSE statement
mac_gyver replied to meOmy's topic in PHP Coding Help
because you have no logic to do this. the else() you have is part of the last if(){} conditional, for the phone number. you should NOT use discrete variables for the error messages. this would require you to either have an additional flag to indicate any errors or you would need to write a conditional test with all the discrete variables. you should instead use an array (arrays are for sets of things where you are going to operate on each member in the set in the same or similar way) to hold the user/validation errors, using the field name as the main array index. after the end of the validation logic, if the array holding the errors is empty, you can use the submitted form data, e.g. - if(empty($errors)) { // use the submitted data here... } to display the errors at the appropriate location in the html document, either test the contents of the array and display all the errors together or display each error adjacent to the field it corresponds to. speaking of/writing about using arrays for sets of things, you should keep the form data as a set in a php array variable, then operate on elements in this array variable throughput the rest of the code. this will allow you to trim all the data at once, using one php array function, and will support dynamically validating and processing the data. speaking of/writing about dynamically validating the data, you should NOT write out - copy/paste logic for every possible value. you should instead use a data-driven design, where you have a data structure (database table, array) that holds the dynamic values, then use this definition to control what general purpose logic does. the only thing that is different between these validation types is the regex pattern. why not store them in an array with the data type, regex pattern and error message, then just get the correct entry and call one general purpose function with the correct type regex pattern and the input value? whatever the code is for this function is probably improper. in general, besides trimming input data, you should NOT modify user submitted data as this changes the meaning of the data. if data is valid, use it. if it is not, tell the user what is wrong with it and let the user fix then resubmit the data. -
here's even more issues - the email column should be defined as a unique index. this will prevent duplicate email addresses, which could be the cause of the current symptom. then, in the registration code, the exception error handling for the INSERT query would test for a duplicate index error number, and setup a message that the email address is already in use. when you make the database connection, you need to set the character set to match your database table's character set, so that no character conversion occurs over the connection. this is doubly important when using emulated prepared queries, which you are using but which should be avoided whenever possible, so that sql special characters in a value won't be able to break the sql query syntax. when you make the database connection, you should set the emulated prepared query setting to false, i.e. you want to use true prepared queries whenever possible. when you make the database connection, you should set the default fetch mode to assoc, so that you only get the type of fetched data that you want and so that you don't need to specify the fetch mode in each fetch statement.
-
@LeonLatex, because you haven't posted the registration and login forms (you could have a typo between the field names and the php code), aren't trimming and validating the inputs before using them (any of the inputs could be empty or contain additional characters due to typo mistakes or something javascript is adding) , and have lumped together the test for the user row and password_verfy() (you currently have no single place to test if a user row was not found, then if the password_hash() didn't match), no one here can determine why this isn't working. you could also be overwriting the $password variable if your actual code is requiring the database connection code right before executing the query. edit: here's another couple of problems with the form processing code you have posted. you should only store the user_id in a session variable, then query on each page request to get any other user data, such as permissions/roles, so that any change in these other values will take effect on the very next page request after they have been changed. the redirect should be to the exact same URL of the current page to cause a get request to be registered in the browser for that URL. by redirecting to a different URL, anyone can walk up to a computer that was used to register/login using this code and browse back to the form page, which will cause the browser to resubmit the form data, where you can use the browser's developer console network tab to see what the submitted form data is.
-
@Phi11W, password_hash() produces a different hash for the same input, every time it is called, because it generates a random salt each time it is called. this is precisely why password_verify() must be used. it takes the algorithm, cost and salt from the existing hashed value, hashes the submitted password using these values, then compares the resulting hash of the submitted value with the existing hashed value.
-
where is the error being generated at or seen at? i suspect is it a php error message being displayed in the 'chat' output? the reason it may appear differently is because it contains html markup, which may or may not get rendered by the browser. out of memory errors as usually caused by logic that either has an initial condition problem, where an assumption being made isn't satisfied, or a loop that runs forever and keeps adding data until all memory is consumed, without removing older data.
-
you should not modify data, then use the modified value, as this changes the meaning of the data (ask the author's of this forum software about the email 'cleaning' they did a number of years ago that allowed hackers to create a real and valid email addresses that was similar to an administrator's, that after 'cleaning' allowed it to be used to do a password recovery for that administrator, and allowed the hackers to log in as that administrator.) you should only trim user submitted data, mainly so that you can detect if it was all white-space characters, then validate that the trimmed value meets the 'business' needs of your application. if the data is valid, use it securely in whatever context it is being used in. if the data is not valid, tell the user what was wrong with the data and let them correct and resubmit it.
-
the above only produces a php warning if $conn is a false value. it also results in a false value for the if() conditional test, which looks like a connection without an error (this is yet another f___ up with the mysqli extension implementation.) it is also due to the different error responses between procedural mysqli and oop mysqli statements. you should be using exceptions for database statement errors (this is the default setting now in php8+) and in most cases simply do nothing else in your code and let php handle any database 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.) you can then eliminate all the existing error handling logic, since it won't get executed upon an error, simplifying your code. a) you need to set php's error_reporting to E_ALL and display_errors to ON, preferably in the php.ini on your development system, so that php will help you by reporting and displaying all the errors it detects. stop and start your web server to get any changes made to the php.ini to take effect and use a phpinfo() statement in a .php scrip to confirm that the settings actually took effect. b) you should be using php8+. c) to use exceptions for the mysqli extension in older versions of php, add the following line of code before the point where you make the database connection - mysqli_report(MYSQLI_REPORT_ERROR | MYSQLI_REPORT_STRICT);
-
From the documentation - it seems like you should be using an UPDATE query for this operation?
-
if you had properly stored your data as has been previously suggested in your threads, with date and amount columns and a row for each piece of data, this would be a simple task that could be accomplished in a straight-forward sql query. but because you have a year column and separate month name columns, your code must take the year and month from the 'from' and 'to' values and figure out which rows and which columns to use to calculate the result.
-
PHP MYSQL Insert Array on Same Table Row
mac_gyver replied to experience40's topic in PHP Coding Help
to normalize this data, you need an attribute table, with - id (autoincrement primary index) and name columns. as new attributes are defined, they would be inserted into this table. this table establishes attribute ids. the map table would map the csv column number to these attribute ids. you would JOIN with this table when displaying information to get the meaningful names for each attribute id. the stock table would not be laid out as a spreadsheet. you would instead insert only the unique one-time information for each item into the stock table. this would establish a stock id for each item for each user/feed. you would then have a stock attribute table with - id (autoincrement primary index), stock id, attribute id, and value columns. you would insert a separate row into the the stock attribute table for each csv column value for each stock id. when you are inserting the data, you would query the map table to get the set of rows for the user/feed and fetch these into an array. when you read each row of data from the csv file, you would then loop over this array of map data, use the csv column number to get the data value from the row of csv data, then use this data value and the attribute id for executing the stock attribute insert query. -
Display search result by joining tables in php
mac_gyver replied to johnman's topic in PHP Coding Help
the following is the syntax definition for a SELECT query - SELECT [ALL | DISTINCT | DISTINCTROW ] [HIGH_PRIORITY] [STRAIGHT_JOIN] [SQL_SMALL_RESULT] [SQL_BIG_RESULT] [SQL_BUFFER_RESULT] [SQL_CACHE | SQL_NO_CACHE] [SQL_CALC_FOUND_ROWS] select_expr [, select_expr ...] [FROM table_references [WHERE where_condition] [GROUP BY {col_name | expr | position} [ASC | DESC], ... [WITH ROLLUP]] [HAVING where_condition] [ORDER BY {col_name | expr | position} [ASC | DESC], ...] [LIMIT {[offset,] row_count | row_count OFFSET offset}] [PROCEDURE procedure_name(argument_list)] [INTO OUTFILE 'file_name' [CHARACTER SET charset_name] export_options | INTO DUMPFILE 'file_name' | INTO var_name [, var_name]] [FOR UPDATE | LOCK IN SHARE MODE]] the relevant parts that you need for this query are - SELECT select_expr , select_expr ... FROM table_references WHERE where_condition ORDER BY {col_name | expr | position} [ASC | DESC], ... you should list out the columns you are selecting, i.e. don't use * there would only be one FROM keyword in this particular query. the table_references section is where the JOIN part of the query goes. the where_condition would contain the columns, comparison operators, and prepared query place-holders for the brand and location matching. almost every query that can return a set of rows should have an ORDER BY ... term so that the result is in a desired order. the existing sql statement should be producing a query error and probably php errors. you should use exceptions for database statement errors (which is the default now in php8+) 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.) you would set the PDO error mode to exceptions when you make the database connection. you should also set emulated prepared queries to false, so that you use true prepared queries. do you have php's error_reporting set to E_ALL and display_errors set to ON, preferably in the php.ini on your development system, so that php will report and display all the errors it detects (which will now include database statement errors)? as to the rest of the code - the form processing and form should be on the same page. this will result in the least amount of code and provide the best user experience. the code for any 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. you need to validate the resulting web pages at validator.w3.org don't prepare and execute a non-prepared query. just use the ->query() method. if you set the default fetch mode when you make the database connection, you won't need to specify it in each fetch statement. don't use the ->rowCount() method. it is not guaranteed to work with SELECT queries. since you have fetched all the rows of data from the query into a php variable, you can just test that variable to determine if the the query matched any rows. for the required attribute to work for a select/option menu, the value attribute for the 1st option/prompt choice must be an empty string (the w3.org validator will point out this problem.) the form should be 'sticky' and re-select the option choices that match any existing search data, so that if the search doesn't match things the user is interested in, the user can just make a different selection and resubmit the form. in the location query, listing the column name twice does nothing. you only need to specify the column name once in the query. you can reference it as many times are you want in the result set. if you put the label tags around (before/after) the field they belong with, you can eliminate the for='...' and corresponding id='...' attributes, which will eliminate more of the markup errors (ids must be unique.) if you use implicit binding, by suppling an array of values to the ->execute([...]) call, you can eliminate the bindParam() statements. if you use simple positional prepared query place-holders ?, you can eliminate the repetitive typing of place-holder names in the query and php code. -
While with foreach not giving correct value
mac_gyver replied to Adamhumbug's topic in PHP Coding Help
the way to correct and simplify this is to - this will eliminate all the sumOpenAndAcceptedValueByClient() usage. the single query will give you the quote total (accepted and open) and open total (open only) values per client_id. you would then just use those values as you are looping to produce the output. i hope you are not doing this in every function that needs a database connection? this is creating multiple database connections per instance of your script and a database connection is one of the slowest operations you can perform. your main code should create one database connection, then supply it to any function that needs it as a call-time parameter. you should also use 'require' for things that your code must have for it to work. if you eliminate all the bindColumn() statements and just fetch each row into $row, you would reference each column value as $row['column_name'], which is probably how the code was originally written. this will also eliminate the multiple different names you are using for the same meaning data. the id/client_id will never be null from this query since it is from the primary table. this conditional test is unnecessary. -
this is just indexing/pivoting the data by a column. PDO has a fetch mode that will 'automatically' do that for you, assuming that the sectionName is the first column in the SELECT ... list - $data = $stmt -> fetchAll(PDO::FETCH_GROUP); if for some reason you require the name values to be referenced as itemName in the fetched data, assign it that name as an alias in the sql query.
-
On duplicate key update always inserting new row
mac_gyver replied to Adamhumbug's topic in MySQL Help
the place-holder mismatch will work using emulated prepared queries (which should be avoided whenever possible), but not for true prepared queries. the emulator is more of an approximator, because there are differences that you can detect between how it works vs a true prepared query. -
Function runs great when called isolated but only that way
mac_gyver replied to Javiigahe's topic in PHP Coding Help
the user table would hold the common one-time information. you would have a second table holding the unique information for the different types of users, with a separate row for each piece of information, related back to the user table through the user id. -
Function runs great when called isolated but only that way
mac_gyver replied to Javiigahe's topic in PHP Coding Help
the problem is you are using the $password variable for two different things. 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 have one user database table with both client and psychologist registration/login data and a 'type' column holding a value that indicates which type of user they are. Don't Repeat Yourself (DRY.) it is not the responsibility of the sendOTPmail() function to create a database connection. you already have a database connection in the application, supply it to any function that needs it as a call-time parameter. -
While with foreach not giving correct value
mac_gyver replied to Adamhumbug's topic in PHP Coding Help
what's the difference between accepted and open and why doesn't the first query use accepted = 1 too? i'm betting that the $out = ""; variable in the first posted code is being used to build the output in the image, listing the results by the left-hand ranges (guessing per client), and the echo statements are just there for debugging? this is going to be a case of - don't show us your non-working attempt and expect us to figure out what's wrong, instead show us sample data and what result you expect from that data. also - don't waste your time with bindColumn() statements, this is wasted typing. just fetch the data and reference the data by its associative index (column) name. lastly, why are you preparing a non-prepared query that doesn't have any values being supplied to it? just directly use the ->query() method. -
while there probably is a way of doing this in a query, with a sub-query/self-join, i would just perform the calculation when you produce the output.
-
you would add GROUP BY client.id to the query to get a SUM() per client. in fact, if your database was set to STRICT mode, you would be getting an error with the current query about using an aggerate function without a GROUP BY term. then, if you want both the SUM() per client and the grand total, you can add WITH ROLLUP to the end of the GROUP BY ... term.
-
change the value of ID of newest group of fields
mac_gyver replied to jasonc310771's topic in Javascript Help
client-side validation is a nicety for legitimate visitors. since you must validate the data on the server before using it, why go to all this trouble to duplicate the logic in the client that you (already) have in the server-side code? and if you want to pre-validate the data in the client, why not just send the relevant data values to the server using ajax, run it through the (existing) validation logic, and display the result returned by that validation logic? -
change the value of ID of newest group of fields
mac_gyver replied to jasonc310771's topic in Javascript Help
what item? we cannot help you unless you post the complete code the reproduces the problem. in programming, there are just too many different ways of accomplishing a task. it takes knowing how you go to a point, in order to help you.