PHP select from not working

Question

I am trying to do to a user-pass validation. Updated code:

$username="------";
$password="------";
$database="------";
$host = "------";


mysql_connect($host,$username,$password);
mysql_select_db($database) or die( "Unable to select database");

$username = $_POST['username'];
$password = $_POST['password'];

echo 'username: ' . $username . '<br>';  //myname
echo 'password: ' . $password . '<br>';  //mypass

function checkpass()
{
    //$result = mysql_query("SELECT * FROM REG_USERS where USERNAME='$_POST[username]' and PASSWORD='$_POST[password]' ") or die(mysql_error());  
    $result = mysql_query("SELECT * FROM REG_USERS where USERNAME='" . $username . "' and PASSWORD='" . $password . "'") or die(mysql_error());  
    return  mysql_num_rows($result);
}

checkpass();
echo 'checkpass:'. checkpass();

In the first query checkpass returns 1. In the second one it returns 0. Why?

"Doesn't work" - you should be getting an error message. Which is it? Also, your code is vulnerable to SQL injection.... — Pekka, Jan 09 '13 at 19:08
Just as a note, make sure you escape your user inputs ALWAYS. — Eoin Murphy, Jan 09 '13 at 19:09
Like njk and Eoin say, this code is very dangerous, since it does no security. Please consider using PDO or MySQLi. Edit: Looks like it took me too long to type this :P — Erty Seidohl, Jan 09 '13 at 19:09
I use trim, stripslashes and htmlspecialchars, just removed them for simplicity. — erdomester, Jan 09 '13 at 19:12
`trim, stripslashes and htmlspecialchars` dont escape... you need to be using `mysql_real_escape_string`. Of course you shouldnt be using `mysql_*` anyhow. You should be using `mysqli` or `PDO`. — prodigitalson, Jan 09 '13 at 19:14
By not working I mean that result gives me 0 rows. In the first example it finds the user-password pair. — erdomester, Jan 09 '13 at 19:16
Have you ever considered enabling `error_reporting(E_ALL);` when something doesn't work? — mario, Jan 09 '13 at 19:20
@popnoodles I checked it again. The second line is wrong for some reason. — erdomester, Jan 09 '13 at 19:22
Can you show all the code from connecting to the db (remove the connection details) to iterating the results set — Popnoodles, Jan 09 '13 at 19:24
And can people please stop obsessing about telling anyone who posts code with mysql_ in it that they must immediately use mysqli. There is no rush. http://stackoverflow.com/questions/7206599/switch-to-mysqli-or-stay-with-mysql — Popnoodles, Jan 09 '13 at 19:29
Well at this point, the only difference I see is that final space at the end of the first query. — DWright, Jan 09 '13 at 19:48
Just for kicks, since this is way mysterious. Try this: `$result = mysql_query("SELECT * FROM REG_USERS where USERNAME='$username' and PASSWORD='$password'") or die(mysql_error());` — DWright, Jan 09 '13 at 19:49
And this: `$result = mysql_query("SELECT * FROM REG_USERS where USERNAME='{$_POST['username']}' and PASSWORD='{$_POST['password']}'") or die(mysql_error());` — DWright, Jan 09 '13 at 19:51
This is why **too localized** questions shouldn't be answered in the first place. OP just didn't comprehend variable scope, refused to show the actual code, or read the obvious error messages. Personal debugging session, of no use to future users. — mario, Jan 09 '13 at 19:51

Popnoodles · Accepted Answer · 2013-01-09T19:55:49.300

2

OK now we have the WHOLE code it's obvious.

$username and $password aren't global. The function doesn't know what they are set to.

$username = $_POST['username'];
$password = $_POST['password'];

echo 'username: ' . $username . '<br>';  //myname
echo 'password: ' . $password . '<br>';  //mypass

function checkpass()
{
    $username = mysql_real_escape_string($_POST['username']);
    $password = mysql_real_escape_string($_POST['password']);

    //$result = mysql_query("SELECT * FROM REG_USERS where USERNAME='$_POST[username]' and PASSWORD='$_POST[password]' ") or die(mysql_error());  
    $result = mysql_query("SELECT * FROM REG_USERS where USERNAME='" . $username . "' and PASSWORD='" . $password . "'") or die(mysql_error());  
    return  mysql_num_rows($result);
}

NB I know about mysqli. OP isn't asking about it, I'm not obsessing about it but I have added mysql_real_escape_string for good measure.

edited Jan 09 '13 at 19:55

answered Jan 09 '13 at 19:48

Popnoodles

28,090
2
45
53

What a noob mistake. Thank you. – erdomester Jan 09 '13 at 19:50
You're not obsessing about it? Does that mean you're not discouraging the use of it? – Kermit Jan 09 '13 at 19:53
Given the code in the first place, all of us would have spotted it. – Popnoodles Jan 09 '13 at 19:58
1

Can I just point out mario commented about using `error_reporting(E_ALL);` - PHP would have told you those variables weren't set. – Popnoodles Jan 09 '13 at 20:04

DWright · Answer 2 · 2013-01-09T19:45:43.813

0

