c# ado.net sqlparameter fails

Question

0.00/5 (No votes)

See more:

I am passing sqlparameter in localize language (Persian) from c# but no rows retrieves. Database already collate for persioan_100_ci_ai and tables are collate database_default

C#

SqlCommand cmd = new SqlCommand();
DataTable dt = new DataTable();
SqlDataReader dr = default(SqlDataReader);
dt.TableName = "temp";
try {
    if (!(conn.State == ConnectionState.Closed))
        conn.Close();
    if (conn.State == ConnectionState.Closed)
        conn.Open();
    cmd.Connection = conn;
    string qry = "Select * from users WHERE [Name]=@UserName AND [Pwd]=@Password";
    cmd.commandtext = qry;
    cmd.Parameters.Add("@UserName", SqlDbType.NVarChar, 50).Value = "ادمین";
    cmd.Parameters.Add("@Password", SqlDbType.NVarChar, 50).Value = "ادمین";
    dr = cmd.ExecuteReader(CommandBehavior.CloseConnection);
    if (dr.HasRows) {
        dt.Load(dr);
    }

    return dt;
} catch (Exception ex) {
    return null;
} finally {
    dt = null;
    cmd.Connection = null;
    cmd.Parameters.Clear();
    cmd.Dispose();
}

It works in SSMS

SQL

declare @UserName nvarchar(50) = 'ادمين'
declare @Password nvarchar(50)= 'ادمين'
select * from Users where [name]=@UserName and [Pwd] = @Password

It even works when I am embedding variables in query instead of parameter

C#

SqlCommand cmd = new SqlCommand();
DataTable dt = new DataTable();
SqlDataReader dr = default(SqlDataReader);
string pLoginName = "ادمین";
string pPassword = "ادمین";
dt.TableName = "temp";
try {
    if (!(conn.State == ConnectionState.Closed))
        conn.Close();
    if (conn.State == ConnectionState.Closed)
        conn.Open();
    cmd.Connection = conn;
    string qry = "Select * from users WHERE [Name]='" + pLoginName + "' AND [Pwd]='" + pPassword + "'";
    cmd.CommandText = qry;
    dr = cmd.ExecuteReader(CommandBehavior.CloseConnection);
    if (dr.HasRows) {
        dt.Load(dr);
    }

    return dt;
} catch (Exception ex) {
    return null;
} finally {
    dt = null;
    cmd.Connection = null;
    cmd.Parameters.Clear();
    cmd.Dispose();
}

Cannot figure out where I am wrong. Please, if any one can point out.

Posted 3-Nov-14 4:55am

shailesh_pujara

Add a Solution

Comments

Maciej Los 3-Nov-14 11:26am

Have you tried to use SP?

shailesh_pujara 3-Nov-14 23:49pm

The original code is in SP only. For simplification I have mentioned the same in query.

PIEBALDconsult 3-Nov-14 12:54pm

Setting the datatype of the parameter is needless; try using AddWithValue and see if that helps.

shailesh_pujara 3-Nov-14 23:50pm

The parameter should be NVARCHAR as i am using unicode characters.

PIEBALDconsult 4-Nov-14 8:16am

Yes, but the system will set it when you set the value.

Richard Deeming 3-Nov-14 14:23pm

It looks like you're storing passwords in plain text. That's a very bad idea.

You should only ever store a salted hash of the user's password, using multiple rounds of a strong hashing algorithm:
Salted Password Hashing - Doing it Right[^]

shailesh_pujara 3-Nov-14 23:51pm

Actual password is in encrypted mode only. Just for simplification I have given here the plain text password.

Richard Deeming 4-Nov-14 5:30am

Encryption is still not a suitable way to store a password for authentication. It's only slightly better than plain text. You still need to store and manage your encryption key securely, and it provides a single point of failure.

I can only think of two situations where storing encrypted passwords is appropriate, and user authentication isn't one of them.

You should be storing a salted hash of the user's password. That way, an attacker cannot recover the original password; they can only try to guess the password and see if the hash matches.

4 solutions

Solution 3

I'm thinking it's this:

C#

} finally {
    dt = null;

(Experimentation has ruled that out.)

Let's try to simplify your code then -- and use using statements as well.

C#

DataTable dt = null ;
using ( SqlCommand cmd = conn.CreateCommand() )
{
    cmd.commandtext = "Select * from users WHERE [Name]=@UserName AND [Pwd]=@Password";
    cmd.Parameters.AddWithValue("@UserName" , "ادمین" ) ;
    cmd.Parameters.AddWithValue("@Password" , "ادمین" ) ;
    if (conn.State == ConnectionState.Closed) conn.Open();
    using ( SqlDataReader dr = cmd.ExecuteReader(CommandBehavior.CloseConnection) )
    {
        dt = new DataTable("temp");
        dt.Load(dr);
    }
}
return dt;

You'll notice I also removed the try/catch, you can add one back in if you need to.

Posted 3-Nov-14 6:54am

PIEBALDconsult

Updated 3-Nov-14 7:32am

v2

Comments

Maciej Los 3-Nov-14 12:57pm

Hawk eye!
+5!

PIEBALDconsult 3-Nov-14 12:59pm

Yes, there are so much needless code in there that it's hard to see what's going on.

shailesh_pujara 3-Nov-14 23:52pm

in any way result dr.hasrows = false

PIEBALDconsult 4-Nov-14 8:18am

You don't need to check that.

Solution 2

Instead of such of query:

C#

string qry = "Select * from users WHERE [Name]='" + pLoginName + "' AND [Pwd]='" + pPassword + "'";

use stored procedure[^].

For further information, please see:
How to: Execute a Stored Procedure that Returns Rows[^]
Walkthrough: Displaying Data Using a Stored Procedure in the GridView Web Server Control[^]

Why to use stored procedures? Because of SQL Injection[^].

SQL Injection and how to avoid it[^]
How To: Protect From SQL Injection in ASP.NET[^]

Posted 3-Nov-14 5:32am

Maciej Los

Comments

DamithSL 3-Nov-14 11:37am

I think what OP asking is why the parameter version of code not working while other two working.

Maciej Los 3-Nov-14 11:42am

True. My advice is to use SP instead such of queries. On the other hand, Manas is explaining what OP does wrong.

DamithSL 3-Nov-14 12:50pm

OP trying to use parameterized sql and failed, but inline version and management studio sql statement both working fine. Parameterize quarry will be secure option similar as stored procedure. You have provide good links but I think it is not relevant to OP's exact question. ( But there is no reason to downvote this answer, I'm not the one who downvoted)

