I'm having a bit of trouble with this global. This is listed in a while statement. while (condition){ $test="warthog"; global $test; do stuff here } Outside the statement echoing $test yields nothing. echo "It's not a $test"; What am I doing wrong?
First thing, is error reporting turned on? If not, turn error reporting on and set it for ALL, so you can see any issues. Next, try print_r( $test ); and see if anything is printed. Now, why do you need to global $test in the first place? There is probably a better way of doing things other than globaling the variable. Second, When assigning values to a variable, try using ' ' singles instead of " ". In larger scripts, the script will run faster. You can try turning error reportin on using ... error_reporting( E_ALL ); Let me know what is printed or what you find.
Ok, global is working after all on the same page. The problem is the variable is not being passed to the next page. Here's what I'm working on: I have a form and the fields are being prepopulated with variables pulled from a DB. I modify the fields (if needed) in the form, hit submit (method=POST) and then run: if($_POST['check']=="modify"){ echo "ID: $id <br>"; //global does not echo here echo "test: $test <br>";//global does echo here $sql = "UPDATE search_tbl SET test='$test' WHERE id='$id'"; if (mysql_query($sql, $conn) or die(mysql_error())){ echo "record modified!"; } else { echo "something went wrong"; } } I get record modified message when the page is submitted. However, the vars are blank (even when using print_r). Undefined variable is the only error in log.
Ok, I stripped down the code and this is a working version up to the point of submitting the form: <?php $id=$_GET['id']; //get id from url $conn = mysql_connect("localhost", "user_name", "super_secret_password"); mysql_select_db("DB_name", $conn); $TotalResult=mysql_query("SELECT * FROM table_name WHERE id = $id"); while($row = mysql_fetch_object($TotalResult)) { $row_color = 'eeeeee'; //table background color = row_color variable $column1 = $row->column1; if (isset($column1)) { echo "<left><table bgcolor=".$row_color.">"; echo "<tr>"; echo "<td>$column1 </td>"; echo "</tr>"; echo "</table></left><br>"; } //end if echo "<FORM ACTION=\"for_forum.php\" METHOD=POST>"; echo "<p><b>Search Term 1:</b> <input type=text name=\"column1\" size=45 value=\"".$column1."\"></p>"; echo "<p> <input type=\"submit\" name=\"submit\" value=\"Modify Record\"></p><br>"; echo "<input type=\"hidden\" name=\"check\" value=\"modify\">"; } echo "Here is column1: $column1 <br>"; // variable value appears first time but not second echo "Now id: $id <br>"; // variable value appears first time but not second if($_POST['check']=="modify"){ // adding an echo id or column1 at this point shows nothing $sql = "UPDATE table_name SET column1='$column1' WHERE id='$id'"; if (mysql_query($sql, $conn) or die(mysql_error())){ echo "record modified!"; // I get this message, but nothing has been modified because id and column1 are empty } else { echo "something went wrong"; } } ?> Any ideas?
Just adding this after if($_POST['check']=="modify") ... $id=$_POST['id']; $column1=$_POST['column1']; Pretty much made it work (just a little neatening up to do). I guess if you don't post it, it won't be there. =) Thanks.
It works just fine, but when I look at the error log (to make sure everything is good) I see one problem. It says that "check" is an undefined index. Any help? Thanks. <?php $conn = mysql_connect("localhost", "user", "pass"); mysql_select_db("db_name", $conn); $check=''; $id=''; $column1=''; if($_POST['check']!="modify"){ // ****ERROR refers to this line **** $id=$_GET['id']; //get id from url $TotalResult=mysql_query("SELECT * FROM db_name WHERE id = $id"); while($row = mysql_fetch_object($TotalResult)) { $row_color = 'eeeeee'; //table background color = row_color variable $column1 = $row->column1; if (isset($column1)) { echo "<left><table bgcolor=".$row_color.">"; echo "<tr>"; echo "<td>$column1 </td>"; echo "</tr>"; echo "</table></left><br>"; } //end if echo "<FORM ACTION=\"for_forum.php\" METHOD=POST>"; echo "<p><input type=hidden name=\"id\" value=\"".$id."\"></p>"; echo "<p><b>Search Term 1:</b> <input type=text name=\"column1\" size=45 value=\"".$column1."\"></p>"; echo "<p> <input type=\"submit\" name=\"submit\" value=\"Modify Record\"></p><br>"; echo "<input type=\"hidden\" name=\"check\" value=\"modify\">"; } } elseif ($_POST['check']=="modify"){ $id=$_POST['id']; $column1=$_POST['column1']; $sql = "UPDATE search_tbl SET column1='$column1' WHERE id='$id'"; if (mysql_query($sql, $conn) or die(mysql_error())){ echo "record modified!"; } else { echo "something went wrong"; } } ?>
I think you misunderstand the concept of scope. Global variables are not automatically declared in every script just because you declare them in one. Properly POSTing them was the proper thing to do. I'm going to go ahead and point this out because I am seeing this a lot and it is a very serious security hole. You need to sanitize (and preferably validate) all user input. That means anything you recieve via POST, GET, COOKIE, even SERVER and sometimes SESSION. In this case you have a potential SQL injection just waiting to happen. $id=$_GET['id']; //get id from url $TotalResult=mysql_query("SELECT * FROM db_name WHERE id = $id"); ... $id=$_POST['id']; $column1=$_POST['column1']; $sql = "UPDATE search_tbl SET column1='$column1' WHERE id='$id'"; PHP: These variables need to be properly escaped before putting them into your query, otherwise you run the risk of compromising the confidentiality of your data or losing your entire database. Do this instead: $id=mysql_real_escape_string($_GET['id']); //get id from url $TotalResult=mysql_query("SELECT * FROM db_name WHERE id = $id"); ... $id=mysql_real_escape_string($_POST['id']); $column1=mysql_real_escape_string($_POST['column1']); $sql = "UPDATE search_tbl SET column1='$column1' WHERE id='$id'"; PHP: As for the error you are recieving, it's because $_POST['check'] doesn't exist until you post the form. Since the value is unimportant, just use isset($_POST['check']) instead. However, you are still using $_GET['id'] instead of $_POST['id'], so that needs to be changed in order for it to work properly.
Thanks for the security tip. Always can use these. I'm not worried too much about them in this situation, because the folder (containing the page) is password protected, but still, always good to err on the side of caution. Also, the id is originally passed to this page from another page (and it's passed in the url), so I think I have to use get, right? Thanks for all your suggestions. I'm learning more each project. =)
I was assuming that the form posted to itself, which is certainly how it appears. If it is, then you should use $_REQUEST['id'] instead of $_GET or $_POST since it covers both of your bases. Just using $_GET won't pass the id parameter after you post the page because you don't tell it to in the form action. I took the liberty of rewriting your code. There are just a few minor changes in addition to the ones I suggested before, since I didn't know how much of your other code was dependant on your existing code. I also used my own scripting conventions in a few places out of habit, but it doesn't affect anything other than readability and it would be trivial to change it back. <?php $conn = mysql_connect("localhost", "user", "pass"); mysql_select_db("db_name", $conn); $id=mysql_real_escape_string($_REQUEST['id']); if(!isset($_POST['check'])) { $TotalResult=mysql_query("SELECT column1 FROM db_name WHERE id = {$id}"); while($row = @mysql_fetch_assoc($TotalResult)) { $row_color = 'eeeeee'; //table background color = row_color variable $column1 = $row['column1']; if (isset($row['column1'])) { echo "<left><table style=\"background:#{$row_color}\"> <tr><td>{$column1} </td></tr> </table></left><br />"; } echo <<<FRM <FORM ACTION="for_forum.php" METHOD="POST"> <p><input type="hidden" name="id" value="{$id}"></p> <p><b>Search Term 1:</b> <input type=text name="column1" size="45" value="{$column1}"></p> <p> <input type="submit" name="submit" value="Modify Record"></p><br /> <input type="hidden" name="check" value="modify"> FRM; } } else { $column1=mysql_real_escape_string($_POST['column1']); $sql = "UPDATE search_tbl SET column1='{$column1}' WHERE id='{$id}'"; if (mysql_query($sql, $conn)){echo "record modified!";} else {die(mysql_error());} } ?> PHP: Change log: Removed predeclared variables. It isn't necessary and some aren't even used. Moved $id outside of your conditional statements since it is defined either way. Changed elseif to else, since it is the same thing in this case. Changed form to use heredoc syntax Changed while loop to use mysql_fetch_assoc since I think it's faster. Fixed the if in the while loop ($column1 is always set since you define it just before you check). Fixed table to use style instead of background (you should still get rid of <left>). CSS is your friend. Enclosed variables within double quotes in curly braces. Optimized your first SQL query. Fixed error reporting in second query. It's not perfect, but it should work.
The Critic, Thanks. You're awesome. I didn't expect a rewrite, but I appreciate it. Gives me a look at how to make my coding better. New to PHP (last time I did real coding was in the 80's - Pascal) I'll check out the code this weekend. Id is passed by url from another page initially, and then posted from then on. I had the predeclared vars because I have dabbled with C, too, and everything should be declared and initialized. Guess good habits die hard. I hadn't really looked at the look/feel of it yet, as parts of it were just copied and pasted from examples (although, the overall concept and function were mine - my first !!!). Again, I appreciate all your help and feedback. =)
Happy to help. Technically, you should declare your variables even though PHP doesn't require it simply because it's good coding practice. However, I removed these in this case because 1) $check wasn't used anywhere in the script outside of the declaration, 2) $id was moved and declaring it just before defining it didn't serve much purpose, and 3) $column1 got thrown out with the others (plus declaring it took it out of scope). I don't want to give the impression that it's better to not declare your variables because it isn't, but it's a very uncommon practice in PHP and certainly won't hurt anything if you don't. In all, a good job for your first effort (you should've seen mine ) and I'm sure you'll be churning out works of art in no time.