SqlDataReader reads every other row?

Question

I tried going through my code for hours trying to see where I went wrong and google doesn't seem to have the answer either. Basically I am running this code:

public bool LoginRequest(string ReceivedUsername, string ReceivedPassword)
    {

        bool ValidLogin = false;

        try
        {

            using (SqlConnection myConnection = new SqlConnection(ConnectString))
            {
                myConnection.Open();
                Log.Debug("Succesful sql connection");
                SqlCommand userSELECTcom = new SqlCommand("SELECT username,password FROM users;", myConnection);
                SqlDataReader reader = userSELECTcom.ExecuteReader();

                    //verify login
                    while (reader.Read())
                    {
                        CompareUsername = reader["username"].ToString();
                        ComparePassword = reader["password"].ToString();
                        Log.Debug(ReceivedUsername + " against " + CompareUsername);
                        Log.Debug(ReceivedPassword + " against " + ComparePassword);

                        if (CompareUsername == ReceivedUsername && ComparePassword == ReceivedPassword)
                        {
                            ValidLogin = true;
                            Log.Debug(ReceivedUsername + " has logged in successfully!!!");
                            myConnection.Close();//close sql conn
                            reader.Close();//close sqldatareader
                            return ValidLogin;
                        }

                        else if (CompareUsername != ReceivedUsername || ComparePassword != ReceivedPassword)
                        {
                            if (!reader.Read())
                            {
                                Log.Debug(ReceivedUsername + " has not logged in successfully with password: " + ReceivedPassword);
                                myConnection.Close();//close sql conn
                                reader.Close();//close sql data reader
                                return ValidLogin;
                            }
                        }
                    }
                    //end of verify sequence
            }

        }
        //logging any login request issues
        catch (Exception e)
        {
            Log.Debug(e);
        }
        return ValidLogin;

    }

I have a logging program set up that tells me everything thats happening as the code gets executed. These lines: " Log.Debug(ReceivedUsername + " against " + CompareUsername); Log.Debug(ReceivedPassword + " against " + ComparePassword); "

helps me see which row is being checked by the reader. I tried with six rows each with unique usernames and passwords and the result basically shows that only row 1, 3 and 5 is checked by the reader against the input from the user. So if I tried to log in with my client using a username and password from row 2, 4 or 6 I get an error saying my log in failed. Can anyone explain why this happens?

Just a side note for you. If you are using the 'using' keyword around your connection there is NO need for you to call. Close on it. It inherits IDisposable, which using calls for you. — Cubicle.Jockey
– Cubicle.Jockey, Commented Aug 28, 2014 at 4:05
Normally it is good practice to query directly from database instead of retrieving all data from Table. Recommended to use where clause. — Hassan Nisar
– Hassan Nisar, Commented Aug 28, 2014 at 4:06
Definitely taking that into account Cubicle.Jockey. And could you explain further Hassan? — Jaja
– Jaja, Commented Aug 28, 2014 at 4:11
@Jaja. Yes. I am asking to add where clause into your sql query. — Hassan Nisar
– Hassan Nisar, Commented Aug 28, 2014 at 4:13
@HassanNisar Didn't quite know about this clause so I did some research on it. I may be wrong but wont this sort of statement leave you vulnerable to SQL Injection attacks? SELECT username FROM users WHERE username = 'var_user_input' — Jaja
– Jaja, Commented Aug 28, 2014 at 4:24

Andrew Barber · Accepted Answer · 2014-08-28 04:04:52Z

2

You have an extra Reader.Read() call in your condition where you didn't find the login that time. That's skipping to the next record, then your main loop's Reader.Read() goes to the next.

You don't need to loop like this, though. Build a query that looks for a record by the username. If there are no records, login fails. If there is one, check the password.

answered Aug 28, 2014 at 4:04

Andrew Barber

40.3k20 gold badges97 silver badges124 bronze badges

Sign up to request clarification or add additional context in comments.

2 Comments

Jaja Over a year ago

Ahhhh I see....makes sense. But before I try the code, how can I check that the reader still has rows left without the condition !Reader.read()?

Hassan Nisar Over a year ago

Reader.HasRows could be used.

Wagner DosAnjos · Accepted Answer · 2014-08-28 04:05:42Z

1

You have a 2nd reader.Read() on the if statement inside the while block. That is causing your code to skip a record.

answered Aug 28, 2014 at 4:05

Wagner DosAnjos

6,3841 gold badge18 silver badges30 bronze badges

Comments

Hassan Nisar · Accepted Answer · 2014-08-28 04:17:09Z

0

To keep things simple you could directly query from database.

Below is example code to check if the received username and password exists in the db:

string sql = @"SELECT username,password FROM users 
             WHERE username=@username and password = @password";

SqlCommand userSELECTcom = new SqlCommand(sql, myConnection);
userSELECTcom.Parameters.AddWithValue(@username, ReceivedUsername);
userSELECTcom.Parameters.AddWithValue(@password, ReceivedPassword);

using(SqlDataReader reader = userSELECTcom.ExecuteReader())
{
   ValidLogin = reader.HasRows; 
}

answered Aug 28, 2014 at 4:17

Hassan Nisar

5,4502 gold badges26 silver badges35 bronze badges

6 Comments

Jaja Over a year ago

Thanks alot...much shorter than the code I used! I'm going to modify my code right away.

Hassan Nisar Over a year ago

@Jaja. Please note that. You could avoid declaration of bool ValidLogin. You could directly use return reader.HasRows;. Also when you use using statement for SqlConnection then you don't have to close the connection, you could skip myConnection.Close().

Jaja Over a year ago

Yes, I removed all of my connection.Close() statements where they were not needed. I am wondering whether or not now to return false in my catch statement though?

Hassan Nisar Over a year ago

Well you could return false within catch block. Also you could write entry in Log as well.

Jaja Over a year ago

Thanks...and one last question: is it better to assign the return value of a function to a boolean before using it in an if condition statement or should I just call the function within the if parentheses?

|

senthilkumar2185 · Accepted Answer · 2014-08-28 04:23:31Z

0

else if (CompareUsername != ReceivedUsername || ComparePassword != ReceivedPassword)
{
if (!reader.Read())  //remove this condition it will skip the current loop                             
{
Log.Debug(ReceivedUsername + " has not logged in successfully with password: " + ReceivedPassword);
myConnection.Close();//close sql conn
reader.Close();//close sql data reader
return ValidLogin;
}
}

edited Aug 28, 2014 at 4:23

senthilkumar2185

2,5983 gold badges23 silver badges38 bronze badges

answered Aug 28, 2014 at 4:11

ESWARAN M

4710 bronze badges

1 Comment

Hassan Nisar Over a year ago

Can you indent your answer properly? It looks very odd.

Collectives™ on Stack Overflow

SqlDataReader reads every other row?

4 Answers 4

2 Comments

Comments

6 Comments

1 Comment

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

4 Answers 4

2 Comments

Comments

6 Comments

1 Comment

Your Answer

Sign up or log in

Post as a guest

Related