Is this the best way to do it?

[Jick]

I have this php script that someone made for me and it works fine and does exactally what I want but I wanted to know if the code I'm posting is the best way to accomplish what I want? What the script does is when I go to my (login.html) page and enter my username, password, and status if the username and password are right then it will change my status to what I want which is kept in a text file and then displayed on my main page.

Here is (process.php) which is the code that does all the work:<?
IF ($user == "")
{
echo "The (<b>Username</b>) feild needs to be filled in.<br><a title='Go Back' href='login.html'>Go Back</a>";
exit;
}
IF ($pass == "")
{
echo "The (<b>Password</b>) feild needs to be filled in.<br><a title='Go Back' href='login.html'>Go Back</a>";
exit;
}
IF ($stat == "Custom" and $customtext == "")
{
echo "The (<b>Custom</b>) feild needs to be filled in.<br><a title='Go Back' href='login.html'>Go Back</a>";
exit;
}
?>
<?
IF ($stat == "Custom")
{
$stat = "<font color=blue>$customtext</font>";
}
ELSEIF ($stat == "Online")
{
$stat = "<font color=green>$stat</font>";
}
ELSEIF ($stat == "Offline")
{
$stat = "<font color=red>$stat</font>";
}
?>
<?
IF ($stat == "Custom")
{
$stat = $customtext;
}
IF ($user == "user" and $pass == "pass")
{
echo "Your status has been changed to: (<b>$stat</b>).<br><a title='Go Back' href='login.html'>Go Back</a>";
$openit = fopen("status.txt", "w");
fwrite($openit, "$stat");
}
else
{
echo "Sorry, you have entered a wrong username and/or password.<br><a title='Go Back' href='login.html'>Go Back</a>";
}
?>
Here is (login.html) which is where I login from:<form method="POST" action="process.php">
Username:<br><input type="text" name="user"><br>
Password:<br><input type="password" name="pass"><br>
Online:<br><input type="radio" value="Online" checked name="stat"><br>
Offline:<br><input type="radio" value="Offline" name="stat"><br>
Custom:<br><input type="radio" value="Custom" name="stat"><input name="customtext" maxlength="15" size="20"><br>
<input type="submit" value="Login" name="submit">
</form>I would greatly appreciate any insite on this matter. :D

[pyro]

It's not how I would have done it, but hey, who cares, right?

Anyway, a few things I noticed...

