Jump to content

Psycho

Moderators
  • Posts

    12,157
  • Joined

  • Last visited

  • Days Won

    129

Everything posted by Psycho

  1. Also, make sure that you are trying to load the file by going through a web server (even if it is a local server): e.g. http://localhost/somefile.php You cannot just open the file in a web browser: e.g. file:///C:/path/to/file/somefile.php
  2. You must not have implemented it correctly, because what you show is what would be in the URL when the form is submitted. The code I provided will create a URL based on the form fields and do a redirect to that URL - the form is never submitted. Because your form is still being submitted, either the javascript is not being called or there is an error. Show the code you have.
  3. You can SUBMIT a form via GET or POST and you cannot change how the data is sent. There is a standard. However, you could just use the form to redirect to a page based on the values in the form. Create a function that is called when the form is posted and have it take the values and dynamically generate the page to redirect to. Modified form tag <form id="search-form" action="/collections/" method="get" onsubmit="return submitAction(this);"> Function to execute when form is submitted. The return false is required to prevent the form from submitting as it normally would. Also note that this is just a bare bones, "example" function. You need to validate that the fields have necessary data before trying to redirect. You should spend some time to complete this. function submitAction(formObj) { var root = formObj.action; var type = formObj.elements['type'].value; var make = formObj.elements['make'].value; var year = formObj.elements['year'].value; var model = formObj.elements['model'].value; var href = root + type + '/' + make + '+' + year + '+' + model; window.location.href = href; return false; }
  4. Psycho

    Spacing issues

    You have some invalid HTML. 1. Multiple elements have the same ID. (e.g. 'asbooked1') 2. Some elements are using invalid parameters (Labels don't have a name parameter) 3. A lot of unnecessary code Give this a try <html> <head> <style> .forminput_wrapper { width: 49%; min-width: 300px; height:30px; white-space: nowrap; border-bottom:1px solid #565656; border-radius:0; font-family:'Open Sans'; font-size:13px; color:#B4B4B4; line-height: 30px; } input { padding: 0px; } .forminput_wrapper select { border:none; font-family:'Open Sans'; font-size:13px; color:#B4B4B4; width: 49%; } .checkboxLine label { padding-left: 5px; } .checkboxLine input[type="checkbox"] { margin-top: 10px; float: right; margin-right: 49%; } </style> </head> <body> <div id="asbooked1_" class="forminput_wrapper checkboxLine"> <label for="asbooked1">As Booked</label> <input type='checkbox' name='asbooked[]' value='2' id='asbooked1' /> </div> <div id="username-wrapper1" class="forminput_wrapper"> <select id="houroptions" value="" name="fromhour[]" placeholder="From Hour (hh)" > <option>From Hour (hh)</option> <option>00</option> ... <option>23</option> </select> <select id="minuteoptions" value="" name="fromminutes[]" placeholder="From Minute (mm)" > <option>From Minutes (mm)</option> <option>00</option> ... <option>55</option> </select> </div> <div id="password-wrapper1" class="forminput_wrapper"> <select id="houroptions1" value="" name="tohour[]" placeholder="To Hour (hh)" > <option>To Hour (hh)</option> <option>00</option> ... <option>23</option> </select> <select id="minuteoptions1" value="" name="tominutes[]" placeholder="From Minute (mm)" > <option>To Minutes (mm)</option> <option>00</option> ... <option>55</option> </select> </div> <div id="asbooked1" class="forminput_wrapper checkboxLine"> <label for="didnotwork1">Did Not Work</label> <input type='checkbox' name='didnotwork[]' value='2' id='didnotwork1' /> </div> </body> </html>
  5. When dealing with an indeterminate number of variables, using prepared queries is cumbersome. Although some would disagree, a possible solution would be to force the values to be integers to ensure they are safe to use in a query. Non-numerical values will default to 0, which (depending on the context) would not match any values anyway. <?php $classIDs = array_filter(array_keys($_SESSION['cart']), 'intval'); $query = 'SELECT * FROM cooking_classes WHERE class_id IN (' . implode(', ', $classIDs) . ')'; $result = mysqli_query($conn, $query) or die(mysqli_error()); ?>
  6. Good to hear, but as I stated, IF the same keyword exists in a file the previous code will not find it. Also, there is one small flaw that may not be consequential. strpos() will return 0 if the search term is at the beginning of the haystack string. So, if any placeholder exists at the very beginning it will not be found. The following will find multiple occurrences of a placeholder in a file and will also find ones if they are at the very beginning. public function findPlaceholders() { $filenames = array("index.html", "areas.html", "testing.html"); $placeholders = array("{TITLETAG_PLACEHOLDER}","{METADESC_PLACEHOLDER}","{METAKEYWORDS_PLACEHOLDER}","{H1_PLACEHOLDER}","{AREALINKS_PLACEHOLDER}"); foreach($filenames as $filename) { echo "<br/>Filename: {$filename}<br/>\n"; $source = file_get_contents($this->skeleton_dir.$filename); foreach ($placeholders as $placeholder) { $offset = 0; while(strpos($source, $placeholder, $offset)!==false) { $position = strpos($source, $placeholder, $offset); echo " - Placeholder {$placeholder} found inside the file named {$filename} at position {$position}<br/>\n"; $offset = $position +1; } } } }
  7. Are you saying that different placeholder in the same file are not being reported or that the same placeholder in a file isn't being reported? Looking at your code, I see no reason why different placeholders would not be found. But, if the same placeholder exists in a file it will only find the first one because you are only checking once.
  8. If you don't want users to see the URL of the file (which they shouldn't see), then the url shoudl not be anywhere in the HTML code for the page. Instead you would have an ID that is statically associated with each URL and that shoudl be the value of the options. But, you are dynamically creating a unique ID when you create the form, so when the ID is passed upon form submission there is no way for your code to know which record the ID is for. I don't know why you are reading this data from a text file. You should instead have a database with each record having a unique ID in the database. then when the user submits their selection you can match up the ID with the appropriate record in the DB. A less optimal solution is to have that ID in the data file. Although I would put the data into an array options_file.php <?php $options = array( 1 => array('title' => 'Title 1', 'url'=>'http://somedomain.com/url1'), 2 => array('title' => 'Title 2', 'url'=>'http://somedomain.com/url2'), 3 => array('title' => 'Title 3', 'url'=>'http://somedomain.com/url3') ); ?>
  9. Assuming that this default value is the only one with an empty value, I would suggest checking the value of the field instead of the text/label. Labels can and do change (e.g. showing different labels based on the user's preferred language), but the values should not change.
  10. To add a little more context to Jacques1's response. If a user entered "ain't" as the search term, the query that would be built using the current code would be The single quote in the word "ain't" closes the start of the search string leaving the t%' content hanging outside the search string - thus creating an invalid query. As Jacques1 was stating someone could use this security hole to do nasty things in your database by using content that would create a valid query(ies) to do as they please. To prevent such a problem the content need to be escaped so the database understands that the quote mark in the word "ain't" is part of the search term and not a closing quote for the term. In the old days we would use a function that would escape certain characters based on the Database type (for MySql this would put a backslash before certain characters). Now, with prepared queries, as you are using, you need to put a placeholder in the prepared query and then pass the value to use for that placeholder when executing the query. The database engine will escape the values automatically. Be sure to pay attention to how Jacques1 instantiates the DB connection. There is a parameter he included to make sure the prepared queries are actually doing what they are supposed to do - the default is to only emulate prepared statements which is far less secure.
  11. For this part, you did not provide the necessary information. Where are the names stored? Are they stored in "yourtable"? If so, include the name field in the SELECT query. If the names are in a different table, then you should have a FK (Foreign Key) in the "yourtable" so that you can JOIN the records in the "yourtable" to the records in the related table with the names.
  12. That was by no means a thorough analysis - it was just a handful of things that I readily saw. There were plenty of other things, I just don't have the time to comment on everything.
  13. Some other feedback: 1. You are using GLOBAL in a function - don't! It is a bad practice. Instead, pass the variables to the function that it needs. You are creating a lot of them, so rather than pass each variable as individual values, create the data in an array then simple pass the array to the function. 2. Give your variables and functions meaningful names. test2_1() doesn't tell you anything about what that function does. Yes, as you are actively working on it you know what id does, but when you need others to help you with code or when you need to go back to code that is even a day or two old you have to relearn what they are. 3. You are trying to enforce business rules that you should not. For example, you are forcing the names to start with an uppercase character and the remaining letters to be lower case. There are many names where this would be invalid: "De La Hoya", "da Vinci", etc. 4. You have some 'validations' that have holes. E.g. !empty ($_POST["Lname"]) If the user just enters one or more spaces it will pass this validation. Additionally, values such as "0" or "FALSE" would cause that function to return the same as an empty string. For a name that might be fine, but in other contexts the string "false" or a "0" (zero) may be legitimate values. Instead, you should first trim() the value before checking to see if a value was passed. 5. There are conditions with no handling of the false results. E.g. if(isset($_POST["Lname"]) && !empty ($_POST["Lname"])) { $Lname = $_POST["Lname"]; if(preg_match("/[^a-zA-Z]*$/",$Lname)){ //### Upper Case First Letter of Last Name $LnameUc = ucfirst($Lname); } else { echo"<strong>The Last Name you entered is not valid.</strong>". PHP_EOL; echo "<br />\n"; exit; } } What happens if $_POST["Lname"] is empty or if it is empty? There is no error handling and the code will go continue on and possibly fail when it tries to reference $LnameUc 6. Error handling should not require an exit() function and it should not stop checking for errors on the first error encountered. You should try to provide the user with as many of the error conditions as possible. For example, if they are inserting a new record, check for the possible date entry errors on each piece of data (required fields, format, data type (e.g. numbers), etc.) and store the errors into an array. Then after all the validations are complete, if there are any errors, provide them to the user and complete the page. If there are no errors then attempt the next stage in the process, such as inserting the record. If there are errors at that point then show that error and complete the page. There's nothing more frustrating then submitting a form to be told a piece of data needs to be corrected, resubmitting and then being told a different piece of data was also invalid - but not mentioned previously.
  14. You should not have to do nested loops to get the data you need. This is very inefficient. As I said, if you are generating this data, then you can put them in a more logical format for your needs. Yes, the data is straight forward - for a human to read. But, the fact that a nested loop is needed should be a red flag that it is not the right format. For example, since the time value is so important, the time value could be the index for the array values and the values could be sub-arrays of the records with their title & desc. Then you don't even need a loop, let alone nested loops. E.g. $items=array ( '201601221400' => array ( array ( 'title' => 'FABLife' 'desc' => 'Mark Cuban (``Shark Tank\'\'); top five must-haves; collectors try to guess the prices of celebrity memorabilia; creating a high-end playroom and eliminating toy clutter without breaking the bank.' ), array ( 'title' => 'The First 48', 'desc' => 'A young man is robbed and killed while meeting up with a girl he met earlier; a man is gunned down outside an annual football game.', ), array ( 'title' => 'Teen Titans Go!', 'desc' => 'Robin makes the other Titans sell their treasured mementos from past adventures.', ) ) ), '201601221600' => array ( array ( 'title' => 'Some other Title' 'desc' => 'Some other description' ) ) ); But, as I also said, if this is coming from a database, then you shouldn't need arrays at all. But, you have not provided enough data to know what the best method really is since all we have to go on is the arrays you've provided. Not really. Your code had several flaws. Here is your code with comments. There are two major flaws indicated with '***'. I suggest you write comments with your code as it can help you identify logic errors. //Create a loop for the number of elements in $control for($i=0; $i< count($control); $i++) { //Loop through each instance of $items foreach ($items as $item) { //Run a while() loop as long as the current $item['time'] matches the $control[$i] value at $i //And $r is less than 5 //**** Assuming this condition is ever true, the while() loop will be repeated // for THE EXACT SAME $item['time'] and $control[$i] values until $r = 5 // because there is no logic in the loop to iterate to the next $time record // or increment $i While (($items['time']==$control[$i]) || ($r < 5 )) { //Check if $r > 5 //**** THIS IS IMPOSSIBLE DUE TO THE WHILE CONDITION ABOVE if ($r > 5) { echo "OUT. . ."."<BR>"; $r=0; } else { $r=$r+1; echo $control[$i]." ". $r."<br>"; } } } }
  15. I don't know what the source of your data is, but based on what you need to do they could be structured in a much better format. If you are the one building these arrays you should fix that. If this data is coming from a database, you shouldn't need to build these arrays at all. As to your code, it is simply illogical. It's not worth my time to try and break it down and explain why it doesn't work. //Create array to hold all the found matches $foundValues = array(); //Iterate over each control value foreach($control as $controlVal) { //Iterate over each item record foreach($items as $itemVal) { //Check for a match and append to found array if($controlVal == $itemVal['time']) { $foundValues[] = $controlVal; } //Break the current loop if found count is 5 if(count($foundValues)==5) { break; } } //Break the current loop if found count is 5 if(count($foundValues)==5) { break; } } //Output the results foreach($foundValues as $index => $value) { $number = $index + 1; echo "{$value} {$number}<br>\n"; } if(count($foundValues)==5) { echo "OUT. . . <br>\n"; }
  16. As I stated before, you have no data in the POST data to associate each of the records with existing records in the database. For example, with this $playerDeckCardIds=$_GET['deckcardids']; //This contains a string that looks something like this. "67 98 3 66 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31 32 33 34 35 36 37 38 39 40 41 42 78 -1 -1 -1 -1 -1 -1 -1 -1 -1 -1 -1 -1 -1 -1 -1 -1 -1 " Which row_id should be updated with the card_id of 67? Are you going to *assume* that the first record in the post string should be associated with the lowest number card_id for the user? (FYI: You should never assume when it comes with interpreting data). I'm really not interested in reviewing your project and the business rules to determine all the changes needed suffice to say it appears to be seriously flawed. But, I will give you this: 1) If the card_id is not used as a reference in other tables you could just delete all the existing records for the user then create new ones based on the POSTed data. 2. Run a query to get a list of the card_id's for all the users records, then run a loop over each one and update each with the 'next' record in the post data. This assumes there is no importance in which record is updated with what value. This appears to be what you were trying to achieve - but you were trying to reference an array value as an object property
  17. To add to mac_gyver's responsse: The above "works" because PHP is a loosely typed language. In other words, it tries to make judgement calls on values that are not strictly the same. E.g. if(1 == '1') { echo "TRUE"; } will output TRUE even though the number '1' and the string '1' are not the same type of value. In your example above you have this if($undeclaredVariable == FALSE) The$undeclaredVariable is actually not defined, i.e. NULL. PHP will loosely compare that to FALSE. Depending on your error reporting level you might actually get a warning trying to compare that undefined variable. So, you should either define a default for the variable first or if there is a chance a variable may not be set when you need to test it you could use the isset() function along with whatever trivial check you are doing if(!isset($undeclaredVariable) || $undeclaredVariable == FALSE) The isset() should come first. If that is true, then the code does not proceed to the OR condition and prevents the possible warning. EDIT: Some additional points: This page shows the difference between loose comparisons and strict comparisons in PHP: http://php.net/manual/en/language.operators.comparison.php Here is some example code to illustrate if(!isset($undeclaredVariable)) { echo 'the undeclared variable is not set'; } if($undeclaredVariable == TRUE) { echo 'the undeclared variable is INTERPRETED as a "TRUE" value: it is set and not null, not 0, not an empty string, not an empty array, etc.'; } if($undeclaredVariable === TRUE) //Note tripple equal signs === { echo 'the undeclared variable IS exactly the BOOLEAN "TRUE"'; } if($undeclaredVariable == FALSE) { echo 'the undeclared variable is INTERPRETED as a "FALSE" value: not set, is null, is 0, is an empty string or array, etc.'; } if($undeclaredVariable == FALSE) //Note the !== { echo 'the undeclared variable IS exactly the BOOLEAN "FALSE"'; }
  18. OK, I'm trying to give you some code, but I see something else that doesn't make sense. Your code appears to be trying to change the card_id of the records. I would think you would be changing the values. You example of the passed data is $playerDeckCardIds=$_GET['deckcardids']; //This contains a string that looks something like this. "1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31 32 33 34 35 36 37 38 39 40 -1 -1 -1 -1 -1 -1 -1 -1 -1 -1 -1 -1 -1 -1 -1 -1 -1 -1 -1 -1 " Based on the image of your table those are values not IDs (yet the code is trying to update the IDs) - very confusing. I'm not even sure why this is getting passed as a string and not an array. But, it is impossible to associate that list of values (?) with the existing records - except that you know they are the records for the current user. I don't know what the form looks like or what you are doing. It would be advantageous to have a unique input field for each card so you can associate the correct value with each card id. E.g. <input type='text' name='card[4]' value='-1' > <input type='text' name='card[5]' value='2' > <input type='text' name='card[6]' value='3' > That will be passed as a multi-dimensional array with the ids 4, 5, & 6 with those associated values.
  19. The reason why the values are getting set to 0 is simple - looking at your previous code (and comments) and the current code should tell you why As your comment in the prior post shows you are retrieving the results as an array. But, you are trying to bind the value as if it is an object. But, you are doing this the wrong way. There is no need for a select query. Just the update query. Also, you are using prepared queried all wrong. You prepare the query one time, then execute it multiple times while changing the bound data.
  20. Not, to be rude but, yes, I think you are missing something. I am not talking about treating the "super admin" as an anonymous user by any means. Based on one of the OP's previous responses he intends to have "roles" that can be customized to have different permissions granted or not. The Roles table would still have a record for the Super Admin role (id = 1) and the same tables the OP described above would still exists to describe the permissions available for all the other role types (but not the super admin). Upon login, the system would store the role id in a session variable and upon attempting to access any functionality that requires a specific permission a service would be called to check if the user is assigned to a role with rights to that functionality. That service could look like this: //Very generalized code function checkPermision($roleID, $permissionID) { //Always return true for super-admin if($roleID==1) { return true; } //Else, query the DB to determine if the user's role has rights to the permission //Will return 1 if role has the permission, else 0 $query = "SELECT COUNT(*) FROM role_permission WHERE role_id = $roleID AND permission_id = '$permissionID'"; $result = mysqli_query($link, $query); $mysqli->query($query) $row = $result->fetch_row()) //Retuyrn 1/0 (i.e. true/false) return $row[0]; } The reason to do this is it ensure that the super admin does not lose permission because the role based permissions are accidentally deleted, corrupted, whatever. Imagine the scenario where only the super admin has permissions to edit roles and somehow that permission got deleted for the super admin. There would be no way to resolve the issue through the application. It would require someone with knowledge of the application and access to the DB to create a query to manually add/edit the necessary records. This could be catastrophic if it were to occur when the application is mission critical. Sure, you can build the application to prevent such things, but those pesky business rules to mandate very narrow logic such as that are the ones that get tested the least. And - bugs happen. By always returning true for the Super Admin when checking permissions you do not need to worry about any permission being removed either intentionally or by accident.
  21. One suggestion: I assume the super_admin role would have full permissions and should never have limited permissions. So, you don't need to have any records in the role_permissions table for the super_admin. Instead, if the role is super_admin just assume all permissions are available. The reason for this is if any records from the role_permissions table were inadvertently deleted the super_admin could lose the ability to do things that no one has the rights to do. This basically ensure the system doesn't come to a halt. You would likely have a service to check a permission for a user based on their role. That service would simply return true is the user is the super_admin instead of checking the role assignments.
  22. If you get the list via JSON (or some other method) then you could update the select list with the values and labels from that JSON list.
  23. There are literally hundreds, if not thousands, of way to "optimize" code. It all depends on what your code is specifically doing. If you are running queries in loops - don't. If you are using RegEx try and find string function alternatives, etc. etc. etc. It is a waste of time for us to try and suggest optimizations that may be appropriate for your need without seeing any code.
  24. Also, you would not put quotes around the values within the query string and the values should be URL encoded (e.g. a space would be %20 www.mypage.com?name=John%20Smith
  25. I've not tested it, but I would think having different form names would work too.
×
×
  • 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.