datatable.rows.count always returns 0

Question

I'm creating a simple web service to authenticate a user.

I have a problem filling the datatable(used to store the results of a select statement) properly, 'dt.rows.count' (dt is the name of the datatable) always returns 0 even if the select statement returns nothing. I've tried clearing the datatable before filling it, and after the if condition as well, but to no avail, I get the same result.

Would really appreciate any advice on how to proceed.

[WebMethod]
    public string Authen(string a, string b)
    {
       var con = new SqlConnection("Data Source=SERVER-SQL;Initial Catalog=DECA-DB;Persist Security Info=True;User ID=sa;Password=*****");
       var sda = new SqlDataAdapter("SELECT * FROM Login_Matrix WHERE Username = ' " + a + " ' AND Password = ' " + b + " '", con);
        var dt = new DataTable();

        con.Open();
        dt.Clear();
        sda.Fill(dt);            
        con.Close();
        int x = dt.Rows.Count;
        //return (x);

        if ( x >0)
        {
            dt.Clear();
            return ("In");
        }

        else
        {
            dt.Clear();
            return ("out");
        }

        }
    }

Seems like you close the `sqlconnection` – Ilyes Apr 15 '17 at 08:49 — Ilyes, Apr 15 '17 at 08:49
close your sql connection after else part. – Mahi Apr 15 '17 at 08:51 — Mahi, Apr 15 '17 at 08:51

score 3 · Accepted Answer · edited May 23 '17 at 12:32

Adding a space after and before the single quotes makes your query search for inexistant user names and passords (like " Steve ") and it return no records

A quick fix could be

var sda = new SqlDataAdapter(@"SELECT * FROM Login_Matrix 
                               WHERE Username = '" + a + "' 
                                 AND Password = '" + b + "'", con);

but this is very dangerous.
This code is vulnerable to Sql Injection attacks.
You should use parameters

var sda = new SqlDataAdapter(@"SELECT * FROM Login_Matrix 
                              WHERE Username =  @uname 
                                AND Password = @pwd", con);
sda.SelectCommand.Parameters.Add("@uname", SqlDbType.NVarChar).Value = a;
sda.SelectCommand.Parameters.Add("@pwd", SqlDbType.NVarChar).Value = b;

And on the same line about security, another thing to consider as soon as possible, is that storing plain text password in your database is a really big security risk. You should search how to salt and store an hash of the password

There are other parts of this code to improve.

First you need to have using statements around the disposable objects like the connection or the command.
Second, there is no need to have a full SqlDataAdapter and a DataTable to just check if the user exists or not.

So you can rewrite your code as:

string cmdText = @"IF EXISTS(SELECT 1 FROM Login_Matrix 
                   WHERE Username = @uname AND Password = @pwd)
                   SELECT 1 ELSE SELECT 0";

using(SqlConnection con = new SqlConnection("....."))
using(SqlCommand cmd = new SqlCommand(cmdText, con))
{
    con.Open();
    int result = (int)cmd.ExecuteScalar();
    return ( result == 1 ? "In" : "out");
}

datatable.rows.count always returns 0

1 Answers1