Jump to content

mac_gyver

Staff Alumni
  • Posts

    5,142
  • Joined

  • Days Won

    165

Everything posted by mac_gyver

  1. you are getting a fatal error in the browser. you need to use your browser's develop tools console tab to debug what is going on in the browser. the element you have given id="selectYear" to, is the button, not the select menu. as to the $cusid. this is already known on the server when the main page was requested. why are you passing it through the form? external data submitted to your site must always be validated before using it. if you use the already validated value that you already know on the server, you can simplify the code.
  2. sorry for how this sounds, but this code is programming nonsense. the biggest problem with it is that OOP is not about wrapping class definitions around main application code, then needing to add $var-> in front of everything to get it to work. all this is doing is creating a wall of unnecessary typing, that's changing one defining and calling syntax for another more verbose one. additional problems with this code is that it is insecure in may places, it is applying htmlspecialchars and strip_tags improperly to input data, it has no validation or error handling logic, it is using a prepared query for queries that don't have any external data being used, it is not properly using a prepared query that have external data being used, it is repetitive, and it's class methods/functions don't accept input data as call-time parameters (requiring even more $var->property statements to make it work.) the only correct programming practices it contains are - it is using dependency injection to make a single database connection available to the classes that need it and it is using a COUNT() query in a few places to get counts of matching data. don't waste any time on using this code, either as a learning resource or as a way of achieving a discussion forum.
  3. data ($_POST, $_GET, $_COOKIE, $_FILES, and some $_SERVER variables) submitted to any site can come from anywhere, not just your forms/links/cookies, can be set to anything, and cannot be trusted. you should trim all external data, mainly so that you can detect if it is all white-space characters, validate it, then use it securely in whatever context it is being use as. in a html context, such as an email body or a web page, you should apply htmlentities() to any external, unknown, dynamic values to help prevent cross site scripting. for a value that has a specific format, like an email address, after you have validated that it is not an empty string, you should validate that it is a properly formatted email address. for something like a contact form, where you don't have the ability to limit its use to only known, logged in users, yes, you should use a captcha. your post method form processing code should - if the form and form processing code are not on the same page, they should be. this will allow you to display any validation errors and repopulate the form field values upon a validation error. detect if a post method form has been submitted. this will insure that if the page is ever requested via a get request, that it won't waste time running code when there is no post data to use. keep the form data as a set in an array variable, then use elements in this array variable throughout the rest of the code, i.e. don't write out lines of code copying variables to other variables for nothing. after you do item #3 on this list, you can trim all the data at once using one single line of code. don't unconditionally loop over the $_POST data. hackers can submit 100's of post variables to your code. you should instead define an array of expected fields, then loop over this defining array when validating and using the submitted data. you should not directly accept the subject from external data. if there's a need for different subject values, define them in an array, with a numerical id indexes, then get the actual subject from the submitted numerical id value. you should have error handling logic for the mail() call. if it returns a false value, the email was not accepted by the sending email server. you would setup a general failure message for the user, and you would log everything about the attempted email so that you will both know that it failed and can perhaps see a pattern as to why it may have failed. you would only display the success content if the mail() call returned a true value. after successfully using the submitted form data, with no errors, you should perform a redirect to the exact same url of the current page to cause a get request for that page. this will prevent the browser from resubmitting the form data if the user reloads that page or navigates away from and back to that page. 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. don't echo static markup. there's no php code in that success message. just drop our of 'php' mode and put the markup in-line in the file.
  4. except during testing, when you enter your own email address, these emails are NOT being set From the name/email address that is entered in the form. they are being sent from the mail server at the web hosting. the domain in the From email address must either directly correspond to the sending mail server or there must be dns zone records (specifically an SPF record) at the domain in the From email address that indicates your email server can send email for that domain. short-version, use a From email address that exists at the web hosting.
  5. there's so much wrong with just this small section of code - it is using a post method form for determining which record to edit. this should use a get method form/link, so that if the user wants to return to the same point, they can bookmark the url or if there's a problem they can submit the url so that someone else can view and see the problem. the only time you should edit/update data is when correcting a mistake in a value or just changing a status value, not when performing normal data operations. for normal data operations, you should insert a new row in an appropriately defined table for every separate operation, so that you have an audit/history trail, that would let you detect if a programming mistake, accidental key press, or nefarious activity caused data to be entered and also so that you can produce a history report. the use of sprintf() provides no security against sql special characters in a value from being able to break the sql query syntax, which is how sql injection is accomplished. the simplest, fool-proof way, to provide security in an sql context, for all data types, is to use a prepared query. since the mysqli extension is overly complicated and inconsistent, especially when dealing with prepared queries, this would be a good time to switch to the much simpler and more modern PDO extension. the database should enforce uniqueness, by defining any column must not contain duplicate values as a unique index. this will produce a duplicate index error when attempting to insert/update duplicate values, that the error handling would detect and report back to the user that the value they tried to insert/update already exists. the code should not use a loop to fetch data from a query that is expected to match at most one row of data. based on the columns oil_due, inspect_date in a table named Trucks, the database is not designed correctly. There should be a table where the one-time/unique information about each vehicle is stored. this table will define a vehicle id (autoincrement primary index) that gets used when storing any related data. service records would then be stored in a separate table, one row per service, using the vehicle id to relate them back to the vehicle they belong to. if there are multiple primary defining records for any vehicle, these should be found and consolidated into a single record, the columns that must be unique need to be defined as unique indexes, and the error handling for any insert/update query need to be changed to detect a unique index error number and report to the user that the data already exists. if this Trucks table is actually the table holding the service records, there need to be a row per service operation per vehicle. the code would then query for any records that have a pending/open status value. when a service is performed, the status for that record would be updated to a value indicating it is 'complete' and what datetime it was completed. any future scheduled service would be inserted as a new row with a pending/open status value.
  6. programming is an exact science. when someone points out a problem with some code and asks what is that code, it means you need to post exactly the code that was mentioned. if someone asks what is the expected value for something, it means to post exactly what that value is.
  7. the error means that $login_level is a boolean (true or false) value, but the code expects it to be an array with a group_status element. either something has changed to cause the find_by_groupLevel() function to return an unexpected value OR that part of the code (which is using an exact comparison with a string consisting of '0') never actually worked, wasn't fully tested, and was probably always producing this error, but the error wasn't being reported/displayed. what is the code for the find_by_groupLevel() function and what is the expected value it should return for both a non-banned and a banned user?
  8. if you are asking how to make the selected option 'sticky', you would output the selected attribute for the option choice that matches any existing gname input value. if you are asking how to narrow down the choices, this is called autocomplete/typeahead and uses javascript and ajax requests to populate a dropdown list with entries from your database that match the portion of text that has been typed in a text input field. you can then just click on the choice you want to complete the entry in the field. there are countless example scripts posted on the web.
  9. you need to attempt to implement each point that was made and observe the result. ask specific programming questions if you get stuck after attempting each point.
  10. the uploaded file is at the path/name given by the ['tmp_name'] element. you must also test if the file upload was successful before referencing any of the uploaded file information. if the total size of the post form data exceeds the post_max_size setting, both the $_POST and $_FILES arrays will be empty. after you detect if a post method form was submitted, you must test for this condition. after you have detected that there is data in $_FILES, you must test the ['error'] element to make sure the file was uploaded without an error. ref: https://www.php.net/manual/en/features.file-upload.php for the $_POST field data, use the $_POST array, not the $_REQUEST array. don't copy variables to variables for nothing, this is just a waste of typing. if you have more than about two form fields, you should use a data-driven design, and dynamically validate and process the form data. except maybe during testing, when you are entering your own email address at your web hosting in the form, these emails are not being sent from the email address that someone enters in the form. they are being sent from the mail server at your web hosting and the from email address must either directly correspond to the sending mail server or you must set the dns zone records at the sending mail server to indicate it is allowed to send email for the domain in the from email address.
  11. that would be exactly the information found in the php documentation - https://www.php.net/manual/en/book.pdo.php specifically for Prepare() and Execute() - https://www.php.net/manual/en/pdo.prepare.php and https://www.php.net/manual/en/pdostatement.execute.php
  12. the points that have been made have INCLUDED the reason why you should do these things. these points have been to make your code - secure (it currently is not), provide a good user experience, simplify (i and others have used a form of the word simple in several of the replies) the code (what you have shown contains a lot of unnecessary and unused code, variables, and queries), reduce resources and speed up the code (making a database connection is one of the most time consuming operations you can do on a page, which is why you should make only one per page), and through having useful error handling and validation logic, help get your code/query(ies) to either work or get them to tell you why they won't. the reason we are trying to get you to simplify the code is because you MUST go though all this code at least once to update it. if you can eliminate the unnecessary code, variables, and queries, you will have less things that must be updated. if you really have hundreds of pages of bespoke code like what you have shown so far, it may be a better use of your time learning and using a general-purpose data driven design, so that you can get the computer to do the work for you, rather than you spending hours upon hours writing and trying to maintain code on so many web pages.
  13. if you make use of the suggestions given in this thread for the LogMeX2 function code, you should end up with this simple, and secure code (untested) - // verify the user log in credentials // return user id if a match, else return false function LogMeX2($pdo,$username,$password) { $sql = "SELECT User, UserKey FROM LIBusersX WHERE UserN = ?"; $stmt = $pdo->prepare($sql); $stmt->execute([$username]); // fetch/test if a row was found, i.e. the username was found if(!$row = $stmt->fetch()) { // username not found return false; } // username found, test password if(!password_verify($password,$row['UserKey'])) { // passwrod didn't match return false; } // username and password matched, return user id return $row['User']; }
  14. nothing has changed about or die() being supported. it was always a bad practice to use for error handling and since the PDO extension uses exceptions for errors for all (default in php8+) the statements that interact with the database server - connection, query, prepare, execute, and exec, you were told the or die() logic will never be executed upon an error and should be removed. the great thing about using exceptions for errors is that your main in-line code only 'sees' error free execution, since execution transfers to the nearest correct type of exception handler upon an error or to php if there is no exception handling in your code. if execution continues to the line right after you have executed a query, you know that there was no error, without needing any conditional logic, simplifying the code. the posted logic makes no sense. you are running a SELECT query to get a count of the number of rows, to decide to run the SELECT query again. just run the second SELECT query and fetch the data from it. if the query matched a row, the fetched data will be a boolean true value. if the query didn't match a row, the fetched data will be a boolen false value. you were also told elsewhere that magic_quotes, which provided some security for string data values, has also been removed from php and that you need to use a prepared query when using external, unknown, dynamic data with a query. converting any query to a prepared query is extremely simple - remove the variables that are inside the sql query statement (keep these variables for use later). remove any single-quotes that were around the variables and any {} or concatenation dots that were used to get the variables into the sql query statement. put a prepared query place-holder ? into the sql query statement where each variable was at. call the ->prepare() method for the resulting sql query statement. this returns a PDOstatement object, and should be named $stmt or similar. call the ->execute([...]) method with an array containing the variables you removed in step #1. for a SELECT query, use either the ->fetch() method (for a single row of data), the ->fetchAll() method (for a set of data), or sometimes the ->fetchColumn() method (when you want a single value from one row of data.) lastly, md5() was never intended for password hashing. you should be using php's password_hash() and password_verify() for passwords. also, why are you hashing names/usernames? this prevents any wild-card searching or sorting.
  15. you have stated you have a lot of code that needs to be converted. a GOOD reason to name the variable as to the type of connection it holds, e.g. $pdo, is so that you can tell by looking/searching which code has been converted and which code hasn't, then after it has been converted, anyone looking at the code can tell which database extension it is using (parts of the mysqli extension look exactly like the PDO extension.)
  16. the OP is attempting to learn, develop, and debug code/query(ies) on a live/public server. either the last posted code wasn't successfully getting uploaded to the server or a lot of cheap web hosting set disk caching (time) to a high value to get as many accounts onto one server as possible and it was taking some time for the changes to take effect. you should be learning, developing, and debugging your code/query(ies) on a localhost development system and only putting your code onto a live/public server when it is mostly completed.
  17. when you make the connection, you should set the error mode to exceptions, which is what you are doing, set emulated prepared queries to false, you need to add this, and set the default fetch mode to assoc, you are setting it to obj, which is not what you are using in the example code and is probably not what your existing code is using with the fetched data, which will require you to make even more changes to your existing code.
  18. the php documentation lists the major changes going between each version - https://www.php.net/manual/en/appendices.php if there's not a huge amount of code, you could just post the entire project somewhere (github) and someone can review and list what things in it will need to be changed. agree with @benanamen, you should be updating to the latest php version, not php7.
  19. the error is due to a counting problem. you have 5 place-holders in the sql query statement, but are only supplying 4 values. as to the posted code - don't attempt to SELECT data first to determine if it exists. there's a race condition between multiple concurrent instances of your script, where they will all find from the existing SELECT query that the data doesn't exist, then try to insert duplicate values. instead, define the username and email columns as unique indexes, just attempt to insert the data, and detect if a duplicate index error (number) occurred. if the data was successful inserted, the values didn't exist. if you did get a duplicate index error, and since there is more than one unique index defined, it is at this point where you would query to find which column(s) contain duplicate values. the SELECT query, in addition to needing = (comparison operators) needs an OR between the WHERE terms, to find any matching rows with either value, not just one row with both values. if you use implicit binding, by supplying an array of values to the ->execute([...]) call, you can eliminate all the bindValue and bindParam calls, simplifying the code. if you set the default fetch mode to assoc when you make the database connection, you won't need to specify it in each fetch statement, simplifying the code. you should use exceptions for database statement errors and only catch and handle a database exception in your code when inserting/updating duplicate user supplied values (which is what you are trying to do.) the PDO extension always uses exceptions for errors for the connection, and in php8+ the default is to use exceptions for all the other database statements that can fail. you would have exception try/catch logic for the insert query, then test if the error number is for a unique index error, then query to find which/both of the two unique columns already contain the submitted values. for all other error numbers, just rethrow the exception and let php handle it. edit: you should also be using php's password_hash() and password_verify() to hash and test the password value.
  20. your login system should only store the id of the logged in user in a session variable. this is the only session variable you should be using related to what you are trying to accomplish in this thread. you would then query the database table(s) on each page request to get any other user data, such as a username, user permissions, membership level, ... the reason for doing this is so that any change made to this user data will take effect on the very next page request (without requiring the user to log out and back in again.) once you have the user data, or lack of, in an appropriately named variable (or instance of a class), you would use it, or lack of, in logic to control what the current user can do or see on the current page.
  21. php builds associative indexed arrays from left to right and top to bottom. when you have the same index name more than once in the data, the last value overwrites any previous value. since you are SELECTing the accounts.id last, that is the value you are getting for the id index. are you even using the accounts.id in the fetched data? if not, don't include it in the SELECT list. likewise for any other data values you are not actually using in the feteched data.
  22. true prepared queries, regardless of the database extension, are equally safe, since they separate the parsing of the sql query syntax from the evaluation of the data. PDO has emulated prepared queries, that should be avoided whenever possible, since, if you haven't set the character set that php is using to match your database tables, which is rarely shown in connection code examples, it is possible for sql special characters in a value to break the sql query syntax, which is how sql injection is accomplished. the PDO examples you have seen that look messier, are probably using named place-holders. PDO also has positional ? place-holders, just like the mysqli extension, which makes converting from using the mysqli extension to PDO straight forward, since the sql query syntax using ? place-holders is exactly the same. to convert to using the PDO extension, you would eliminate the existing msyqli bind_param() call, and supply the array of input data to the ->execute() call. because you can directly fetch data from a PDO prepared query, exactly the same as for a non-prepared query, you don't need to deal with mysqli's bind_result() or get_result() (which is not portable between systems that use and don't use the msyqlnd driver.) another issue with the mysqli extension is that the procedural and OOP notation have different php error responses (though php finally made the mysqli connection always throw an exception upon an error.) things which are fatal problems, that produce fatal php errors when using OOP notation, only produce warnings when using mysqli procedural notation, and allow code to continue to run, producing follow-on errors. an advantage of learning the PDO extension, is that the same php statements work with 12 different database types, so, if you ever need to use a different database type, you don't need to learn a completely new set of php statements for each one.
  23. or you could use the much simpler, more consistent, and more modern PDO extension, which doesn't have any function/method that takes variable length arguments. in fact, you don't need to use explicit binding with the PDO extension. you can simply supply an array of inputs to the ->execute([...]) method call, which can be an empty array if you happen to dynamically build a query that doesn't have any input parameters in it.
  24. you need to post all the code in the forum, using the forum's <> menu button. redact any sensitive information, such as the recpatcha private key.
×
×
  • 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.