Let's set aside the issues with mysql interface (not good to use going forward) and the fact that you really really don't want to put any kind of user affected variables straight into your sql strings [SQL injection!!!!], you say the following in your description that this:

$result = mysql_query("SELECT * FROM REG_USERS where USERNAME='" . $username . "' and PASSWORD='" . $password . "'") or die(mysql_error());

doesn't work.

Why would it? $username and $password are set to nothing. In your first example, you first set them to something, but then you don't use them (you use the straight $_POST variables instead).

In your second example, you don't set them.

So does the following work (note you don't need to concatenate the variables to get them into the string here)?

   $username = $_POST['username'];  
   $password = $_POST['password'];  

   $result = mysql_query("SELECT * FROM REG_USERS where  
   USERNAME='$username' and PASSWORD='$password'") or die(mysql_error());

Again, make sure you never actually do this in real code (just putting variables right into the SQL string). Use prepared statements instead.

edited Jan 09 '13 at 19:45

answered Jan 09 '13 at 19:29

DWright

9,258
4
36
53

I don't think the examples are isolated such that those vars aren't set, but I have asked for the whole code, just to check. No response yet. – Popnoodles Jan 09 '13 at 19:32
I had missed that in the comments. – DWright Jan 09 '13 at 19:39
THe examples are not isolated, of course, so this answer is pointless. – erdomester Jan 09 '13 at 19:40
Ok, but if you improve this in the question that will be evident. – DWright Jan 09 '13 at 19:42
And did you double check whether my snippet also failed? That would be interesting to know although it would not answer you question yet. – DWright Jan 09 '13 at 19:44
And are you running both of your examples with the same user/pass? – DWright Jan 09 '13 at 19:44
I have just copy pasted a block from your code and updated the question with it. I am using the same user-pass pair always. – erdomester Jan 09 '13 at 19:45
Ok, good to know. (I figured you would). – DWright Jan 09 '13 at 19:46
Thank you all guys for trying to help me. Even though popnoodles found the error, I am going with PDO. Looks like I have a lot to read in the upcoming days. – erdomester Jan 09 '13 at 19:51
Final comment. @popnoodles totally fixed this. But you see the value of truly clarifying what code is being executed, which is what I was trying to achieve in my stuff (and so was popnoodles all along). So this kind of answer may not seem helpful, and may not be the answer that should ultimately get the vote (@popnoodles answer rightly earned it), but it can still really help clarity. And it's hard to do in comments. – DWright Jan 09 '13 at 19:57

score -1 · Answer 3 · answered Jan 09 '13 at 19:16

-1

Here's a solution using mysqli_:

$link = mysqli_connect("localhost", "my_user", "my_password", "world");

/* check connection */
if (mysqli_connect_errno()) {
    printf("Connect failed: %s\n", mysqli_connect_error());
    exit();
}

$city = "Amersfoort";

/* create a prepared statement */
if ($stmt = mysqli_prepare($link, "SELECT USERNAME FROM REG_USERS WHERE USERNAME = ? AND PASSWORD = ?")) {

    /* bind parameters for markers */
    mysqli_stmt_bind_param($stmt, "ss", $_POST[username], $_POST[password]);

    /* execute query */
    mysqli_stmt_execute($stmt);

    /* bind result variables */
    mysqli_stmt_bind_result($stmt, $USERNAME);

    /* fetch value */
    mysqli_stmt_fetch($stmt);

    echo "The username is $USERNAME";

    /* close statement */
    mysqli_stmt_close($stmt);
}

/* close connection */
mysqli_close($link);

answered Jan 09 '13 at 19:16

Kermit

33,827
13
85
121

Why downvote this. @njk provide a helpful example of doing this properly. – DWright Jan 09 '13 at 19:34
2

I would imagine because it doesn't actually answer the question. – Popnoodles Jan 09 '13 at 19:35
@DWright It takes more than a single click to leave a comment. That's why Amazon is so addicting. Nice to see another individual from the Chicago area. – Kermit Jan 09 '13 at 19:36
@popnoodles. I see. But if the user follows erdomester follows njk's example, whatever the problem is will clear up and erodmester will have some nice, correct code. i.e., njk erodmester will find the question to have been answered in any case. – DWright Jan 09 '13 at 19:40
I wouldn't say that with confidence because it doesn't look like there's actually anything wrong with the code given. Changing 1 line the OP understands into many using something new to them probably won't help. – Popnoodles Jan 09 '13 at 19:45
Per [SO's FAQ](http://stackoverflow.com/questions/how-to-answer), *"Make sure your answer provides that – or a viable alternative."* Seems like this is quite a viable alternative. – Kermit Jan 09 '13 at 19:48
Let's be fair. Now we've been given the whole code you can see this wouldn't have fixed the issue. – Popnoodles Jan 09 '13 at 19:54
@popnoodles Actually, it would have. My code uses `$_POST`. – Kermit Jan 09 '13 at 19:55
1

so you have. my bad. but they would never have understood what was wrong and it didn't answer the question which was why wasn't it working. – Popnoodles Jan 09 '13 at 19:57

PHP select from not working

3 Answers3