shailesh_pujara 3-Nov-14 23:41pm

parameter version is not working - the original code was in Stored procedure only. for simplification I have given as query.

Maciej Los 4-Nov-14 2:02am

I'm glad to hear that you're using stored procedures.
Do not simplify anything in a future. We can get wrong input.

shailesh_pujara 3-Nov-14 23:41pm

you are right - parameter version is not working - the original code was in Stored procedure only. for simplification I have given as query.

Manas Bhardwaj 3-Nov-14 11:40am

Yes, valid point regarding injection +5!

Maciej Los 3-Nov-14 11:42am

Thank you, Manas ;)

Maciej Los 3-Nov-14 12:49pm

I hate downvoters, which down-voted answer without any explanation!

PIEBALDconsult 3-Nov-14 12:49pm

He's having trouble passing parameters, whether he's passing them to a procedure or not doesn't matter.

Maciej Los 3-Nov-14 13:00pm

So what? Is it wrong to recommend SP instead such of query? Is it wrong to use methods which can help avoid sql injection?

PIEBALDconsult 3-Nov-14 13:04pm

The use of parameters is all that's needed to protect against SQL injection. Procedures are not a silver bullet and can still be subject to SQL injection if done wrong.
And the OP is trying to use parameters already, while your statements are true, they are not a solution to the problem and would be better as comments.

Maciej Los 3-Nov-14 13:21pm

Thank you for your opinion. I disagree with this statement: "(...) not a solution (...) would be better as comments". So many times my answers have helped questioners to solve their issues.

shailesh_pujara 3-Nov-14 23:42pm

you are right - parameter version is not working - the original code was in Stored procedure only. for simplification I have given as query.

Solution 1

The code you have provided works for me. However, the first example wont even compile unless you change to this.

from

cmd.commandtext = qry;

to

cmd.CommandText = qry;

Posted 3-Nov-14 5:08am

Manas Bhardwaj

Updated 3-Nov-14 5:12am

v2

Comments

Manas Bhardwaj 3-Nov-14 11:13am

The downvoter would please explain the reason as well.

shailesh_pujara 3-Nov-14 23:46pm

You are right- c# is case sensitive language - the code was originally in vb.net but C# community is large - the same question with vb.net not yet replied - while i got 3 feedback instantly here. What I have done is just converted the code to C# and asked the question again.

Maciej Los 3-Nov-14 11:32am

Good point, +5!
C# is case sensitive programming language.

PIEBALDconsult 3-Nov-14 12:29pm

(It wasn't me.)
"no rows retrieves" -- apparently he has the casing correct enough to compile, but just didn't do a clean copy/paste.

shailesh_pujara 3-Nov-14 23:46pm

You are right- c# is case sensitive language - the code was originally in vb.net but C# community is large - the same question with vb.net not yet replied - while i got 3 feedback instantly here. What I have done is just converted the code to C# and asked the question again.

PIEBALDconsult 4-Nov-14 8:37am

"You have caused confusion and delay." -- Mr. Topham Hatt
Don't try to convert to a language you don't know.
If at all possible, abandon VB.net and stick with C#.

shailesh_pujara 3-Nov-14 23:56pm

The code should works as it works in SSMS. But not works at my end. I thought, it might be my mistake at SQL Server - but database correctly collated and the query and even stored procedure call works fine in SSMS. Just it do not works when I do the same thing from ADO.Net.

Add a Solution

Add your solution here

Treat my content as plain text, not as HTML

Preview 0

…

Existing Members

Sign in to your account

...or Join us

Download, Vote, Comment, Publish.

Your Email
Password
Forgot your password?

Your Email
This email is in use. Do you need your password?
Optional Password

I have read and agree to the Terms of Service and Privacy Policy
Please subscribe me to the CodeProject newsletters

When answering a question please:

Read the question carefully.
Understand that English isn't everyone's first language so be lenient of bad spelling and grammar.
If a question is poorly phrased then either ask for clarification, ignore it, or edit the question and fix the problem. Insults are not welcome.
Don't tell someone to read the manual. Chances are they have and don't get it. Provide an answer or move on to the next question.

Let's work to help developers, not make them feel stupid.

This content, along with any associated source code and files, is licensed under The Code Project Open License (CPOL)

shailesh_pujara · Accepted Answer · 2014-11-21T17:18:00

Thanks all for your answers.

I found where I was wrong. There were two databases - English and Persian. The connection class which I have created to connect to appropriate database has bug and sometime due to bug it was connecting to English database i/o Persian.

Now I resolved the bug. Thanks to my colleague Chintan Bilimoria, for pointing out - whether you are selecting right database?

c# ado.net sqlparameter fails

4 solutions

Solution 3

Solution 2

Solution 1

Solution 4

Add your solution here

Preview 0

Existing Members

...or Join us