The script uses global variables (http://www.webdevfaqs.com/php.php#globalvariables), which is a bad thing...

Also, these lines are interesting...
<?
IF ($stat == "Custom")
{
$stat = "<font color=blue>$customtext</font>";
}
//your other code...
IF ($stat == "Custom")
{
$stat = $customtext;
}Looks a bit redundant to me...

Other than that, the style is a bit screwed. Why does the script get in and out of PHP 3 times? Once should suffice. It's also my own personal preference not to use short tags <? ?>, as that is an option that can be enabled/disabled in the php.ini file, so if you try using the script on a server with them disabled, you'd have to change it anyway...

Another thing, is if this is running on a *nix machine, you should be locking the file before writing to it, using flock (http://www.php.net/flock).

And last but certainly not least, the code outputs the horrid <font> tag. Try this out:

<span style="color: blue;">text</span>

[Jick]

<?PHP
IF ($user == "")
{
echo "The (<b>Username</b>) feild needs to be filled in.<br><a title='Go Back' href='login.html'>Go Back</a>";
exit;
}
IF ($pass == "")
{
echo "The (<b>Password</b>) feild needs to be filled in.<br><a title='Go Back' href='login.html'>Go Back</a>";
exit;
}
IF ($stat == "Custom" and $customtext == "")
{
echo "The (<b>Custom</b>) feild needs to be filled in.<br><a title='Go Back' href='login.html'>Go Back</a>";
exit;
}
IF ($stat == "Custom")
{
$stat = "<span style="color: blue;">$customtext</span>";
}
ELSEIF ($stat == "Online")
{
$stat = "<span style="color: blue;">$stat</span>";
}
ELSEIF ($stat == "Offline")
{
$stat = "<span style="color: blue;">$stat</span>";
}
IF ($stat == "Custom")
{
$stat = $customtext;
}
IF ($user == "user" and $pass == "pass")
{
echo "Your status has been changed to: (<b>$stat</b> ).<br><a title='Go Back' href='login.html'>Go Back</a>";
$openit = fopen("status.txt", "w");
fwrite($openit, "$stat");
}
else
{
echo "Sorry, you have entered a wrong username and/or password.<br><a title='Go Back' href='login.html'>Go Back</a>";
}
?>Thank you very much Pyro. I fixed some of the things that I understood but there are one or two things you were talking about that I did not get. I did not get the "flock" thing but I did look at the link. I was wundering if you could explain that a little more and maybe post some code if you feel up to it?

[pyro]

You also didn't change the script to not use global variables. Add this to the top of the script to take care of that problem:

$user = $_POST['user'];
$pass = $_POST['pass'];
$stat = $_POST['stat'];
$customtext = $_POST['customtext'];

And, to lock the file, you do this:

$openit = fopen("status.txt", "w");
flock ($openit, LOCK_EX); #lock the file
fwrite($openit, "$stat");
flock($openit, LOCK_UN); #release the file

You also introduced a whole set of new errors, in that where you put the <span> tags, you did not escape the slashes. Take this line for instance:

$stat = "<span style="color: blue;">$customtext</span>";That should be:

$stat = "<span style=\"color: blue;\">$customtext</span>";

[Jick]

Ok, thank you for that. I added the stuff you said. How does it look now? Do I need to fix any thing else? Maybe you could give me a few hints on my script syntax because you mentioned somthing about it not looking right.<?PHP
$user = $_POST['user'];
$pass = $_POST['pass'];
$stat = $_POST['stat'];
$customtext = $_POST['customtext'];

IF ($user == "")
{
echo "The (<b>Username</b>) feild needs to be filled in.<br><a title='Go Back' href='login.html'>Go Back</a>";
exit;
}
IF ($pass == "")
{
echo "The (<b>Password</b>) feild needs to be filled in.<br><a title='Go Back' href='login.html'>Go Back</a>";
exit;
}
IF ($stat == "Custom" and $customtext == "")
{
echo "The (<b>Custom</b>) feild needs to be filled in.<br><a title='Go Back' href='login.html'>Go Back</a>";
exit;
}
IF ($stat == "Custom")
{
$stat = "<span style=\"color: blue;\">$customtext</span>";
}
ELSEIF ($stat == "Online")
{
$stat = "<span style=\"color: blue;\">$stat</span>";
}
ELSEIF ($stat == "Offline")
{
$stat = "<span style=\"color: blue;\">$stat</span>";
}
IF ($stat == "Custom")
{
$stat = $customtext;
}
IF ($user == "user" and $pass == "pass")
{
echo "Your status has been changed to: (<b>$stat</b> ).<br><a title='Go Back' href='login.html'>Go Back</a>";
$openit = fopen("status.txt", "w");
flock ($openit, LOCK_EX); #lock the file
fwrite($openit, "$stat");
flock($fp, LOCK_UN); #release the file
}
else
{
echo "Sorry, you have entered a wrong username and/or password.<br><a title='Go Back' href='login.html'>Go Back</a>";
}
?>Thank you so much for all your help sofar. You are the kindest man I know. Seriously. :)

[pyro]

Something along these lines would probably be how I'd do it (this assumes that all fields need to be filled out):

Note that it is untested code:

<?PHP

$user = $_POST['user'];
$pass = $_POST['pass'];
$stat = $_POST['stat'];
$customtext = $_POST['customtext'];

if ((empty($user)) || (empty($pass)) || (empty($stat)) || (empty($customtext))) {
echo "All fields must be filled in.<br><a title='Go Back' href='login.html'>Go Back</a>";
exit;
}
if ($stat == "Custom") {
$stat = "<span style=\"color: blue;\">$customtext</span>";
}
else {
$stat = "<span style=\"color: blue;\">$stat</span>";
}
if (($user == "user") and ($pass == "pass")) {
echo "Your status has been changed to: (<b>$stat</b> ).<br><a title='Go Back' href='login.html'>Go Back</a>";
$openit = fopen("status.txt", "w");
flock ($openit, LOCK_EX); #lock the file
fwrite($openit, "$stat");
flock($openit, LOCK_UN); #release the file
}
else {
echo "Sorry, you have entered a wrong username and/or password.<br><a title='Go Back' href='login.html'>Go Back</a>";
}
?>

[Jick]

Thank you so much. But I'm now getting this when I enter my username and password and select online for status:

Your status has been changed to: (Online).
Go Back
Warning: flock(): supplied argument is not a valid stream resource in /home/infini15/public_html/kd7pyo/status/process.php on line 23

I also get this for when I do offline too. Do you know whats wrong? I also have one last question. What can I add to process.php to make it display the current status if someone trys to go to process.php without logng in. Right now if I go to that file direct I get this:

All fields must be filled in.
Go Back

Thanks for all your help. :D

[pyro]

My bad... I edited my above post to fix that error.

[Jick]

I also have one last question. What can I add to process.php to make it display the current status if someone trys to go to process.php direct without logging in. Right now if I go to that file direct I get this:

All fields must be filled in.
Go Back

Thanks for all your help. :D

[PunkSktBrdr01]

Add this to the login form:


<input type="hidden" name="fromlogin" value="yes">


Add this to process.php:


$fromlogin = $_POST['fromlogin'];

if($fromlogin != "yes") {
echo "error message";
}


Hope that helps!

[pyro]

Or, you could just wrap this around the whole script:

if (isset($_POST['submit'])) {
#code here..
}
else {
#they didn't come in through the login page...
}

[Jick]

<?PHP
if (isset($_POST['submit'])) {

$user = $_POST['user'];
$pass = $_POST['pass'];
$stat = $_POST['stat'];
$customtext = $_POST['customtext'];

if ((empty($user)) || (empty($pass)) || (empty($stat))) {
echo "All fields must be filled in.<br><a title='Go Back' href='login.php'>Go Back</a>";
exit;
}
if ($stat == "Custom") {
$stat = "$customtext";
}
else {
$stat = "$stat";
}
if (($user == "user") and ($pass == "pass")) {
echo "Your status has been changed to: (<b>$stat</b>).<br><a title='Go Back' href='login.php'>Go Back</a>";
$openit = fopen("status.txt", "w");
flock ($openit, LOCK_EX); #lock the file
fwrite($openit, "$stat");
flock($openit, LOCK_UN); #release the file
}
else {
echo "Sorry, you have entered a wrong username and/or password.<br><a title='Go Back' href='login.php'>Go Back</a>";
}

}
else {
#they didn't come in through the login page...
}
?>This is what I have now. Now when ever I try to login I just get forwarded to a blank page. Any suggestions? Maybe I just inserted it wrong.

[pyro]

That means that the post variable "submit" is not set. Check to make sure that your submit button is named submit...

[Jick]

I checked and it ends up it wasn't but then I changed it to submit but it still did not work. I still got the blank page. Any ideas? Thank you for your help! :)

[pyro]

Post your code for both the login page, and the PHP script...

[Jick]

<?PHP
if (isset($_POST['submit'])) {

$user = $_POST['user'];
$pass = $_POST['pass'];
$stat = $_POST['stat'];
$customtext = $_POST['customtext'];

if ((empty($user)) || (empty($pass)) || (empty($stat))) {
echo "All fields must be filled in.<br><a title='Go Back' href='login.php'>Go Back</a>";
exit;
}
if ($stat == "Custom") {
$stat = "$customtext";
}
else {
$stat = "$stat";
}
if (($user == "user") and ($pass == "pass")) {
echo "Your status has been changed to: (<b>$stat</b>).<br><a title='Go Back' href='login.php'>Go Back</a>";
$openit = fopen("status.txt", "w");
flock ($openit, LOCK_EX); #lock the file
fwrite($openit, "$stat");
flock($openit, LOCK_UN); #release the file
}
else {
echo "Sorry, you have entered a wrong username and/or password.<br><a title='Go Back' href='login.php'>Go Back</a>";
}
}
else {
#they didn't come in through the login page...
}
?><form method="POST" action="process.php">
Username:<br><input type="text" name="user"><br>
Password:<br><input type="password" name="pass"><br>
Online:<br><input type="radio" value="Online" checked name="stat"><br>
Offline:<br><input type="radio" value="Offline" name="stat"><br>
Custom:<br><input type="radio" value="Custom" name="stat"><input name="customtext" maxlength="15" size="20"><br>
<input type="submit" value="Login" name="submit">
</form>

[pyro]

It is working fine for me...

[Jick]

I wunder why it's not working for me? :(