How to do if query is false?

Anon

Here's the problem.

I have an account creation page, which will store name, plain-text password, crypt-text password, and access number (it's just defaulted at "10" for now).

In the creation, I want to only create an entry if that name doesn't already exist.

This is what I have so far:

$results = !mysql_query("SELECT * FROM logins WHERE name=$user_name")
or die ("That username is not available.");

if (isset($results)) {
mysql_query("INSERT INTO logins (name, clearpass, cryptpass, access) VALUES ('$user_name', '$user_password', '$cryptpass', '$access')")
or die ("Failed to create login.");
}

That works to create, but of course it fails to correctly see if the name exists.

I think I'm just going about it the wrong way.

I want to see if $user_name is already listed in the "name" field inside the "logins" table.

If it does exist, let the user know that name is already taken.

If it does not exist, go ahead and create it.

Any ideas?

Anon

if (mysql_query("SELECT * FROM logins WHERE name=$user_name")) {
echo "Account was not created.";
} else {
mysql_query("INSERT INTO logins (name, clearpass, cryptpass, access) VALUES ('$user_name', '$user_password', '$cryptpass', '$access')")
or die ("Failed to create login.");
echo "Account created.";
}
}

Now, it checks to see if the name exists. If it finds the name to be in existance, it should return as "TRUE (non-zero)" (according to the manual), then it's statement then prints out that the account was not created.

If the name was not found it should return as "FALSE", and thus the else statement should be executed.

Of course, it seems to never equal true, and thus always create an account grrrr.

Help?

Anon

I think I'm just going about it the wrong way.

On several levels.
<ul><li>A variable with the value 'false' is nevertheless SET. Therefore, saying if (isset($results)) is meaningless because $results will always be set.<li>Testing the return value of the mysql_query() won't tell you what you want to know. Quoting from the FM: A return value of TRUE
means that the query was legal and could be executed by the server. It does not indicate anything about the number of rows
affected or returned. It is perfectly possible for a query to succeed but affect no rows or return no rows. Instead, you need to call mysql_num_rows() and look for a return value of 0, but...<li>This is the wrong way to go about testing for uniqueness, anyway. Unless you lock the entire database before your query, it's possible for two different clients to be making the same query for the existence of the same name at the same time, and both find it not taken and both try to do an insert. </ul>
Instead, you should first of all make sure that there is a unique or primary key defined for name, so that the database won't let you insert duplicate records. Then, you simply try to insert the row, and catch the error if it doesn't work:

$result = @ (
  "insert into logins (name, ...) values ('$user_name', ...)"

if (! $result)
  {
  if (mysql_errno() == 1062)
    echo "Sorry, $user_name is already taken...";
  else
    echo "Something very bad happened: " . mysqy_error();
  }
else
  {
  // $user_name was added
  }

Anon

Oops, sorry I missed an end tag <b&ht; there; not really meaning to shout at you all.

Also I meant to point out that the '@' in the call to mysql_query() is very important, as otherwise the raw error message (including the source file name and line) will be dumped straight to the html output.

Anon

Ah, excellant. It "almost" works.

However now I get a parse error.

Oh yes, joyous me. But it is functionally working!

So, the problem is with that second "else".

The brackets are done like:

if {A
if {B
if {C
}C else {D
}D else {E
}E
}B
}A

(the letters are added to "pair" each opening bracket with a closing one)

Now, the parsing error is on the second else. Using echo "something";, gives an error as much as an empty else does, so I believe I can assume the statement isn't the problem.

Are these "if" statements nested incorrectly?

By removing else, and both "E" brackets the whole thing will work without a parse error. So at this point, that's what I'll do.

Any ideas as to why that extra "else { }" is causing problems? Do I need to place the else directly after the if statement it's part of?

But if that's true, doesn't that negate the whole point of having 2 nested if statements?

wobbles around a bit

Insights welcome...again :\

Anon

Why don't you toss up the revised code? Being as how
I'm not a compiler, the lack of indentation is pretty
debilitating. Let's try it with some indentation:

if (A)
  {
  if (😎
    {
    if (C)
      {
      }
    else // !C
      {
      }
    else // extraneous else! (your E)
      {
      }
    }
  }

But let's see what you've written.

Anon

Now that I look at it, the lack of indention is horrid, lol.

Well, here is the code from the first if to the last bracket:

if ($submit && $user_name && $user_password) {
include("mysql.php");
db_link();
$cryptpass = md5($user_password);
$access = "10";

$result = @ ("INSERT INTO logins (name, clearpass, cryptpass, access) VALUES ('$user_name', '$user_password', '$cryptpass', '$access')");

if (!$result) {
if (mysql_errno() == "1062") {
echo "Sorry, $user_name is already taken. Please choose another name. ";
} else {
echo "Something very bad happened: " . mysql_error();
} else
// Our nasty little else.
}
}
}
// end of code

$user_name and $user_password are provided via form. "db_link()" is to connect to database and select the database to use.

In the above form, the parse error is at that 2nd else.

I can substitute the need for that last "else" with an:
if ($results) {
// print confirmation
}

Thus it works, but now I just have to figure out why the original 2 elses won't work...hehe.

Anon

Ok I copied it wrong and it stripped my indentions. Trying it the correct way:

if ($submit && $user_name && $user_password) {

include("mysql.php");
db_link();
$cryptpass = md5($user_password);
$access = "10";

$result = @ ("INSERT INTO logins (name, clearpass, cryptpass, access) VALUES ('$user_name', '$user_password', '$cryptpass', '$access')");

Hope that worked. The change is that extra bracket behind the else...it was there in the original, but I deleted it during the copying.

Anon

Well, before I have to type out all the whitespace html code, and make more and more posts, I'm just going to handle this the good old fashion way...

With a text file. sigh

Sorry for the extra posts, lol. If only for a post edit 🙂

http://crazyoddball.datablocks.net/codetext.txt

That's the code snippit, in properly indented format.

Anon

Ok, here's the code you posted, but reformatted in my
preferred style. (When I teach C, I politely but firmly
try to steer people away from the old K&R style. I don't
care how venerable it is, it fails the most basic
test, which is that the blocks you write aren't visually
bracketed! What's the use of that???) Also, I broke
some of the longer lines so they wouldn't wrap. With
any luck...

if ($submit && $user_name && $user_password)
  {
  include("mysql.php");

  db_link();

  $cryptpass = md5($user_password);
  $access = "10";

  $result = @(
    "INSERT INTO logins"
    . " (name, clearpass, cryptpass, access)"
    . " VALUES ('$user_name', '$user_password',"
    . " '$cryptpass', '$access')" );

if ($result)
    {
    echo "$user_name was added. ";
    }

if (!$result)
    {
    if (mysql_errno() == "1062")
      {
      echo "Sorry, $user_name is already taken."
        . "Please choose another name. ";

      }
    else
      {
      echo "Something bad happened: " . mysql_error();

      }
    else
      {
      // our nasty little else
      }
    }
  }

Anon

  $result = @(
    "INSERT INTO logins"
    . " (name, clearpass, cryptpass, access)"
...

Why are you saving the plaintext password? There's no point
saving the md5 hash if you also have the plaintext available.
But of course the more secure thing is to save just the hash.

if ($result)
    {
    echo "$user_name was added. ";
    }

  if (!$result)

This should be else; having two mutually-exclusive
if's in a row might give the same effect, but it's quite harmful
to the reader who is trying to understand the intent of your
code.

{
    if (mysql_errno() == "1062")
      {
      echo "Sorry, $user_name is already taken."
        . "Please choose another name. ";

      }
    else
      {
      echo "Something bad happened: " . mysql_error();

      }
    else
      {
      // our nasty little else
      }
    }
  }

This last else should just go away; it doesn't represent
anything that should happen.

One improvement on the above might be to use elseif to
flatten out the nesting:

if ($result)
    {
    echo "$user_name was added. ";
    }
  elseif (mysql_errno() == "1062")
    {
    echo "Sorry, $user_name is already taken."
    . "Please choose another name. ";
    exit;
    }
  else
    {
    echo "Something bad happened: " . mysql_error();
    exit;
    }

I've added exit to the elseif and else to emphasize that you
wouldn't go further in the script, the user needs to go back and choose
another name.

Anon

Why are you saving the plaintext password?
There's no point saving the md5 hash if you
also have the plaintext available.

I agree. The only reason I am saving the plaintext for now is strictly for debugging reasons.

The only accounts for now will be made by me, and I need to test out "access" privelages in a wide amount of areas. So to avoid forgetting the password of a particular access account, I just save the plaintext there.

Later, when the script is open for other people to use, I'll simply drop the entire table, and recreate it using (most likely) only the encrypted password. One question I have is if a password retrieval is required in the program, md5 obviously makes that rather impossible.

So, I guess I'll have to think of another way of doing it...rather in just an option to save only the encrypted password, or an option to just use a plaintext password. Hmmm...

I like the idea of simply using elseif. I think it certainly flattents it out and makes it easier to read and understand.

http://crazyoddball.datablocks.net/codetext.txt

I really like it.

I removed the exit at the else point, because I need to print out an $html_footer to assure that the html is complete in case of very whiny browers.

I'll need to add links to login screens or back to creation screens, but that's the general idea.

Thanks alot! I can finally get on with the coding, at long last.

Anon

password retrieval

If you need/have encryption anyway, you can always encrypt
the password instead of hashing it. But then emailing a password
back to a user is insecure, too. Do you have a password-expiration
mechanism? If so, you can always just generate a new, random,
one-time-use password and email that back to the user upon
request. Yes, it's still exposed in plaintext, but at least the
window of opportunity where it's valid is much shorter.

There's still an attack possible, however, if the attacker has
access to observing the victim's email (e.g. access to a POP
server's mail file.) Just wait till you know the victim is unlikely
to be online, file a bogus lost-my-password report, and pick
up the password (recovered or new) out of the victim's
mail spool file.

It all depends on how secure you need it to be. A message
board like this one has far different requirements from a
stock-trading system. :-)

Anon

For the initial testing stages I only have hashing, however hopefully I'll get access to mcrypt later. That'll clear up the need for plaintext.

Hm...password retrieval is such a tricky thing. I'll probably just set an option to remove retrieval for those who are paranoid or without secure email accounts.

The need for security isn't actually present, however it's a hobby of mine to overdo things on the security side 😉

To cook up a good retrieval mechanism...other than telling them not to forget the password.

If only things were that easy...