-
Posts
5,510 -
Joined
-
Days Won
185
Everything posted by mac_gyver
-
Can this email validation page be improved?
mac_gyver replied to foxclone's topic in PHP Coding Help
at the success point in the form processing code, you can log or insert the data in a database table so that you have a record of what should have been emailed. some mail servers are configured to silently ignore some invalid attempts and the code could appear like it's sending, when it actually isn't. -
Can this email validation page be improved?
mac_gyver replied to foxclone's topic in PHP Coding Help
if you apply all the corrections and suggestions, you should end up with something that looks like this (tested except for actually sending the email) - <?php // initialization session_start(); $email_contact = "[email protected]"; $email_website = "[email protected]"; // definition of permitted types/subject/category. use to dynamically build the option list, // pre-selecting any existing choice, and used in the validation logic $permitted_types = ['Questions', 'Report Problem', 'Suggestion', 'Other', 'Website Problem']; $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') { // trim all the data at once $post = array_map('trim',$_POST); // if any of the fields are arrays, use a recursive call-back function here instead of php's trim function // inputs: name, email, type/subject/category, message - all required // validate the inputs if($post['name'] === '') { $errors['name'] = 'Name is required.'; } if($post['email'] === '') { $errors['email'] = 'Email is required.'; } else { // while it is true that the following email format validation will produce an error // for an empty value, you should specifically tell the visitor what is wrong with what // they submitted if (false === filter_var($post['email'], FILTER_VALIDATE_EMAIL)) { $errors['email'] = 'The Email Address you entered does not appear to be valid.'; } } if($post['type'] === '') { $errors['type'] = 'You must select a Type/Subject/Category.'; } else { // you will only see the following error due to a programming mistake or someone/something submitting their own values if(!in_array($post['type'],$permitted_types)) { $errors['type'] = 'The selected Type is invalid.'; // you would want to log the occurrence of this here... } } if($post['message'] === '') { $errors['message'] = 'Message is required.'; } else { if(strlen($post['message']) < 10) { $errors['message'] = 'The Message must be at least 10 characters.'; } } // if no errors, use the submitted data if(empty($errors)) { // apply htmlentities() to help prevent cross site scripting when viewing the received email in a browser $formcontent = htmlentities("From: {$post['name']}\r\nEmail: {$post['email']}\r\nSubject: {$post['type']}\r\nMessage: {$post['message']}", ENT_QUOTES); if ($post['type'] === "Website Problem") { $recipient=$email_website; } else { $recipient=$email_contact; } // add $post['email'] as a Reply-to: header if desired, it is one, valid email address at this point $mailheader = "From: $recipient\r\n"; if(!mail($recipient, $post['type'], $formcontent, $mailheader)) { // an error // setup the user error message $errors['mail'] = 'The email could not be sent, the site owner has been notified.'; // system error handling goes here... - datatime, get the last error message, include the mail parameter values... // at this point, all parameters are either an internal value, have been validated they they are just an expected value/format, or have had htmlentities() applied. } // if no errors at this point, success if(empty($errors)) { $_SESSION['success_message'] = "Mail Sent. Thank you {$post['name']}, we will contact you shortly.."; die(header("Refresh:0")); } } } // html document starts here... ?> <?php // display any success message if(!empty($_SESSION['success_message'])) { // for demo purposes, just output it as a paragraph. add any markup/styling you want echo '<p>'; echo htmlentities($_SESSION['success_message'], ENT_QUOTES); echo " - <a href='index.php#home' style='color:#ff0099;'> Return Home</a>"; echo '</p>'; unset($_SESSION['success_message']); } ?> <?php // display any errors if(!empty($errors)) { // for demo purposes, just output them as a paragraph. add any markup/styling you want echo '<p>'; echo implode('<br>',$errors); echo '</p>'; } ?> <?php // (re)display the form here..., re-populating the fields with any existing values ?> -
Can this email validation page be improved?
mac_gyver replied to foxclone's topic in PHP Coding Help
it's your contact form. for those users that it may not work for, how could they let you know? -
Can this email validation page be improved?
mac_gyver replied to foxclone's topic in PHP Coding Help
there's something that can be fixed, simplified, and even removed in just about every line of code. the code is insecure (several places), provides a bad User eXperience (vague, cryptic user messages and it appears that the form and from processing code are on different pages), contains unnecessary and unused variables and code, has no error handling that would tell you (the developer) when and why it doesn't work, and contains an operational problem that will probably make it not send emails from actual visitors (which may have actually worked for the testing you did.) a list just going from top to bottom - the php error related settings should be in the php.ini on your system, so that they can be changed in a single place. when you put your application onto a live/public server, you want to log all php errors (you don't want to display them as this gives hackers useful information when they intentionally do things that cause errors.) if the php.ini on your live server is already setup to do this, you don't have to touch your code when moving it. with the settings in your code, you have to find and edit/remove these lines of code every place they exist. Keep It Simple (KISS.) you should detect if a post method form has been submitted, rather than to detect if a specific field is set (and don't try to detect if the submit button is set because there are cases where it won't be.) by hard-coding a specific field in the test, you are creating more work every time you write new code. the two lines after the main comment in the code are configuration settings. any value that is application specific in the body of the code should be set in this section, so that you don't need to go through the code to find all of them or to remove them when posting code in a help forum (do you really want your email addresses to get indexed by search engines on phpfreaks?) despite you editing the first of these two variables, neither are being used anywhere in the code. the died() function is ridiculous and has been floating around on the internet for years. you should display any user/validation errors when you re-display the form and repopulate the form fields with any existing entered/selected/checked values, so that the visitor doesn't need to keep reentering things over and over. the form processing code and the form should be on the same page. the user should not need to be pressing any keys to get to the point of correcting any errors. this function does not support doing any of this. the long list of isset(), which are now empty(trim()) calls, is pointless. firstly, if you had a form with 30 fields, would writing out a statement like this make sense? (your answer should be no.) next, except for unchecked checkbox/radio fields, all other fields will be set once the form has been subtitled. this line does nothing useful. don't copy variables to other variables for nothing. this is just a waste of time typing and making typo mistakes. something that would be useful, would be to trim all the data values, before validating them. this can be done with one single line of code, by keeping the input data as a set, in an array variable, then operating on elements in this array variable throughout the rest of the code. use the same name for any particular item throughout the code. you are referring to one item as 'type', 'subject', and 'category'. you should store user/validation errors in an array, using the field name as the array index, and don't store html markup as part of the message. this will let you preform dependent validation tests only when appropriate, output the error next to the corresponding field if you want, and keep any markup/styling in the html document where it belongs. to test if there are no errors, the array will be empty. to display the errors at the appropriate location(s) in the html document, you would test if the array is not empty. as @gizmola already pointed out, use php's filter_var with the FILTER_VALIDATE_EMAIL flag. the if($subject='') { line only uses one =, so it is an assignment operator, not a comparison operator. the next test, if(($subject)!=('Questions'||...)) { doesn't work at all. you would need an array of the permitted choices (which you should have anyway, so that you can dynamically build the select/option menu), then use in_array() to test if the submitted value is not in the array of permitted choices. the clean_string function is another piece of junk found on the internet. delete it and forget you ever saw it. it doesn't even do an case-insensitive replacement, so just including a capital letter in any of the values will bypass it. you are also not using the $email_message that's being built with calls to it in it. any external, unknown, dynamic value that you output in a html context, such as an email body or on a web page, should have htmlentities() applied to it to help prevent cross site scripting. these emails are NOT being sent from the email address that someone has entered in the form (except perhaps during your testing.) they are being sent from the mail server at your web hosting. the From: mail header must contain an email address that corresponds to your web hosting. you can include the submitted email address as a Reply-to: mail header, after validating that it is exactly and only one validly formatted email address. the error handling for the mail() call should setup and display a helpful message for the visitor, e.g. - The email could not be sent, the site owner has been notified, and you would LOG all the information you have abut the problem such as the datetime, the actual error returned by the mail() call, and the values that were used in the mail() call. by sending the response email to the arbitrarily submitted email address, you are allowing spammers/bots send email through your mail server. forget about doing this. just setup and display the success message. you should actually switch to use either the phpmailer or swiftmailer class. they take care of formatting message bodies and mail headers, so you will have more success at getting your emails to work. after the successful completion of the form processing code (no errors), you should perform a header() redirect to the exact same url of the current page to cause a get request for that page. this will prevent the browser from re-submitting the form data if the user reloads the page or browses away from and back to that page. if you want to display a one-time success message, store it in a session variable, then test, display, and clear that variable at the appropriate location in the html document, not forgetting to apply .htmlentities() to any external, unknown, dynamic value in the message. -
When i press publish button code can't run
mac_gyver replied to Ehsan_collboy's topic in PHP Coding Help
where in your markup are the <form ...> </form> tags for the post method form? -
Contact form does not work on dynamic site (blank data)
mac_gyver replied to rsdias's topic in PHP Coding Help
edit: it turns out the main problem isn't in the form or form processing code - the reason for the incorrect operation of the form(s) is because the markup on the page is broken. there's an opening <form .. tag on line 33 in navbar.php, for a search feature, that is never closed. since nested forms are invalid, any <form tag after that point on the page is ignored. i determined this by noticing the browser address bar changing to include the form field values, meaning a get method form was in effect. since the intended form is a post method form, i searched through the code to find all form tags, leading to the one on line 33 of navbar.php, with no corresponding closing </form> tag. you need to validate all your resulting web pages at validator.w3.org some items for your form processing code - the form processing code and the form should be on the same page. this results in the simplest code and the best User eXperience (UX.) the form processing code should first detect if a post method form has been submitted, trim, then validate all the input data, storing user/validation errors in an array using the field name as the array index, then only use the data if there are no validation errors. if there are validation errors, you would display them when you re-display the form and re-populate the form field values with the existing values so that the user doesn't need to keep reentering data over and over. they can just correct the validation errors. the only redirect in your code should be 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 user reloads the page or browses away from and back to that page. if you want to display a one time success message, dialog box, alert,... store the message in a session variable, then test, display, and clear that session variable at the appropriate location in the html document. -
Contact form does not work on dynamic site (blank data)
mac_gyver replied to rsdias's topic in PHP Coding Help
the only thing we can deduce based on the above two items is that there's something wrong somewhere, starting with the form and ending with however the code (finally) reaches the 'home' page. to directly solve this, we need see the actual (not a picture) of all the code needed to reproduce the problem. this indicates that the form processing code isn't first detecting if a post method form was submitted before doing anything at all, since a link would cause a get request for the page. this also indicates that the form processing code doesn't have any/enough server-side validation logic. the email should never get sent if there's no input data. -
the form definition (each field's - id, label, type, name, placeholder text, choices/options, required attribute, validation attributes, ...) needs to be stored in a data structure (database table or array) and to allow for multiple different forms, the data structure should hold multiple definitions, each one receiving a unique id (auto-increment primary index.) the logged in user would then select which form definition to fill in, or if only one definition, be presented with that form. to save each field's entered/selected/checked value, you would need to use ajax to submit the data to the server. the storage of the data on the server would be similar to a database based shopping cart/ordering system, where you would have a table holding the unique/one-time information about the instances of a form being filled out with - an id, the user_id, the form_id (from the defining structure), datetime, status, ... this establishes a form_instance_id. you would use this id to store the submitted form field data in a second table with - an id, form_instance_id, form_field_id, value, ... when the visitor returns to the site and logs in, you would query the data to find if there are any form(s) in progress (the status column would be a value other than the one that corresponds to 'complete'.) you would give the user the choice of picking from among the incomplete form(s) or starting a new instance of a form. if they pick an incomplete form, you would query for the existing data values to re-populate the fields with. when you output the form, you would set the focus to the field after the last submitted value, i.e. the next form_field_id after that in the last row in the table holding the submitted data.
-
Contact form does not work on dynamic site (blank data)
mac_gyver replied to rsdias's topic in PHP Coding Help
slightly off topic, but don't do the following - $_GET['p'] can be anything and cannot be trusted. by submitting a value that uses directory traversal, any .php file anywhere on your server can get required, allowing for example, an administrative action page to be accessed, since the resulting path/file will exist and that code will happily require and execute it for the current visitor. you must validate all inputs before using them. you must test in your code that the $pagina value is only and exactly a permitted, directly accessible page. back to the topic, we cannot help you with what's wrong with your code unless you post all the code need to reproduce the problem. there are multiple different ways to accomplish a task, and without knowing exactly what your code is, no one can simply tell you what to do to make your code work. -
no. it only involves adding one table, a table that defines the checkbox choices, which i specifically mentioned in a reply. in @Barand's reply that shows the table structures, the user_demo table and your form table perform the same purpose (his table only has columns necessary for the demonstration), his user_transport and your vehicle table perform the same purpose and even look the same (after you fix storing an id instead of an abbreviated name), and the transport_type table is the checkbox choices defining table. no, the type of loop is correct, what it is looping over is not. if you look at both the code that he and i have posted, at the point of producing the checkbox markup, you will see that everyone is using a foreach loop, but we are both looping over the definition of the checkbox choices, adding the checked attribute when there is an existing, checked choice. in your code, you are looping over an array of the existing, checked choices. what can that array contain? from zero to three rows of data, resulting in zero to three sets of markup. does that make sense to you? the first three lines of your code make no sense toward getting this task to work. you are testing if some unknown variable is not empty, looping over an array of data, that is apparently the existing checked data, then setting the first variable to be an element of the data you are looping over. actually, that won't work unless there is already at least one checked choice for each one in the vehicle table.
-
no it doesn't. if it did it, it would always produce 3 checkboxes with the existing choices pre-checked. you need to break down problems into the individual steps need, then design, write, test, and debug the code for each step, before going on to the next step. you need to get to the point where you can first, always, produce the 3 checkboxes. the code i posted above is a very simple stand-alone working example. i assumed that $table is somehow the definition of the checkboxes, which what you do need to loop over to produce the output. however, it is apparently the existing choices, which is not the data you need to be looping over. if you run my code, you can observe and learn how it works. there will initially be 3 unchecked checkboxes, because there's no code to query for the existing choices. if you check any/combination of the boxes and submit the form, the boxes will retain their checked state. this is a feature you will need as part of general-purpose form processing/form code, where there can be other form fields, and you don't want to reset all the fields back their initial values should there be a validation error in any of the fields. once you understand how to dynamically produce the 3 checkboxes, you can move onto the next step of querying for and retrieving the existing choices to initially use to pre-check the checkboxes with.
-
what you are missing, relevant to both my reply above and the reply by @Barand, is a table where the (currently 3) vehicle choices are DEFINED. it is this definition that will allow you to produce the checkbox markup without writing out code for every possible value. see the following example form processing/form code, related to the method stated in my reply above (there are comments in the code at the point where you would query for and retrieve the existing data to be edited) - <?php // you would query to get the following data from whereever it is stored // assuming that $table in the OP's code is an array of fetched data defining the checkbox choices $table = []; // the index is the id // do you really want the 'I have a' text with every entry? how about just a heading to select the vehicles the user has? $table[1] = ['name'=>'I have a yacht']; $table[2] = ['name'=>'I have a super car']; $table[3] = ['name'=>'I have a plane']; $post = []; // array to hold a trimmed working copy of the form data $errors = []; // array to hold user/validation errors // recursive function to trim data function _trim($val){ if(is_array($val)){ return array_map('_trim',$val); } else { return trim($val); } } // post method form processing code if($_SERVER['REQUEST_METHOD'] == 'POST') { // trim all the data at once $post = array_map('_trim',$_POST); // validate the data here, storing validation errors in $errors, using the field name as the array index // if no errors, use the submitted data if(empty($errors)) { // insert new data or update existing data here... } } // get method business logic - get/produce data need to display the dynamic content on the page // if editing existing saved choices, there would be a get input to validate, then used to query // for and retrieve the existing choice data, but only if the form has never been submitted ($post // is empty), storing the fetched data in $post, in a sub-array named 'vehicle', in the same format // as the form will submit it as. // html document starts here... ?> <form method='post'> <?php // display the checkboxes, with any existing choices pre-checked foreach($table as $id=>$arr) { $chk = in_array($id,$post['vehicle'] ?? []) ? ' checked' : ''; // note: if you put the <label></label> around the form field, you don't need to generate unique ids to make it work echo "<label class='boxstyle'><input type='checkbox' name='vehicle[]' value='$id'$chk> {$arr['name']}</label><br>\n"; } ?> <input type='submit'> </form>
-
if you find yourself writing out code for each possible value, it's a sign that you are doing something the hardest possible way. just to add a value, you would need to edit the code and add another repeated block of code. the list of (currently 3) choices should be defined in a data structure (database table, array), with a numerical id (auto-increment primary index) and a name/label. it is this definition that you would loop over to produce the output. if you are pre-checking existing choices when you are looping to produce the output, either based on some stored choice ids or due to submitted ids from a form, you would test if the current id being output is in an array (see php's in_array()) of the existing choices to determine if the 'checked' attribute should be output.
-
Warning: count(): Parameter must be an array or an object
mac_gyver replied to jarvis's topic in PHP Coding Help
all that did was hide the actual problem. if your code is expecting an array of data as its input, it's an application error if it isn't. either a programming mistake or an incorrect user action (directly requesting a delete action page without first visiting the page selecting what to delete) caused the expected input to not be the expected data type. for programming mistakes, you need to find an fix the root cause of the problem. for an incorrect user action, you need to setup and display a message telling the user either what they did wrong, i.e. you cannot directly request this page, or what they need to do to correct the problem, i.e. you must select at least one notification to delete. -
wherever you found this, forget you saw it. here's everything that's bad about this - don't use output buffing unless you WANT to buffer output. by using it to make bad code work, you are hiding mistakes in the code and it will make finding and fixing problems harder. don't conditionally start a session. if you want to use sessions, simply, unconditionally start them once, in an initialization/configuration file. as already stated about the error, and stated in the documentation for session_set_cookie_params(), you must call this before the session_start. session_set_cookie_params(0) is only valid in php8, where that usage would set the session.cookie_lifetime to zero, but the default is already zero. set_time_limit(0) and ini_set('max_execution_time','3600') set the same thing, so these are first setting the max_execution_time to zero (which you should never do since a programming mistake that causes a script to never end will require that you restart the web server to regain control), then sets it to 3600. you cannot set post_max_size in a php script, because the value is used by the server before a php script is ever invoked.
-
$response[0]['results'] is the array of data that you should be looping over in the foreach loop. $value['campaign']['name'] would be what to echo (don't create variables for nothing.)
-
as the error states, you cannot set the session cookie parameters after a session has been started. you must call session_set_cookie_params() before calling session_start().
-
Telnet response does not output all data (using PHPTelnet Class)
mac_gyver replied to jaybo's topic in PHP Coding Help
your print_r() output (you should use var_dump instead) actually confirms this. when you print_r a true value, you get a 1. when you print_r a false value, you get nothing. the print_r output indicates a false eof value, i.e. you are not at the end of the file, with a zero unread_bytes value. -
Telnet response does not output all data (using PHPTelnet Class)
mac_gyver replied to jaybo's topic in PHP Coding Help
the php documentation for socket_get_status/stream_get_meta_data has a note to not use the unread_bytes value in a script, which is what the posted code's do/while loop is using, probably because it is just the number of bytes currently contained in the PHP's own internal buffer. this does not indicate that you are done reading the stream, just that there's a pause in the data received such that you have emptied php's internal buffer. in the same documentation, for the eof value - whoever wrote and tested this code probably had a situation such that unread_bytes was always non-zero until the end of the data. -
has a post method request ever worked on this server? do you have any .htaccess redirecting or is this server behind a proxy server?
-
i did NOT state that you should do that. we are trying to determine why a post method form does not work.
-
these type of problems, where it looks like the code should technically work, are often due to copy/pasting code found on the web, where it has been published and the characters, due to the character encoding being used, aren't actually what they appear to be, or someone typing non-ascii characters as part of the syntax (the base characters for the actual syntax must be straight ascii characters.) what does changing the print_r($_POST) to print_r($_GET) show, i.e. the default form method is get when an unknown value is used for the method attribute? you can also add echo $_SERVER['REQUEST_METHOD']; at that point in the code to see what the browser is submitting the data as. if this shows get data/get method, delete and TYPE the entire method='post' attribute (you may in fact need to delete and re-type the entire <form ...> tag if there are some non-ascii characters in it.) btw - your form and form processing code should be on the same page. this will result in the simplest code and the best User eXperience (UX.)
-
Group by Year and Month but prevent month title repitition
mac_gyver replied to jarvis's topic in PHP Coding Help
when you fetch and index/pivot the data, index it by both the year and the month - $data = []; foreach ($results as $result) { $data[$result->Year][$result->Month][] = $result; } to produce the output - foreach($data as $year=>$arr) { // start a new year section here... echo "<h3>$year</h3>"; foreach($arr as $month=>$rows) { // start a new month section here... $monthName = date('F', mktime(0, 0, 0, $month, 10)); echo "<h4>$monthName</h4>"; foreach($rows as $row) { // output the data under each month here... echo "<p>$row->Title</p>"; } } } -
you are missing a logical operator after one/between the isset() statements. most of this typing is unnecessary. when a form has been submitted all the form fields with be set except for unchecked radio and checkbox fields, which you would detect and validate separately. are any of these inputs radio or checkboxes? If not, then that code is a waste of typing.
-
it means that there is no fetch() method. if you are going to use a while(){} loop, you would use the fetch_array() method. if you are going to use a foreach(){} loop, you can directly iterate over the rows in the result set.