-
Posts
5,450 -
Joined
-
Days Won
175
Everything posted by mac_gyver
-
Please Review my code and highlight my mistake
mac_gyver replied to Ehsan_Nawaz's topic in PHP Coding Help
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. -
How to prevent direct url with php and mysql
mac_gyver replied to PNewCode's topic in PHP Coding Help
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. -
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.
-
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.
-
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.
-
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.
-
you need to put the form processing code and the corresponding form on the same page. what you have now takes almost two times the amount of code and by passing messages through the url, it is open to phishing attacks and cross site scripting. the code for any page should be laid out in this general order - initialization post method form processing get method business logic - get/produce the data needed to display the page html document here's a list of points that will help you with your code - validate all resulting web pages at validator.w3.org since you will be putting the form and the form processing code on the same page, you can leave the entire action='...' attribute out of the form tag to cause the form to submit to the same page it is on. if you put the <label></label> tags around the form field they belong with, you can leave out the for='...' and corresponding id='...' attributes, which you don't have anyways. use 'require' for things your code must have for it to work. if you are building multiple web pages, use 'require' for the common parts so that you are not repeating code over and over. the post method form processing code should first detect if a post method form was submitted. in your current code, writing out a series of isset() tests, you are creating bespoke code that must be changed every time you do something different. also, if you had a form with 30 or a 100 fields, would writting out a series of 30 or a 100 isset() statements seem like a good use of your time? forget about this validate() function. it is improperly named and the only thing it is doing that is proper is trimming the data. don't create a bunch of discrete variables for nothing. instead 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. your post method form processing code should trim, than validate all input data, storing user/validation errors in an array using the field name as the array index. after the end of all the validation logic, if there are no errors (the array will be empty), use the submitted form data. the only redirect you should have in your code should be upon successful completion of the post method form processing and it should be to the exact same url of the current page to cause a get request for that page. you should not store plain text passwords. use php's password_hash() and password_verify() you cannot successfully echo or output content before a header() redirect. the only value that you should store in a session variable upon successful login is the user's id. you should query on each page request to get any other user information, such as their username or permissions. if there are user/validation errors, and since you are now putting the form and the form processing code on the same page, your code would continue on to display the html document, display any errors, re-display the form, populating any appropriate fields with there existing values so that the user doesn't need to keep reentering data over and over. any dynamic value that you output in a html context, should have htmlentities() applied to it when you output it, to help prevent cross site scripting. when conditional failure code is much shorter then the success code, if you complement the condition being tested and put the failure code first, it is easier to follow what your code is doing. also, any time you have an exit/die in a conditional branch of code, you don't need to make any following branch 'conditional' since exaction won't continue past that point. don't let visitors (and hackers) know when internal errors have occurred, such as database connection errors. instead, use exceptions for errors and in most cases simply let php catch and handle any exception, where php will use its error related settings to control what happens with the actual error information. also, in the latest php versions, a mysqli connection error automatically uses exceptions, so your existing connection error handling logic will never get executed upon an error and might as well be removed, simplifying the code. the file upload form processing must detect if the $_FILES array is not empty before referencing any of the $_FILES data. if total posted form data exceeds the post_max_size setting, both the $_POST and $_FILES arrays will be empty, you must detect this condition and setup a message for the user that the size of the uploaded file was too large. i suspect this is the reason why you are getting an empty $_FILES array and the undefined array index error. after you have detected that there is data in $_FILES, you must test the ['error'] element. there will only be valid data in the other elements if the error element is a zero (UPLOAD_ERR_OK) you should also setup user messages for the other errors that the user has control over. you can find the definition of the upload errors in the php documentation.
-
just use a single JOIN query to do this. next, you should always list out the columns you are SELECTing in a query so that your code is self-documenting. you should build your sql query statement in a php variable, to make debugging easier and help prevent syntax mistakes, and you should set the default fetch mode to assoc when you make the database connection so that you don't need to specify it in each fetch statement. the reason why your current code doesn't work, is because fetchAll(), the way you are using it, returns an array of the rows of data. if there are three rows in the company table, you will have an array with three rows in it, with each row having an assignedto element. you can use print_r() on the fetched data to see what it is.
-
Sending emails to multiple email ids with nested loop
mac_gyver replied to mythri's topic in PHP Coding Help
how many email addresses per vendor? if there's only one, you don't need the 2nd foreach() loop to fetch it, just directly fetch the single row without using a loop. next you don't even need the 1st foreach() loop, just implode $_POST['vendor'] to make a comma separated list, then use FIND_IN_SET(vendor,?) in the WHERE clause to match all the rows in one query, then just use the result set from that single query. if you use PDO fetchAll() with the PDO::FETCH_COLUMN fetch mode, you will get a one-dimensional array of just the email addresses, that you can directly implode() to produce the comma separated list of email addresses. -
and what is the right result? if it displayed the sql query string, and not a false or null value, all that means is that the query didn't fail with an error. it doesn't mean that the query matched any rows of data.
-
what exactly did you use var_dump() on and what output did it produce?
-
edit: repeats advice already given - use the browser's developer console network tab to see what is being sent from the javascript and returned by the server-side code. next, your search requires an exact match with existing data. you should use a select/option menu instead. also, don't put external, unknown, dynamic values directly into an sql query. use a prepared query instead. you would probably want to switch to the much simpler PDO database extension, since using the mysqli extension with a prepared query is overly complicated and inconsistent. don't unconditionally output raw database connection/query errors onto a web page. the connection error contains the database hostname/ip, username, if you are using a password or not, and web server path information. when a hacker intentionally triggers a connection error, by flooding your site with requests that consume all the available connections, the existing connection error handling will give them 2 or all 3 pieces of information they need to connect to your database. instead use exceptions for database statement errors (the latest php versions automatically does this for mysqli connection errors anyways, so your existing connection error handling will never be executed) and in most cases simply let php catch and handle any database 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.) this will let you remove any database statement error handling logic you have now, simplifying the code. for a query that is expected to match at most one row of data, don't use a loop. just fetch that single row of data.
-
you would need to post your code (use the forum's <> menu button), along with any specific symptoms or errors you are getting, and at what point you are getting them.
-
Notice: Trying to access array offset on value of type null in
mac_gyver replied to Flatronez's topic in PHP Coding Help
if you do this - a majority of this code will go away. assuming i decoded what you are trying to produce as output. the following should be all the code that you need for the data table section - // pre-fetch the data, indexing/pivoting it using the station, then fuel as indexes $data = []; while ($row = sqlsrv_fetch_array($result)) { $data[$row['station']][$row['fuel']] = $row; } // produce the output ?> <table class = "main"> <tr class="main"><td></td> <?php // output the heading foreach($fuel as $fu) { echo "<td class = 'main'>$fu</td>"; } ?> <td class = "main">Atnaujinta</td></tr> <?php // output the data foreach($station as $st) { echo "<tr><td class='main'><a href='levelStation.php?station=$st'><b>$st</b></a></td>"; foreach($fuel as $fu) { if(empty($data[$st][$fu]['vol'])) { echo '<td>-</td>'; } else { $row = $data[$st][$fu]; $class = ''; if ($row['fmm']<=100) $class = 'class="low"'; if ($row['fmm']>100 AND $row['fmm']<250) $class = 'class="alert"'; if ($row['vmm']>=5) $class = 'class="water"'; $qs = []; $qs['station'] = $st; $qs['fuel'] = $fu; $qs['type'] = 'all'; $qs = http_build_query($qs); echo "<td $class><a href='levelFuel.php?$qs'>{$row['vol']}</a></td>"; } } echo "<td>".date_format(date_create($row['date']),"Y-m-d H:i:s")."</td></tr>\n"; } // output the totals ?> <tr><td>Iš viso:</td> <?php foreach($fuel as $fu) { $sum = 0; foreach($station as $st) { $sum += $data[$st][$fu]['vol']??0; } echo "<td class = 'main'>$sum</td>"; } ?> <td></td></tr> </table> -
Notice: Trying to access array offset on value of type null in
mac_gyver replied to Flatronez's topic in PHP Coding Help
the code is unconditionally looping over some number of $fuel choices and expects there to be corresponding data in $cellt. IF (a big if) the data in $cellt has gaps in the 1st index, then the code should be testing if $cellt[$k] isset() before referencing it. what does adding the the following, after the point where $cellt is being built, show - echo '<pre>'; print_r($cellt); echo '</pre>'; this code is apparently (hard to tell without all the actual code, sample data, and desired output) outputting data for each station (starts a new section when the station value changes) and each fuel type. the code is repetitive, because it must complete the previous section when a new section is detected, and it must also complete the last section at the end. you can greatly simplify all of this by pre-fetching the data into an array and indexing/pivoting it by first the station, then the fuel. to produce the output from the pre-fetched data then just takes a couple of loops, without repetition in the code or extra variables to detect when the station changes. you should also use associative index names for the fetched data, so you are not having to keep track of numbers. lastly, in the code building the links, the only thing that is different is the class='...' markup. this is the only thing that should be conditional. the rest of the markup is identical and should only exist once in the code - Don't Repeat Yourself (DRY.) build the class='...' markup in a php variable, then use that variable when building the single link. you should also build the query string key/values in an array, then use http_build_query() to produce the query sting for the link. -
you are getting a http 500 error because the code contains a php syntax error. you don't know this because you haven't put the php error related settings into the php.ini on your system, so that ALL php errors will get reported and displayed. when you put these settings into your code, they don't do anything for php syntax errors in that file since your php code never runs in this case to change those settings. the approximate point where the syntax error is occurring at can be seen since the color highlighting stops changing, both in your programming editor and in the forum post. if you always build sql query statements in a php variable, it helps prevent syntax errors like this by separating the sql query syntax as much as possible from the php code. you can also help prevent syntax errors like this by using a prepared query (which will also prevent sql special characters in a value from being able to break the sql query syntax) when supplying external, unknown, dynamic values to the query when they get executed, since you are only putting a simple ? prepared query place-holder into the sql syntax for each value.
-
just trying different things, and throwing them away when they don't work is not learning. an analogy to the method you are trying to use, would be picking up a piece of a jigsaw puzzle, trying it in every possible location, without looking at what part of the picture it likely matches so that you are trying it in places it has a chance of fitting. this method takes 100 times longer, and sometimes never, to accomplish putting together the puzzle. writing code involves actually learning the meaning of the words and syntax you are using, so that you are writing (and reading) the programming language you are using. without this learning, you cannot debug code when it doesn't work, because you don't know what each statement contributes, and you also cannot write new code, building on what you have learned before, because no learning has occurred to serve as the basis for new learning. you must somehow make the transition from just copy/pasting words you have found to actually learning (internalizing) the meaning of the words, so that you can write meaningful code that does what you want. another reason to learn what the words actually mean and do, is because a large percentage of the code you will find posted on the internet is lacking in security, validation, and error handling. they may 'work' under perfect conditions, but they aren't secure, and when anything unexpected occurs, they won't tell you why they didn't work. here's an example from your original code that appears to work, but isn't actually doing what you think, that reading what the lines of code are doing would point out - $id=$_SESSION['id']; $fname=$_POST['fname']; $email=$_POST['email']; $select= "select * from users where id='$id'"; $sql = mysqli_query($conn,$select); $row = mysqli_fetch_assoc($sql); $res= $row['id']; if($res === $id) all the code on this web page 'requires' a logged in user for the form processing code to be executed or the form to be displayed. to accomplish this, your code must validate the session variable first, before using it, e.g. does it even exist. the code must also test if the query matched a row of data before using the fetched data. the above code, when the session variable doesn't exist ends up testing if null (the query will fail to match a row of data, resulting in $res being null) === null (the nonexistent session variable), which is true comparison, and allows anyone who isn't logged in to cause the UPDATE query to be executed. next, for any post method form processing code, you should not attempt to detect if the submit button is set. there are cases where it won't be. you should instead detect if a post method form was submitted. one of the cases where the submit button won't be set is if the size of the form data exceeds the post_max_size setting on the server. this can occur for any post method form, but is more likely to occur when uploading files, since the size of the uploaded file is out of your control. when this condition occurs, both the $_POST and $_FILES arrays will be empty. after detecting if a post method form was submitted, the code must detect if this condition exists, and setup an error message for the user, since there's no form data for the rest of the code to use. once your code has found that there is form data, you must test the ['error'] element in the $_FILES... array, which is why there's an error element to test. testing the ['name'] element is not enough. for some of the possible upload errors, there will be a valid ['name'] element, but the file upload has failed with whatever error is indicated in the ['error'] element.
-
the error message confirms that the problem is as stated in the first paragraph of my reply. if you are getting an error about an - wouldn't the solution be to make it into a quoted associative index name, e.g. by adding quotes around it?
-
the most likely thing that stopped the code from working are all the $_POST[unquoted associative index name here] in the main code (but not inside of overall double-quoted strings.) in the past, one of php's unfortunate choices assumed that if a defined constant by the given index name doesn't exist, the value was treated as a string. this no longer works and is a fatal run-time error. setting php's error_reporting to E_ALL and display_errors to ON, preferably in the php.ini on your system, will cause php to help you by reporting and displaying all the errors it detects. this code is insecure. by outputting the raw form data on the web page, in the emails, and saving it to a file/importing it into excel, any code in the values will allow cross site scripting. at a minimum, you need to apply htmlentities() to all the values when they are being output, to prevent any html, css, javascript, ... in them from being operated on. the code also has no validation of the input data before using it, could stand to be rewritten in an organized and simplified format, needs error handling and file-locking for the file operations, and needs the markup updated.
-
or use BC (BCD - Binary Coded Decimal) math - https://www.php.net/manual/en/ref.bc.php
-
perhaps if the OP posts the incorrect result and points out what is wrong with it, it would provide a clue as to the problem.
-
Php Mysql Remove from one row and add to another. I'm stuck
mac_gyver replied to PNewCode's topic in PHP Coding Help
the following is an example showing the code and queries needed to accomplish this task - <?php // initialization // these settings should be in the php.ini on your system - error_reporting(E_ALL); ini_set('display_errors', '1'); // Start the session session_start(); // fake a logged in user $_SESSION['user_id'] = 1; // insure the current visitor is logged in if(!isset($_SESSION['user_id'])) { // handle a non-logged in user here... header('location:index.php'); die; // stop code execution } // use exceptions for database statement errors and only handle the exception for user recoverable errors // based on the sql syntax error posted, you ARE using exceptions for database statement errors. do yourself a favor and remove all the existing database error handling logic since it is no longer being used and is just cluttering up your code. require 'pdo_connection.php'; $post = []; // array to hold a trimmed working copy of the form data $errors = []; // array to hold user/validation errors // post method form processing if($_SERVER['REQUEST_METHOD'] === 'POST') { // input(s) - gname, this should be the selected user's id - e.g. gid or similar // trim all the input data at once $post = array_map('trim',$_POST); // if any input is an array, use a recursive trim call-back function here instead of php's trim // validate the input data // the selected user's id if($post['gid'] === '') { $errors['gid'] = 'You must select a user'; } else if($post['gid'] == $_SESSION['user_id']) { $errors['gid'] = 'You cannot select yourself'; } else { // find if the selected user exists $sql = "SELECT COUNT(*) FROM users WHERE id=?"; $stmt=$pdo->prepare($sql); $stmt->execute([ $post['gid'] ]); if(!$stmt->fetchColumn()) { // if you see this error, it is either due to a programming mistake or someone submitting data values that are not from your code $errors['gid'] = 'The selected user does not exist'; } } // if no errors, use the data if(empty($errors)) { // since it is doubtful the OP will change what they are doing, the following updates the value in the token column // start transaction $pdo->beginTransaction(); // subtract 1 token from the current user, if they have enough tokens $sql = "UPDATE users SET token=token-1 WHERE token > 0 AND id=?"; $stmt=$pdo->prepare($sql); $stmt->execute([ $_SESSION['user_id'] ]); if($stmt->rowCount()) { // the UPDATE query affected the row, token was > 0 // add 1 token to the recipient user $sql = "UPDATE users SET token=token+1 WHERE id=?"; $stmt=$pdo->prepare($sql); $stmt->execute([ $post['gid'] ]); } else { // the UPDATE query didn't affect the row, token is not > 0 $errors['token'] = "You don't have enough tokens"; } // if you are here, no database errors/exceptions occurred, commit the transaction $pdo->commit(); } // if no errors, success if(empty($errors)) { $_SESSION['success_message'] = "You have successfully gifted a token"; die(header("Refresh:0")); } } // get method business logic - get/produce data needed to display the page $sql = "SELECT id, username FROM users ORDER BY username"; $stmt = $pdo->query($sql); $all_user_data = $stmt->fetchAll(); // html document starts here... ?> <?php // test for, display, and clear any success message if(!empty($_SESSION['success_message'])) { echo "<p>{$_SESSION['success_message']}</p>"; unset($_SESSION['success_message']); } ?> <?php // test for and display any errors if(!empty($errors)) { echo '<p>'.implode('<br>',$errors).'</p>'; } ?> <?php // display the form, repopulating form fields using any data in $post ?> <form method='post'> <select name='gid'> <option value=''>Select a user</option> <option value='234'>An invalid user</option> <?php foreach($all_user_data as $row) { $sel = isset($post['gid']) && $post['gid'] == $row['id'] ? 'selected' : ''; echo "<option value='{$row['id']}' $sel>{$row['username']}</option>"; } ?> </select><br> <input type='submit'> </form> this code renamed the session variable, because many things have 'id's, what exactly is the id stored in the session? uses the much simpler and more modern PDO database extension. -
Php Mysql Remove from one row and add to another. I'm stuck
mac_gyver replied to PNewCode's topic in PHP Coding Help
the logged in person is known from the $_SESSION['id'] variable, which your code should validate before using, i.e. this page requires a logged in user for it to do anything, and your code should test to make sure that the current visitor is logged in. the form should submit just the id of the selected recipient. any data that you store should use the user ids, not the name/username. this will result in the least about of storage and the fastest queries. why are you making 3 different database connections? if these databases are on the same server, make one connection, then just specify the database name in the query. you should be using exceptions for database statement errors and only catch and handle the exception for user recoverable errors, such as when inserting/updating duplicate or out or range user submitted data. in all other 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 can then eliminate the existing error handling logic, since it will no linger get executed upon an error, simplifying the code. note: in the latest php versions, a mysqli connection error always throws and exception, so that the existing error handling logic you have won't ever be executed upon an error. use a prepared query when supplying external, unknown, dynamic values to a query when it gets executed. this simplifies the sql query syntax, provides protection for all data types, not just strings, and if you switch to the much simpler and more modern PDO database extension, simplifies the php code. don't copy variables to other variables for nothing, just use the original variables. this is just a waste of time typing. you should NOT maintain a single number in a column to keep track of the number of tokens. this does not provide an audit trail that would let you detect if a programming mistake, accidental key press/button click, or nefarious activity cause the value to change. you should instead insert a row of data for each transaction that affects a value. to get the current amount, just sum() the quantity from all the rows of data belonging to the current user. don't bother closing database connections in your code since php will automatically destroy all resources when your script ends. your post method form processing code should - detect if a post method from was submitted. keep the form data as a set in an array variable, then reference elements in this array variable throughout the rest of the code. trim all the input data at once. after you do item #2 on this list, you can accomplish this with one single line of code. validate all inputs, storing user/validation errors in an array using the field name as the array index. after the end of the validation logic, if there are no errors (the array will be empty), use the form data. if after using the form data (which could result in more errors), if there are no errors, redirect to the exact same url of the current page to cause a get request for the page. 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 step #5 or #6, the code will continue on to display the html document, where you would test and display any user/validation errors, and re-display the form, repopulating form fields with the submitted form data. any dynamic value that you output in a html context should have htmlentities() applied to help prevent cross site scripting.